refactor(scanoss): ScanOssResultParser to improve snippets findings

* Generate one Snippet for each detected line range
* Remove duplicate licenses to optimize results
* Remove identified snippets from the summary

Signed-off-by: Agustin Isasmendi <[email protected]>

Author: isasmendiagus

plugins/scanners/scanoss/src/main/kotlin/ScanOssResultParser.kt

@@ -18,13 +18,17 @@
  */
 
 package org.ossreviewtoolkit.plugins.scanners.scanoss
-
+import com.scanoss.dto.LicenseDetails
 import com.scanoss.dto.ScanFileDetails
 import com.scanoss.dto.ScanFileResult
 import com.scanoss.dto.enums.MatchType
+import com.scanoss.dto.enums.StatusType
 
+import java.lang.invoke.MethodHandles
 import java.time.Instant
 
+import org.apache.logging.log4j.kotlin.loggerOf
+
 import org.ossreviewtoolkit.downloader.VcsHost
 import org.ossreviewtoolkit.model.CopyrightFinding
 import org.ossreviewtoolkit.model.LicenseFinding
@@ -36,7 +40,8 @@ import org.ossreviewtoolkit.model.TextLocation
 import org.ossreviewtoolkit.utils.spdx.SpdxConstants
 import org.ossreviewtoolkit.utils.spdx.SpdxExpression
 import org.ossreviewtoolkit.utils.spdx.SpdxLicenseIdExpression
-import org.ossreviewtoolkit.utils.spdx.toExpression
+
+private val logger = loggerOf(MethodHandles.lookup().lookupClass())
 
 /**
  * Generate a summary from the given SCANOSS [result], using [startTime], [endTime] as metadata. This variant can be
@@ -56,16 +61,29 @@ internal fun generateSummary(startTime: Instant, endTime: Instant, results: List
                 }
 
                 MatchType.snippet -> {
-                    val file = requireNotNull(details.file)
-                    val lines = requireNotNull(details.lines)
-                    val sourceLocations = convertLines(file, lines)
-                    val snippets = getSnippets(details)
-
-                    snippets.forEach { snippet ->
-                        sourceLocations.forEach { sourceLocation ->
-                            // TODO: Aggregate the snippet by source file location.
-                            snippetFindings += SnippetFinding(sourceLocation, setOf(snippet))
+                    val file = requireNotNull(result.filePath)
+                    if (details.status == StatusType.pending) {
+                        val lines = requireNotNull(details.lines)
+                        val sourceLocations = convertLines(file, lines)
+                        val snippets = getSnippets(details)
+
+                        // The number of snippets should match the number of source locations.
+                        if (sourceLocations.size != snippets.size) {
+                            logger.warn {
+                                "Unexpected mismatch in '$file': " +
+                                    "${sourceLocations.size} source locations vs ${snippets.size} snippets. " +
+                                    "This indicates a potential issue with line range conversion."
+                            }
                         }
+
+                        // Associate each source location with its corresponding snippet.
+                        sourceLocations.zip(snippets).forEach { (location, snippet) ->
+                            snippetFindings += SnippetFinding(location, setOf(snippet))
+                        }
+                    } else {
+                        logger.warn { "File '$file' is identified, not including on snippet findings" }
+                        licenseFindings += getLicenseFindings(details)
+                        copyrightFindings += getCopyrightFindings(details)
                     }
                 }
 
@@ -134,36 +152,34 @@ private fun getCopyrightFindings(details: ScanFileDetails): List<CopyrightFindin
 }
 
 /**
- * Get the snippet findings from the given [details]. If a snippet returned by ScanOSS contains several Purls,
- * several snippets are created in ORT each containing a single Purl.
+ * Get the snippet findings from the given [details]. If a snippet returned by SCANOSS contains several Purls,
+ * the function uses the first PURL as the primary identifier while storing all PURLs in additionalData
+ * to preserve the complete information.
  */
-private fun getSnippets(details: ScanFileDetails): Set<Snippet> {
+private fun getSnippets(details: ScanFileDetails): List<Snippet> {
     val matched = requireNotNull(details.matched)
     val fileUrl = requireNotNull(details.fileUrl)
     val ossLines = requireNotNull(details.ossLines)
     val url = requireNotNull(details.url)
     val purls = requireNotNull(details.purls)
 
-    val licenses = details.licenseDetails.orEmpty().mapTo(mutableSetOf()) { license ->
-        SpdxExpression.parse(license.name)
-    }
+    val license = getUniqueLicenseExpression(details.licenseDetails.toList())
 
     val score = matched.substringBeforeLast("%").toFloat()
     val locations = convertLines(fileUrl, ossLines)
     // TODO: No resolved revision is available. Should a ArtifactProvenance be created instead ?
     val vcsInfo = VcsHost.parseUrl(url.takeUnless { it == "none" }.orEmpty())
     val provenance = RepositoryProvenance(vcsInfo, ".")
 
-    val additionalData = mapOf("release_date" to details.releaseDate)
-
-    return buildSet {
-        purls.forEach { purl ->
-            locations.forEach { snippetLocation ->
-                val license = licenses.toExpression()?.sorted() ?: SpdxLicenseIdExpression(SpdxConstants.NOASSERTION)
+    // Store all PURLs in additionalData to preserve the complete information.
+    val additionalData = mapOf(
+        "release_date" to details.releaseDate,
+        "all_purls" to purls.joinToString(" ")
+    )
 
-                add(Snippet(score, snippetLocation, provenance, purl, license, additionalData))
-            }
-        }
+    // Create one snippet per location, using the first PURL as the primary identifier.
+    return locations.map { snippetLocation ->
+        Snippet(score, snippetLocation, provenance, purls.firstOrNull().orEmpty(), license, additionalData)
     }
 }
 
@@ -180,3 +196,14 @@ private fun convertLines(file: String, lineRanges: String): List<TextLocation> =
             else -> throw IllegalArgumentException("Unsupported line range '$lineRange'.")
         }
     }
