Skip to content

Commit

Permalink
feat(model)!: Group snippets by source file matching lines
Browse files Browse the repository at this point in the history
All snippets matching the same source file location, line ranges included,
are now grouped in the snippet model.
This commit contains the model change and the support in the FossID
scanner. ScanOSS support has not been added but the current implementation
is still functional.

Signed-off-by: Nicolas Nobelis <[email protected]>
  • Loading branch information
nnobelis authored and mnonnenmacher committed Sep 21, 2023
1 parent 2ca66d5 commit 641f520
Show file tree
Hide file tree
Showing 10 changed files with 222 additions and 55 deletions.
8 changes: 4 additions & 4 deletions model/src/main/kotlin/SnippetFinding.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
package org.ossreviewtoolkit.model

/**
* A class representing a snippet finding for a source file. A snippet finding is a code snippet from another origin,
* matching the code being scanned.
* A class representing the snippet findings for a source file location. A snippet finding is a code snippet from
* another origin, matching the code being scanned.
* It is meant to be reviewed by an operator as it could be a false positive.
*/
data class SnippetFinding(
Expand All @@ -31,7 +31,7 @@ data class SnippetFinding(
val sourceLocation: TextLocation,

/**
* The corresponding snippet.
* The corresponding snippets.
*/
val snippet: Snippet
val snippets: Set<Snippet>
)
3 changes: 1 addition & 2 deletions model/src/main/kotlin/utils/SortedSetConverters.kt
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ class ScopeSortedSetConverter : StdConverter<Set<Scope>, SortedSet<Scope>>() {
}

class SnippetFindingSortedSetConverter : StdConverter<Set<SnippetFinding>, SortedSet<SnippetFinding>>() {
override fun convert(value: Set<SnippetFinding>) =
value.toSortedSet(compareBy<SnippetFinding> { it.sourceLocation }.thenByDescending { it.snippet.purl })
override fun convert(value: Set<SnippetFinding>) = value.toSortedSet(compareBy { it.sourceLocation })
}

private fun Provenance.getSortKey(): String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class FreemarkerTemplateProcessor(
*/
@Suppress("UNUSED") // This function is used in the templates.
fun collectLicenses(snippetFindings: Collection<SnippetFinding>): Set<String> =
snippetFindings.mapTo(mutableSetOf()) { it.snippet.licenses.toString() }
snippetFindings.flatMapTo(mutableSetOf()) { it.snippets.map { snippet -> snippet.licenses.toString() } }

/**
* Return a flag indicating that issues have been encountered during the run of an advisor with the given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ internal fun mapSnippetFindings(rawResults: RawResults, issues: MutableList<Issu
}
}

findings.flatMap { finding -> finding.value.map { SnippetFinding(finding.key, it) } }
findings.map { SnippetFinding(it.key, it.value) }
}.toSet()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class FossIdLicenseMappingTest : WordSpec({
issues should beEmpty()
findings should haveSize(1)
findings.first() shouldNotBeNull {
snippet.licenses.toString() shouldBe "Apache-2.0"
snippets.first() shouldNotBeNull {
licenses.toString() shouldBe "Apache-2.0"
}
}
}

Expand All @@ -80,7 +82,9 @@ class FossIdLicenseMappingTest : WordSpec({
issues should beEmpty()
findings should haveSize(1)
findings.first() shouldNotBeNull {
snippet.licenses.toString() shouldBe "Apache-2.0"
snippets.first() shouldNotBeNull {
licenses.toString() shouldBe "Apache-2.0"
}
}
}

Expand All @@ -98,7 +102,9 @@ class FossIdLicenseMappingTest : WordSpec({
}
findings should haveSize(1)
findings.first() shouldNotBeNull {
snippet.licenses shouldBe SpdxConstants.NOASSERTION.toSpdx()
snippets.first() shouldNotBeNull {
licenses shouldBe SpdxConstants.NOASSERTION.toSpdx()
}
}
}
}
Expand Down
136 changes: 136 additions & 0 deletions plugins/scanners/fossid/src/test/kotlin/FossIdSnippetMappingTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright (C) 2023 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/

package org.ossreviewtoolkit.plugins.scanners.fossid

import io.kotest.core.spec.style.WordSpec
import io.kotest.matchers.collections.beEmpty
import io.kotest.matchers.collections.containExactly
import io.kotest.matchers.collections.shouldHaveSize
import io.kotest.matchers.should
import io.kotest.matchers.shouldBe

import org.ossreviewtoolkit.clients.fossid.PolymorphicList
import org.ossreviewtoolkit.clients.fossid.model.result.MatchType
import org.ossreviewtoolkit.clients.fossid.model.result.MatchedLines
import org.ossreviewtoolkit.clients.fossid.model.result.Snippet
import org.ossreviewtoolkit.model.Issue
import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.utils.test.shouldNotBeNull

class FossIdSnippetMappingTest : WordSpec({
"mapSnippetFindings" should {
"group snippets by source file location" {
val issues = mutableListOf<Issue>()
val listSnippets = mapOf(
"src/main/java/Tokenizer.java" to setOf(
createSnippet(
1,
MatchType.FULL,
"pkg:github/vdurmont/[email protected]",
"MIT",
"src/main/java/com/vdurmont/semver4j/Tokenizer.java"
),
createSnippet(
2,
MatchType.FULL,
"pkg:maven/com.vdurmont/[email protected]",
"MIT",
"com/vdurmont/semver4j/Tokenizer.java"
)
),
"src/main/java/com/vdurmont/semver4j/Requirement.java" to setOf(
createSnippet(
3,
MatchType.PARTIAL,
"pkg:github/vdurmont/[email protected]",
"MIT",
"com/vdurmont/semver4j/Requirement.java"
)
)
)
val localFile = ((1..24) + (45..675)).toPolymorphicList()
val remoteFile = (1..655).toPolymorphicList()
// There is no matched line information for full match snippets.
val snippetMatchedLines = mapOf(3 to MatchedLines(localFile, remoteFile))
val rawResults = RawResults(
emptyList(),
emptyList(),
emptyList(),
emptyList(),
listSnippets,
snippetMatchedLines
)

val mappedSnippets = mapSnippetFindings(rawResults, issues)

issues should beEmpty()
mappedSnippets shouldHaveSize 3
mappedSnippets.first().apply {
sourceLocation shouldBe TextLocation("src/main/java/Tokenizer.java", TextLocation.UNKNOWN_LINE)
snippets shouldHaveSize 2
snippets.map { it.purl } should containExactly(
"pkg:github/vdurmont/[email protected]",
"pkg:maven/com.vdurmont/[email protected]"
)
}
mappedSnippets.elementAtOrNull(1) shouldNotBeNull {
sourceLocation shouldBe TextLocation("src/main/java/com/vdurmont/semver4j/Requirement.java", 1, 24)
snippets.map { it.purl } should containExactly("pkg:github/vdurmont/[email protected]")
}
mappedSnippets.elementAtOrNull(2) shouldNotBeNull {
sourceLocation shouldBe TextLocation("src/main/java/com/vdurmont/semver4j/Requirement.java", 45, 675)
snippets.map { it.purl } should containExactly("pkg:github/vdurmont/[email protected]")
}
}
}
})

private fun createSnippet(id: Int, matchType: MatchType, purl: String, license: String, file: String) =
Snippet(
id = id,
created = "",
scanId = 1,
scanFileId = 1,
fileId = 1,
matchType = matchType,
reason = null,
author = null,
artifact = null,
version = null,
purl = purl,
artifactLicense = license,
artifactLicenseCategory = null,
releaseDate = null,
mirror = null,
file = file,
fileLicense = null,
url = "",
hits = null,
size = null,
updated = null,
cpe = null,
score = "1.0",
matchFileId = null,
classification = null,
highlighting = null
)

private fun IntRange.toPolymorphicList() = toList().toPolymorphicList()
private fun List<Int>.toPolymorphicList() = PolymorphicList(this)
57 changes: 39 additions & 18 deletions plugins/scanners/fossid/src/test/kotlin/FossIdTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ import org.ossreviewtoolkit.plugins.scanners.fossid.FossId.Companion.SERVER_URL_
import org.ossreviewtoolkit.plugins.scanners.fossid.FossId.Companion.convertGitUrlToProjectName
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.utils.spdx.SpdxExpression
import org.ossreviewtoolkit.utils.test.shouldNotBeNull

@Suppress("LargeClass")
class FossIdTest : WordSpec({
Expand Down Expand Up @@ -361,7 +360,7 @@ class FossIdTest : WordSpec({
summary.issues.map { it.copy(timestamp = Instant.EPOCH) } shouldBe expectedIssues
}

"report pending files as snippets" {
"report snippets of pending files" {
val projectCode = projectCode(PROJECT)
val scanCode = scanCode(PROJECT, null)
val config = createConfig(deltaScans = false)
Expand All @@ -382,9 +381,9 @@ class FossIdTest : WordSpec({
val summary = fossId.scan(createPackage(pkgId, vcsInfo)).summary

val expectedPendingFile = (1..5).map(::createPendingFile).toSet()
val expectedSnippetFindings = (1..5).map(::createSnippetFindings).flatten()
val expectedSnippetFindings = (1..5).map(::createSnippetFindings)

summary.snippetFindings shouldHaveSize expectedPendingFile.size * 5
summary.snippetFindings shouldHaveSize expectedPendingFile.size
summary.snippetFindings.map { it.sourceLocation.path }.toSet() shouldBe expectedPendingFile
summary.snippetFindings shouldBe expectedSnippetFindings
}
Expand All @@ -409,14 +408,36 @@ class FossIdTest : WordSpec({

val summary = fossId.scan(createPackage(pkgId, vcsInfo)).summary

summary.snippetFindings shouldHaveSize 1
summary.snippetFindings.first() shouldNotBeNull {
sourceLocation.startLine shouldBe(1)
sourceLocation.endLine shouldBe(3)
snippet.location.startLine shouldBe 11
snippet.location.endLine shouldBe 12
snippet.additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SOURCE] shouldBe "1-3, 21-22, 36"
snippet.additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SNIPPET] shouldBe "11-12"
summary.snippetFindings shouldHaveSize 3
summary.snippetFindings.first().apply {
sourceLocation shouldBe TextLocation("/pending/file/1", 1, 3)
snippets shouldHaveSize 1
snippets.first().apply {
location.startLine shouldBe 11
location.endLine shouldBe 12
additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SOURCE] shouldBe "1-3, 21-22, 36"
additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SNIPPET] shouldBe "11-12"
}
}
summary.snippetFindings.elementAt(1).apply {
sourceLocation shouldBe TextLocation("/pending/file/1", 21, 22)
snippets shouldHaveSize 1
snippets.first().apply {
location.startLine shouldBe 11
location.endLine shouldBe 12
additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SOURCE] shouldBe "1-3, 21-22, 36"
additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SNIPPET] shouldBe "11-12"
}
}
summary.snippetFindings.last().apply {
sourceLocation shouldBe TextLocation("/pending/file/1", 36)
snippets shouldHaveSize 1
snippets.first().apply {
location.startLine shouldBe 11
location.endLine shouldBe 12
additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SOURCE] shouldBe "1-3, 21-22, 36"
additionalData[FossId.SNIPPET_DATA_MATCHED_LINE_SNIPPET] shouldBe "11-12"
}
}
}

Expand Down Expand Up @@ -1410,10 +1431,10 @@ private fun createSnippet(index: Int): Snippet =
/**
* Generate a ORT snippet finding based on the given [index].
*/
private fun createSnippetFindings(index: Int): Set<SnippetFinding> =
(1..5).map { snippetIndex ->
SnippetFinding(
TextLocation("/pending/file/$index", TextLocation.UNKNOWN_LINE),
private fun createSnippetFindings(index: Int): SnippetFinding =
SnippetFinding(
TextLocation("/pending/file/$index", TextLocation.UNKNOWN_LINE),
(1..5).map { snippetIndex ->
OrtSnippet(
snippetIndex.toFloat(),
TextLocation("file$snippetIndex", TextLocation.UNKNOWN_LINE),
Expand All @@ -1427,8 +1448,8 @@ private fun createSnippetFindings(index: Int): Set<SnippetFinding> =
FossId.SNIPPET_DATA_MATCH_TYPE to MatchType.PARTIAL.toString()
)
)
)
}.toSet()
}.toSet()
)

/**
* Prepare this service mock to answer a request for a project with the given [projectCode]. Return a response with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ internal fun generateSummary(startTime: Instant, endTime: Instant, result: FullS
val snippets = getSnippets(scanResponse)

snippets.forEach {
snippetFindings += SnippetFinding(sourceLocation, it)
// TODO: Aggregate the snippet by source file location.
snippetFindings += SnippetFinding(sourceLocation, setOf(it))
}
}
}
Expand Down
28 changes: 15 additions & 13 deletions plugins/scanners/scanoss/src/test/kotlin/ScanOssResultParserTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,21 @@ class ScanOssResultParserTest : WordSpec({
summary.snippetFindings.shouldContainExactly(
SnippetFinding(
TextLocation("src/main/java/com/vdurmont/semver4j/Requirement.java", 1, 710),
Snippet(
98.0f,
TextLocation(
"https://osskb.org/api/file_contents/6ff2427335b985212c9b79dfa795799f",
1,
710
),
RepositoryProvenance(
VcsInfo(VcsType.UNKNOWN, "https://github.com/vdurmont/semver4j", ""),
"."
),
"pkg:github/vdurmont/semver4j",
SpdxExpression.parse("CC-BY-SA-2.0")
setOf(
Snippet(
98.0f,
TextLocation(
"https://osskb.org/api/file_contents/6ff2427335b985212c9b79dfa795799f",
1,
710
),
RepositoryProvenance(
VcsInfo(VcsType.UNKNOWN, "https://github.com/vdurmont/semver4j", ""),
"."
),
"pkg:github/vdurmont/semver4j",
SpdxExpression.parse("CC-BY-SA-2.0")
)
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,20 @@ class ScanOssScannerDirectoryTest : StringSpec({
snippetFindings.shouldContainExactly(
SnippetFinding(
TextLocation("utils/src/main/kotlin/ArchiveUtils.kt", 1, 240),
Snippet(
99.0f,
TextLocation(
"https://osskb.org/api/file_contents/871fb0c5188c2f620d9b997e225b0095",
128,
367
),
RepositoryProvenance(
VcsInfo(VcsType.UNKNOWN, "https://github.com/scanoss/ort", ""), "."
),
"pkg:github/scanoss/ort",
SpdxExpression.parse("Apache-2.0")
setOf(
Snippet(
99.0f,
TextLocation(
"https://osskb.org/api/file_contents/871fb0c5188c2f620d9b997e225b0095",
128,
367
),
RepositoryProvenance(
VcsInfo(VcsType.UNKNOWN, "https://github.com/scanoss/ort", ""), "."
),
"pkg:github/scanoss/ort",
SpdxExpression.parse("Apache-2.0")
)
)
)
)
Expand Down

0 comments on commit 641f520

Please sign in to comment.