diff --git a/plugins/scanners/fossid/src/main/kotlin/FossId.kt b/plugins/scanners/fossid/src/main/kotlin/FossId.kt index 5c3146ea81e37..97e6124e99b44 100644 --- a/plugins/scanners/fossid/src/main/kotlin/FossId.kt +++ b/plugins/scanners/fossid/src/main/kotlin/FossId.kt @@ -302,12 +302,10 @@ class FossId internal constructor( .checkResponse("list scans for project").data checkNotNull(scans) - val issues = mutableListOf() - - val (scanCode, scanId) = if (config.deltaScans) { - checkAndCreateDeltaScan(scans, url, revision, projectCode, projectName, context, issues) + val result = if (config.deltaScans) { + checkAndCreateDeltaScan(scans, url, revision, projectCode, projectName, context) } else { - checkAndCreateScan(scans, url, revision, projectCode, projectName, context, issues) + checkAndCreateScan(scans, url, revision, projectCode, projectName, context) } if (config.waitForResult && provenance is RepositoryProvenance) { @@ -332,14 +330,12 @@ class FossId internal constructor( "false positives." } - val rawResults = getRawResults(scanCode, snippetChoices?.choices.orEmpty()) + val rawResults = getRawResults(result.scanCode, snippetChoices?.choices.orEmpty()) createResultSummary( startTime, provenance, rawResults, - scanCode, - scanId, - issues, + result, context.detectedLicenseMapping, snippetChoices?.choices.orEmpty() ) @@ -356,7 +352,11 @@ class FossId internal constructor( provenance, details, summary, - mapOf(SCAN_CODE_KEY to scanCode, SCAN_ID_KEY to scanId, SERVER_URL_KEY to config.serverUrl) + mapOf( + SCAN_CODE_KEY to result.scanCode, + SCAN_ID_KEY to result.scanId, + SERVER_URL_KEY to config.serverUrl + ) ) } } catch (e: IllegalStateException) { @@ -462,12 +462,11 @@ class FossId internal constructor( revision: String, projectCode: String, projectName: String, - context: ScanContext, - issues: MutableList - ): Pair { + context: ScanContext + ): FossIdResult { val existingScan = scans.recentScansForRepository(url, revision = revision).findLatestPendingOrFinishedScan() - val scanCodeAndId = if (existingScan == null) { + val result = if (existingScan == null) { logger.info { "No scan found for $url and revision $revision. Creating scan..." } val scanCode = namingProvider.createScanCode(projectName = projectName, branch = revision) @@ -478,9 +477,9 @@ class FossId internal constructor( service.downloadFromGit(config.user, config.apiKey, scanCode) .checkResponse("download data from Git", false) - createIgnoreRules(scanCode, context.excludes, issues = issues) + val issues = createIgnoreRules(scanCode, context.excludes) - scanCode to scanId + FossIdResult(scanCode, scanId, issues) } else { logger.info { "Scan '${existingScan.code}' found for $url and revision $revision." } @@ -488,12 +487,12 @@ class FossId internal constructor( "The code for an existing scan must not be null." } - existingScanCode to existingScan.id.toString() + FossIdResult(existingScanCode, existingScan.id.toString()) } - if (config.waitForResult) checkScan(scanCodeAndId.first) + if (config.waitForResult) checkScan(result.scanCode) - return scanCodeAndId + return result } /** @@ -505,9 +504,8 @@ class FossId internal constructor( revision: String, projectCode: String, projectName: String, - context: ScanContext, - issues: MutableList - ): Pair { + context: ScanContext + ): FossIdResult { val projectRevision = context.labels[PROJECT_REVISION_LABEL] val vcs = requireNotNull(VersionControlSystem.forUrl(url)) @@ -561,8 +559,10 @@ class FossId internal constructor( service.downloadFromGit(config.user, config.apiKey, scanCode) .checkResponse("download data from Git", false) + val issues = mutableListOf() + if (existingScan == null) { - createIgnoreRules(scanCode, context.excludes, issues = issues) + issues += createIgnoreRules(scanCode, context.excludes) if (config.waitForResult) checkScan(scanCode) } else { @@ -585,7 +585,7 @@ class FossId internal constructor( val exclusions = setOf(".git", "^\\.git") val filteredRules = rules.filterNot { it.type == RuleType.DIRECTORY && it.value in exclusions } - createIgnoreRules(scanCode, context.excludes, filteredRules, issues) + issues += createIgnoreRules(scanCode, context.excludes, filteredRules) } logger.info { "Reusing identifications from scan '$existingScanCode'." } @@ -601,7 +601,7 @@ class FossId internal constructor( enforceDeltaScanLimit(recentScans) } - return scanCode to scanId + return FossIdResult(scanCode, scanId, issues) } /** @@ -631,17 +631,16 @@ class FossId internal constructor( private suspend fun createIgnoreRules( scanCode: String, excludes: Excludes?, - existingRules: List = emptyList(), - issues : MutableList - ) { - val excludesRules = excludes?.let { - convertRules(it, issues).also { - logger.info { "${it.size} rules from ORT excludes have been found." } + existingRules: List = emptyList() + ): List { + val (excludesRules, excludeRuleIssues) = (excludes ?: Excludes.EMPTY).let { + convertRules(it).also { (rules, _) -> + logger.info { "${rules.size} rules from ORT excludes have been found." } } - }.orEmpty() + } // Create an issue for each legacy rule. - val legacyRules = existingRules.filterLegacyRules(excludesRules, issues) + val (legacyRules, legacyRuleIssues) = existingRules.filterLegacyRules(excludesRules) if (legacyRules.isNotEmpty()) { logger.warn { "${legacyRules.size} legacy rules have been found." } } @@ -654,6 +653,8 @@ class FossId internal constructor( "Ignore rule of type '${it.type}' and value '${it.value}' has been created for the new scan." } } + + return excludeRuleIssues + legacyRuleIssues } /** @@ -910,9 +911,7 @@ class FossId internal constructor( startTime: Instant, provenance: Provenance, rawResults: RawResults, - scanCode: String, - scanId: String, - additionalIssues: List, + result: FossIdResult, detectedLicenseMapping: Map, snippetChoices: List ): ScanResult { @@ -930,7 +929,7 @@ class FossId internal constructor( snippetLicenseFindings ) val newlyMarkedFiles = markFilesWithChosenSnippetsAsIdentified( - scanCode, + result.scanCode, snippetChoices, snippetFindings, rawResults.listPendingFiles, @@ -960,14 +959,14 @@ class FossId internal constructor( licenseFindings = licenseFindings + snippetLicenseFindings, copyrightFindings = copyrightFindings, snippetFindings = snippetFindings, - issues = issues + additionalIssues + issues = issues + result.issues ) return ScanResult( provenance, details, summary, - mapOf(SCAN_CODE_KEY to scanCode, SCAN_ID_KEY to scanId, SERVER_URL_KEY to config.serverUrl) + mapOf(SCAN_CODE_KEY to result.scanCode, SCAN_ID_KEY to result.scanId, SERVER_URL_KEY to config.serverUrl) ) } @@ -1079,3 +1078,9 @@ class FossId internal constructor( return result } } + +private data class FossIdResult( + val scanCode: String, + val scanId: String, + val issues: List = emptyList() +) diff --git a/plugins/scanners/fossid/src/main/kotlin/Utils.kt b/plugins/scanners/fossid/src/main/kotlin/Utils.kt index 88cf28acc9a19..997502bcf511d 100644 --- a/plugins/scanners/fossid/src/main/kotlin/Utils.kt +++ b/plugins/scanners/fossid/src/main/kotlin/Utils.kt @@ -38,11 +38,13 @@ private val EXTENSION_REGEX = "\\*\\.(?\\w+)".toRegex() private val FILE_REGEX = "(?[^/]+)".toRegex() /** - * Convert the ORT [path excludes][Excludes.paths] in [excludes] to FossID [IgnoreRule]s. If an error is encountered - * during the mapping, an issue is added to [issues]. + * Return the ORT [path excludes][Excludes.paths] in [excludes] converted to FossID [IgnoreRule]s and any errors that + * occurred during the conversion. */ -internal fun convertRules(excludes: Excludes, issues: MutableList): List { - return excludes.paths.mapNotNull { pathExclude -> +internal fun convertRules(excludes: Excludes): Pair, List> { + val issues = mutableListOf() + + val ignoreRules = excludes.paths.mapNotNull { pathExclude -> pathExclude.mapToRule().alsoIfNull { issues += Issue( source = "FossID.convertRules", @@ -55,6 +57,8 @@ internal fun convertRules(excludes: Excludes, issues: MutableList): List< } } } + + return ignoreRules to issues } @Suppress("UnsafeCallOnNullableType") @@ -85,20 +89,21 @@ private fun PathExclude.mapToRule(): IgnoreRule? { /** * Filter [IgnoreRule]s which are not contained in the [referenceRules]. These are legacy rules because they were not - * created from the [Excludes] defined in the repository configuration. Also create an [Issue] for each legacy rule and - * add it to [issues]. + * created from the [Excludes] defined in the repository configuration. Also create an [Issue] for each legacy rule. */ -internal fun List.filterLegacyRules( - referenceRules: List, - issues: MutableList -): List = - filterNot { rule -> +internal fun List.filterLegacyRules(referenceRules: List): Pair, List> { + val legacyRules = filterNot { rule -> referenceRules.any { it.value == rule.value && it.type == rule.type } - }.onEach { - issues += Issue( + } + + val issues = legacyRules.map { + Issue( source = "FossID.compare", message = "Rule '${it.value}' with type '${it.type}' is not present in the .ort.yml path excludes. " + "Add it to the .ort.yml file or remove it from the FossID scan.", severity = Severity.HINT ) } + + return legacyRules to issues +} diff --git a/plugins/scanners/fossid/src/test/kotlin/LegacyRulesTest.kt b/plugins/scanners/fossid/src/test/kotlin/LegacyRulesTest.kt index 69748b20e071c..6592184f11ccf 100644 --- a/plugins/scanners/fossid/src/test/kotlin/LegacyRulesTest.kt +++ b/plugins/scanners/fossid/src/test/kotlin/LegacyRulesTest.kt @@ -41,9 +41,8 @@ class LegacyRulesTest : WordSpec({ val rulesToTest = listOf( IgnoreRule(-1, RuleType.EXTENSION, ".pdf", -1, "") ) - val issues = mutableListOf() - val legacyRules = rulesToTest.filterLegacyRules(referenceRules, issues) + val (legacyRules, issues) = rulesToTest.filterLegacyRules(referenceRules) issues shouldHaveSize 1 issues.first() shouldNotBeNull { @@ -63,9 +62,8 @@ class LegacyRulesTest : WordSpec({ val rulesToTest = listOf( IgnoreRule(-1, RuleType.DIRECTORY, "directory", -1, "") ) - val issues = mutableListOf() - val legacyRules = rulesToTest.filterLegacyRules(referenceRules, issues) + val (legacyRules, issues) = rulesToTest.filterLegacyRules(referenceRules) issues should beEmpty() legacyRules should beEmpty() diff --git a/plugins/scanners/fossid/src/test/kotlin/MapIgnoreRulesTest.kt b/plugins/scanners/fossid/src/test/kotlin/MapIgnoreRulesTest.kt index ed02f44f2ccf2..ce000f11e2f09 100644 --- a/plugins/scanners/fossid/src/test/kotlin/MapIgnoreRulesTest.kt +++ b/plugins/scanners/fossid/src/test/kotlin/MapIgnoreRulesTest.kt @@ -27,7 +27,6 @@ import io.kotest.matchers.should import io.kotest.matchers.shouldBe import org.ossreviewtoolkit.clients.fossid.model.rules.RuleType -import org.ossreviewtoolkit.model.Issue import org.ossreviewtoolkit.model.Severity import org.ossreviewtoolkit.model.config.Excludes import org.ossreviewtoolkit.model.config.PathExclude @@ -37,9 +36,8 @@ class MapIgnoreRulesTest : WordSpec({ "convertRules" should { "map rule with directory with **" { val exclude = Excludes(listOf(PathExclude("directory/**", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -52,9 +50,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory containing subdirectories with **" { val exclude = Excludes(listOf(PathExclude("directory/sub1/sub2/**", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -67,9 +64,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory containing a dot" { val exclude = Excludes(listOf(PathExclude(".git/", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, _) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -80,9 +76,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory containing subdirectories with a dot" { val exclude = Excludes(listOf(PathExclude("src/example.test/templates/", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -95,9 +90,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory containing a dash" { val exclude = Excludes(listOf(PathExclude("test-prod/", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, _) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -108,9 +102,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory containing subdirectories with a dash" { val exclude = Excludes(listOf(PathExclude("src/test-prod/templates/", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -123,9 +116,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory" { val exclude = Excludes(listOf(PathExclude("directory/", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -138,9 +130,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with directory containing subdirectories" { val exclude = Excludes(listOf(PathExclude("directory/sub1/sub2/", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -153,9 +144,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with file extensions" { val exclude = Excludes(listOf(PathExclude("*.pdf", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -168,9 +158,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with file" { val exclude = Excludes(listOf(PathExclude("file.txt", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -183,9 +172,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with files (with '.' in their names)" { val exclude = Excludes(listOf(PathExclude("file.old.txt", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -198,9 +186,8 @@ class MapIgnoreRulesTest : WordSpec({ "map rule with files (with '-' in their names)" { val exclude = Excludes(listOf(PathExclude("package-lock.json", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules shouldHaveSize 1 ignoreRules.first() shouldNotBeNull { @@ -213,9 +200,8 @@ class MapIgnoreRulesTest : WordSpec({ "add an issue when the pattern cannot be mapped" { val exclude = Excludes(listOf(PathExclude("directory/**/test/*", PathExcludeReason.OTHER))) - val issues = mutableListOf() - val ignoreRules = convertRules(exclude, issues) + val (ignoreRules, issues) = convertRules(exclude) ignoreRules should beEmpty()