+
+fun getUniqueLicenseExpression(licensesDetails: List<LicenseDetails>): SpdxExpression {
+    if (licensesDetails.isEmpty()) {
+        return SpdxLicenseIdExpression(SpdxConstants.NOASSERTION)
+    }
+
+    return licensesDetails
+        .map { license -> SpdxExpression.parse(license.name) }
+        .reduce { acc, expr -> acc and expr }
+        .simplify()
+}

plugins/scanners/scanoss/src/test/assets/scanoss-multiple-purls.json

@@ -0,0 +1,60 @@
+{
+  "hung_task.c": [
+    {
+      "component": "proton_bluecross",
+      "file": "kernel/hung_task.c",
+      "file_hash": "581734935cfbe570d280a1265aaa2a6b",
+      "file_url": "https://api.scanoss.com/file_contents/581734935cfbe570d280a1265aaa2a6b",
+      "id": "snippet",
+      "latest": "17",
+      "licenses": [
+        {
+          "checklist_url": "https://www.osadl.org/fileadmin/checklists/unreflicenses/GPL-2.0-only.txt",
+          "copyleft": "yes",
+          "incompatible_with": "Apache-1.0, Apache-1.1, Apache-2.0, BSD-4-Clause, BSD-4-Clause-UC, BSD-4.3TAHOE, ECL-2.0, FTL, IJG, LicenseRef-scancode-bsla-no-advert, Minpack, OpenSSL, PHP-3.01, Python-2.0, zlib-acknowledgement, XFree86-1.1",
+          "name": "GPL-2.0-only",
+          "osadl_updated": "2025-02-10T14:26:00+0000",
+          "patent_hints": "yes",
+          "source": "scancode",
+          "url": "https://spdx.org/licenses/GPL-2.0-only.html"
+        },
+        {
+          "name": "GPL-2.0-only WITH Linux-syscall-note",
+          "source": "scancode",
+          "url": "https://spdx.org/licenses/GPL-2.0-only WITH Linux-syscall-note.html"
+        },
+        {
+          "checklist_url": "https://www.osadl.org/fileadmin/checklists/unreflicenses/GPL-2.0-only.txt",
+          "copyleft": "yes",
+          "incompatible_with": "Apache-1.0, Apache-1.1, Apache-2.0, BSD-4-Clause, BSD-4-Clause-UC, BSD-4.3TAHOE, ECL-2.0, FTL, IJG, LicenseRef-scancode-bsla-no-advert, Minpack, OpenSSL, PHP-3.01, Python-2.0, zlib-acknowledgement, XFree86-1.1",
+          "name": "GPL-2.0-only",
+          "osadl_updated": "2025-02-10T14:26:00+0000",
+          "patent_hints": "yes",
+          "source": "scancode",
+          "url": "https://spdx.org/licenses/GPL-2.0-only.html"
+        }
+      ],
+      "lines": "12-150,540-561",
+      "matched": "35%",
+      "oss_lines": "10-148,86-107",
+      "purl": [
+        "pkg:github/kdrag0n/proton_bluecross",
+        "pkg:github/fake/fake_repository"
+      ],
+      "release_date": "2019-02-21",
+      "server": {
+        "kb_version": {
+          "daily": "25.03.27",
+          "monthly": "25.03"
+        },
+        "version": "5.4.10"
+      },
+      "source_hash": "45dd1e50621a8a32f88fbe0251a470ab",
+      "status": "pending",
+      "url": "https://github.com/kdrag0n/proton_bluecross",
+      "url_hash": "a9c1c67f0930dc42dbd40c29e565bcdd",
+      "vendor": "kdrag0n",
+      "version": "15"
+    }
+  ]
+}

plugins/scanners/scanoss/src/test/kotlin/ScanOssResultParserTest.kt

@@ -19,6 +19,7 @@
 
 package org.ossreviewtoolkit.plugins.scanners.scanoss
 
+import com.scanoss.dto.LicenseDetails
 import com.scanoss.utils.JsonUtils
 
 import io.kotest.core.spec.style.WordSpec
@@ -27,6 +28,7 @@ import io.kotest.matchers.collections.containExactlyInAnyOrder
 import io.kotest.matchers.collections.haveSize
 import io.kotest.matchers.collections.shouldContain
 import io.kotest.matchers.should
