Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Pub's pubspec parsing to use a data class and KxS / kaml #9080

Merged
merged 4 commits into from
Sep 5, 2024
Merged

Refactor Pub's pubspec parsing to use a data class and KxS / kaml #9080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd0c482
refactor(pub): Port Pubspec parsing to KxS and use a data class
fviernau Sep 4, 2024
9ff4408
refactor(pub): Use a more speaking name for `pubspec`
fviernau Sep 5, 2024
dc2b9a6
refactor(pub): Rename several `manifest` variable
fviernau Sep 5, 2024
197d265
refactor(pub): Inline some variables in `parseProject()`
fviernau Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions plugins/package-managers/pub/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
plugins {
// Apply precompiled plugins.
id("ort-library-conventions")

// Apply third-party plugins.
alias(libs.plugins.kotlinSerialization)
}

dependencies {
Expand All @@ -40,6 +43,8 @@ dependencies {
implementation(libs.jackson.databind)
implementation(libs.jackson.dataformat.yaml)
implementation(libs.jackson.module.kotlin)
implementation(libs.kotlinx.serialization.core)
implementation(libs.kotlinx.serialization.yaml)

funTestImplementation(projects.plugins.packageManagers.gradlePackageManager)

Expand Down
92 changes: 43 additions & 49 deletions plugins/package-managers/pub/src/main/kotlin/Pub.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.ossreviewtoolkit.plugins.packagemanagers.pub

import com.fasterxml.jackson.databind.JsonNode
import com.fasterxml.jackson.dataformat.yaml.JacksonYAMLParseException

import java.io.File
Expand All @@ -37,7 +36,6 @@ import org.ossreviewtoolkit.analyzer.parseAuthorString
import org.ossreviewtoolkit.analyzer.toPackageReference
import org.ossreviewtoolkit.downloader.VcsHost
import org.ossreviewtoolkit.downloader.VersionControlSystem
import org.ossreviewtoolkit.model.EMPTY_JSON_NODE
import org.ossreviewtoolkit.model.Hash
import org.ossreviewtoolkit.model.HashAlgorithm
import org.ossreviewtoolkit.model.Identifier
Expand All @@ -56,17 +54,15 @@ import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
import org.ossreviewtoolkit.model.config.PackageManagerConfiguration
import org.ossreviewtoolkit.model.config.RepositoryConfiguration
import org.ossreviewtoolkit.model.createAndLogIssue
import org.ossreviewtoolkit.model.yamlMapper
import org.ossreviewtoolkit.plugins.packagemanagers.pub.Pubspec.Dependency
import org.ossreviewtoolkit.plugins.packagemanagers.pub.utils.PubCacheReader
import org.ossreviewtoolkit.utils.common.CommandLineTool
import org.ossreviewtoolkit.utils.common.Os
import org.ossreviewtoolkit.utils.common.ProcessCapture
import org.ossreviewtoolkit.utils.common.collectMessages
import org.ossreviewtoolkit.utils.common.isNotEmpty
import org.ossreviewtoolkit.utils.common.realFile
import org.ossreviewtoolkit.utils.common.safeMkdirs
import org.ossreviewtoolkit.utils.common.splitOnWhitespace
import org.ossreviewtoolkit.utils.common.textValueOrEmpty
import org.ossreviewtoolkit.utils.common.unpack
import org.ossreviewtoolkit.utils.ort.downloadFile
import org.ossreviewtoolkit.utils.ort.normalizeVcsUrl
Expand Down Expand Up @@ -262,11 +258,9 @@ class Pub(

override fun resolveDependencies(definitionFile: File, labels: Map<String, String>): List<ProjectAnalyzerResult> {
val workingDir = definitionFile.parentFile
val manifest = yamlMapper.readTree(definitionFile)
val pubspec = parsePubspec(definitionFile)

val hasDependencies = manifest.fields().asSequence().any { (key, value) ->
key.startsWith("dependencies") && value.isNotEmpty()
}
val hasDependencies = pubspec.getScopeDependencies(SCOPE_NAME_DEPENDENCIES).isNotEmpty()

val packages = mutableMapOf<Identifier, Package>()
val scopes = mutableSetOf<Scope>()
Expand Down Expand Up @@ -318,11 +312,11 @@ class Pub(
logger.info { "Successfully parsed installed packages." }

ALL_PUB_SCOPE_NAMES.mapTo(scopes) { scopeName ->
parseScope(scopeName, manifest, lockfile, parsePackagesResult.packages, labels, workingDir)
parseScope(scopeName, pubspec, lockfile, parsePackagesResult.packages, labels, workingDir)
}
}

val project = parseProject(definitionFile, manifest, scopes)
val project = parseProject(definitionFile, pubspec, scopes)

projectAnalyzerResults += ProjectAnalyzerResult(project, packages.values.toSet(), issues)

Expand All @@ -331,32 +325,32 @@ class Pub(

private fun parseScope(
scopeName: String,
manifest: JsonNode,
pubspec: Pubspec,
lockfile: Lockfile,
packages: Map<Identifier, Package>,
labels: Map<String, String>,
workingDir: File
): Scope {
val packageName = manifest["name"].textValue()
val packageName = pubspec.name

logger.info { "Parsing scope '$scopeName' for package '$packageName'." }

val requiredPackages = manifest[scopeName]?.fieldNames()?.asSequence().orEmpty().toList()
val dependencies = buildDependencyTree(requiredPackages, manifest, lockfile, packages, labels, workingDir)
val requiredPackages = pubspec.getScopeDependencies(scopeName).keys.toList()
val dependencies = buildDependencyTree(requiredPackages, pubspec, lockfile, packages, labels, workingDir)
return Scope(scopeName, dependencies)
}

private fun buildDependencyTree(
dependencies: List<String>,
manifest: JsonNode,
pubspec: Pubspec?,
lockfile: Lockfile,
packages: Map<Identifier, Package>,
labels: Map<String, String>,
workingDir: File,
processedPackages: Set<String> = emptySet()
): Set<PackageReference> {
val packageReferences = mutableSetOf<PackageReference>()
val nameOfCurrentPackage = manifest["name"].textValue()
val nameOfCurrentPackage = pubspec?.name.orEmpty()
val containsFlutter = "flutter" in dependencies

logger.debug { "Building dependency tree for package '$nameOfCurrentPackage'." }
Expand All @@ -382,13 +376,12 @@ class Pub(
val packageInfo = packages[id] ?: throw IOException("Could not find package info for $packageName")

try {
val dependencyYamlFile = readPackageInfoFromCache(pkgInfoFromLockfile, workingDir)
val requiredPackages =
dependencyYamlFile["dependencies"]?.fieldNames()?.asSequence()?.toList().orEmpty()
val dependencyPubspec = readPackageInfoFromCache(pkgInfoFromLockfile, workingDir)
val requiredPackages = dependencyPubspec?.dependencies.orEmpty().keys.toList()

val transitiveDependencies = buildDependencyTree(
dependencies = requiredPackages,
manifest = dependencyYamlFile,
pubspec = dependencyPubspec,
lockfile = lockfile,
packages = packages,
labels = labels,
Expand Down Expand Up @@ -496,24 +489,20 @@ class Pub(
return ProjectAnalyzerResult(Project.EMPTY, emptySet(), listOf(issue))
}

private fun parseProject(definitionFile: File, pubspec: JsonNode, scopes: Set<Scope>): Project {
private fun parseProject(definitionFile: File, pubspec: Pubspec, scopes: Set<Scope>): Project {
// See https://dart.dev/tools/pub/pubspec for supported fields.
val rawName = pubspec["name"]?.textValue() ?: getFallbackProjectName(analysisRoot, definitionFile)
val homepageUrl = pubspec["homepage"].textValueOrEmpty()
val repositoryUrl = pubspec["repository"].textValueOrEmpty()
val authors = parseAuthors(pubspec)

val vcs = VcsHost.parseUrl(repositoryUrl)
val homepageUrl = pubspec.homepage.orEmpty()
val vcs = VcsHost.parseUrl(pubspec.repository.orEmpty())

return Project(
id = Identifier(
type = managerName,
namespace = "",
name = rawName,
version = pubspec["version"].textValueOrEmpty()
name = pubspec.name,
version = pubspec.version.orEmpty()
),
definitionFilePath = VersionControlSystem.getPathInfo(definitionFile).path,
authors = authors,
authors = parseAuthors(pubspec),
// Pub does not declare any licenses in the pubspec files, therefore we keep this empty.
declaredLicenses = emptySet(),
vcs = vcs,
Expand Down Expand Up @@ -564,10 +553,10 @@ class Pub(
source == "git" -> {
val pkgInfoFromYamlFile = readPackageInfoFromCache(packageInfo, workingDir)

rawName = pkgInfoFromYamlFile["name"]?.textValue() ?: packageName
description = pkgInfoFromYamlFile["description"].textValueOrEmpty().trim()
homepageUrl = pkgInfoFromYamlFile["homepage"].textValueOrEmpty()
authors = parseAuthors(pkgInfoFromYamlFile)
rawName = pkgInfoFromYamlFile?.name ?: packageName
description = pkgInfoFromYamlFile?.description.orEmpty().trim()
homepageUrl = pkgInfoFromYamlFile?.homepage.orEmpty()
authors = pkgInfoFromYamlFile?.let { parseAuthors(it) }.orEmpty()

vcs = VcsInfo(
type = VcsType.GIT,
Expand All @@ -581,12 +570,12 @@ class Pub(
source != "sdk" -> {
val pkgInfoFromYamlFile = readPackageInfoFromCache(packageInfo, workingDir)

rawName = pkgInfoFromYamlFile["name"].textValueOrEmpty()
description = pkgInfoFromYamlFile["description"].textValueOrEmpty().trim()
homepageUrl = pkgInfoFromYamlFile["homepage"].textValueOrEmpty()
authors = parseAuthors(pkgInfoFromYamlFile)
rawName = pkgInfoFromYamlFile?.name.orEmpty()
description = pkgInfoFromYamlFile?.description.orEmpty().trim()
homepageUrl = pkgInfoFromYamlFile?.homepage.orEmpty()
authors = pkgInfoFromYamlFile?.let { parseAuthors(it) }.orEmpty()

val repositoryUrl = pkgInfoFromYamlFile["repository"].textValueOrEmpty()
val repositoryUrl = pkgInfoFromYamlFile?.repository.orEmpty()

// Ignore the revision parsed from the repositoryUrl because the URL often points to the
// main or master branch of the repository but never to the correct revision that matches
Expand Down Expand Up @@ -688,7 +677,7 @@ class Pub(
return ParsePackagesResult(packages, issues)
}

private fun readPackageInfoFromCache(packageInfo: PackageInfo, workingDir: File): JsonNode {
private fun readPackageInfoFromCache(packageInfo: PackageInfo, workingDir: File): Pubspec? {
val definitionFile = reader.findFile(packageInfo, workingDir, PUBSPEC_YAML)
if (definitionFile == null) {
createAndLogIssue(
Expand All @@ -697,10 +686,10 @@ class Pub(
severity = Severity.WARNING
)

return EMPTY_JSON_NODE
return null
}

return yamlMapper.readTree(definitionFile)
return parsePubspec(definitionFile)
}

override fun getVersion(workingDir: File?): String {
Expand Down Expand Up @@ -755,9 +744,9 @@ class Pub(
* Check the [PUBSPEC_YAML] within [workingDir] if the project contains the Flutter SDK.
*/
private fun containsFlutterSdk(workingDir: File): Boolean {
val specFile = yamlMapper.readTree(workingDir.resolve(PUBSPEC_YAML))
val pubspec = parsePubspec(workingDir.resolve(PUBSPEC_YAML))

return specFile?.get("dependencies")?.get("flutter")?.get("sdk")?.textValue() == "flutter"
return (pubspec.dependencies?.get("flutter") as? Pubspec.SdkDependency)?.sdk == "flutter"
}

/**
Expand All @@ -777,12 +766,17 @@ class Pub(
/**
* Extract information about package authors from the given [pubspec].
*/
private fun parseAuthors(pubspec: JsonNode): Set<String> =
(setOfNotNull(pubspec["author"]) + pubspec["authors"]?.toSet().orEmpty()).mapNotNullTo(mutableSetOf()) {
parseAuthorString(it.textValue())
}
private fun parseAuthors(pubspec: Pubspec): Set<String> =
(pubspec.authors + pubspec.author).mapNotNullTo(mutableSetOf()) { parseAuthorString(it) }

private fun ProjectAnalyzerResult.collectPackagesByScope(scopeName: String): List<Package> {
val scope = project.scopes.find { it.name == scopeName } ?: return emptyList()
return packages.filter { it.id in scope }
}

private fun Pubspec.getScopeDependencies(scopeName: String): Map<String, Dependency> =
when (scopeName) {
SCOPE_NAME_DEPENDENCIES -> dependencies.orEmpty()
SCOPE_NAME_DEV_DEPENDENCIES -> devDependencies.orEmpty()
else -> error("Invalid scope name: '$scopeName'.")
}
Loading
Loading