+import io.kotest.matchers.shouldBe
 
 import java.io.File
 import java.time.Instant
@@ -39,9 +41,44 @@ import org.ossreviewtoolkit.model.SnippetFinding
 import org.ossreviewtoolkit.model.TextLocation
 import org.ossreviewtoolkit.model.VcsInfo
 import org.ossreviewtoolkit.model.VcsType
+import org.ossreviewtoolkit.utils.spdx.SpdxConstants
 import org.ossreviewtoolkit.utils.spdx.SpdxExpression
+import org.ossreviewtoolkit.utils.spdx.SpdxLicenseIdExpression
 
 class ScanOssResultParserTest : WordSpec({
+    "getUniqueLicenseDetails()" should {
+        "deduplicate complex license expressions" {
+            val uniqueLicenses = getUniqueLicenseExpression(
+                listOf(
+                    LicenseDetails.builder().name("MIT").build(),
+                    LicenseDetails.builder().name("MIT").build(),
+                    LicenseDetails.builder().name("GPL-2.0-only").build(),
+                    LicenseDetails.builder().name("GPL-2.0-only WITH Linux-syscall-note").build(),
+                    LicenseDetails.builder().name("GPL-2.0-only AND MIT").build()
+                )
+            )
+
+            val decomposed = uniqueLicenses.decompose().toList()
+
+            val expressionStrings = decomposed.map { it.toString() }
+
+            // Check that each license appears exactly once
+            expressionStrings.count { it == "MIT" } shouldBe 1
+            expressionStrings.count { it == "GPL-2.0-only" } shouldBe 1
+            expressionStrings.count { it == "GPL-2.0-only WITH Linux-syscall-note" } shouldBe 1
+
+            // Ensure no unexpected elements
+            expressionStrings.size shouldBe 3
+        }
+
+        "handle empty license list" {
+            val emptyLicenses = getUniqueLicenseExpression(listOf())
+
+            // Verify empty license list returns NOASSERTION
+            emptyLicenses shouldBe SpdxLicenseIdExpression(SpdxConstants.NOASSERTION)
+        }
+    }
+
     "generateSummary()" should {
         "properly summarize JUnit 4.12 findings" {
             val results = File("src/test/assets/scanoss-junit-4.12.json").readText().let {
@@ -126,11 +163,51 @@ class ScanOssResultParserTest : WordSpec({
                             ),
                             "pkg:github/vdurmont/semver4j",
                             SpdxExpression.parse("CC-BY-SA-2.0"),
-                            additionalData = mapOf("release_date" to "2019-09-13")
+                            additionalData = mapOf(
+                                "release_date" to "2019-09-13",
+                                "all_purls" to "pkg:github/vdurmont/semver4j"
+                            )
                         )
                     )
                 )
             )
         }
+
+        "should handle multiple PURLs by selecting first as primary and preserving all in metadata" {
+            val results = File("src/test/assets/scanoss-multiple-purls.json").readText().let {
+                JsonUtils.toScanFileResultsFromObject(JsonUtils.toJsonObject(it))
+            }
+
+            val time = Instant.now()
+            val summary = generateSummary(time, time, results)
+
+            // Should have one finding per source location, not per PURL.
+            summary.snippetFindings should haveSize(2)
+
+            with(summary.snippetFindings.first()) {
+                // Check source location (local file).
+                sourceLocation shouldBe TextLocation("hung_task.c", 12, 150)
+
+                // Should use first PURL as primary identifier.
+                snippets should haveSize(1)
+                snippets.first().purl shouldBe "pkg:github/kdrag0n/proton_bluecross"
+
+                // Should preserve all PURLs in additionalData.
+                snippets.first().additionalData["all_purls"] shouldBe
+                    "pkg:github/kdrag0n/proton_bluecross pkg:github/fake/fake_repository"
+
+                // Check OSS location.
+                snippets.first().location shouldBe
+                    TextLocation("https://api.scanoss.com/file_contents/581734935cfbe570d280a1265aaa2a6b", 10, 148)
+            }
+
+            // Verify same behavior for second snippet.
+            with(summary.snippetFindings.last()) {
+                sourceLocation shouldBe TextLocation("hung_task.c", 540, 561)
+                snippets.first().purl shouldBe "pkg:github/kdrag0n/proton_bluecross"
+                snippets.first().location shouldBe
+                    TextLocation("https://api.scanoss.com/file_contents/581734935cfbe570d280a1265aaa2a6b", 86, 107)
+            }
+        }
     }
 })

plugins/scanners/scanoss/src/test/kotlin/ScanOssScannerDirectoryTest.kt

@@ -112,7 +112,11 @@ class ScanOssScannerDirectoryTest : StringSpec({
                             ),
                             "pkg:github/scanoss/ort",
                             SpdxExpression.parse("Apache-2.0"),
-                            additionalData = mapOf("release_date" to "2021-03-18")
+                            additionalData = mapOf(
+                                "release_date" to "2021-03-18",
+                                "all_purls" to "pkg:github/scanoss/ort"
+                            )
+
                         )
                     )
                 )