analyzer: Stop using a SortedSet in ProjectAnalyzerResult

The `ProjectAnalyzerResult` is only serialized as part of analyzer
functional tests when comparing to expected results. So in memory, there
is no need for the `packages` to be a `SortedSet`. Change the code to
only sort `packages` on serialization via a Jackson converter to have a
defined order when comparing to expected results.

This lays the fundation for getting rid of `Comparable` implementations
in various classes. These implementations were only added for sorted
output during serialization, but they break the `Set` contract as
`compareTo() == 0` is not aligned with `equals() == true`, also see [1].

[1]: https://docs.oracle.com/javase/8/docs/api/java/util/SortedSet.html

Signed-off-by: Sebastian Schuberth <[email protected]>

Author: sschuberth

analyzer/src/funTest/kotlin/managers/Extensions.kt

@@ -83,7 +83,7 @@ fun PackageManagerResult.resolveScopes(projectResult: ProjectAnalyzerResult): Pr
     // all packages; this handles corner cases with package managers producing packages not assigned to project scopes.
     val packages = projectResult.packages.takeUnless { it.isEmpty() }
         ?: if (projectResults.size > 1) resolvedProject.filterReferencedPackages(sharedPackages) else sharedPackages
-    return projectResult.copy(project = resolvedProject, packages = packages.toSortedSet())
+    return projectResult.copy(project = resolvedProject, packages = packages)
 }
 
 /**

analyzer/src/funTest/kotlin/managers/SpdxDocumentFileFunTest.kt

@@ -116,7 +116,7 @@ class SpdxDocumentFileFunTest : WordSpec({
                         Scope("default")
                     )
                 ),
-                sortedSetOf()
+                emptySet()
             )
 
             actualResult[opensslId] shouldBe ProjectAnalyzerResult(
@@ -137,7 +137,7 @@ class SpdxDocumentFileFunTest : WordSpec({
                         Scope("default")
                     )
                 ),
-                sortedSetOf()
+                emptySet()
             )
 
             actualResult[zlibId] shouldBe ProjectAnalyzerResult(
@@ -158,7 +158,7 @@ class SpdxDocumentFileFunTest : WordSpec({
                         Scope("default")
                     )
                 ),
-                sortedSetOf()
+                emptySet()
             )
         }
 

analyzer/src/main/kotlin/PackageManager.kt

@@ -292,7 +292,7 @@ abstract class PackageManager(
                         )
                     )
 
-                    result[definitionFile] = listOf(ProjectAnalyzerResult(projectWithIssues, sortedSetOf(), issues))
+                    result[definitionFile] = listOf(ProjectAnalyzerResult(projectWithIssues, emptySet(), issues))
                 }
             }
 
@@ -333,7 +333,7 @@ abstract class PackageManager(
             entry.value.map { projectResult ->
                 val projectReferences = projectResult.packages.filterTo(mutableSetOf()) { it.id in projectIds }
                 projectResult.takeIf { projectReferences.isEmpty() }
-                    ?: projectResult.copy(packages = (projectResult.packages - projectReferences).toSortedSet())
+                    ?: projectResult.copy(packages = projectResult.packages - projectReferences)
                         .also {
                             logger.info { "Removing ${projectReferences.size} packages that are projects." }
 

analyzer/src/main/kotlin/PackageManagerResult.kt

@@ -48,5 +48,5 @@ data class PackageManagerResult(
      * produce a shared [DependencyGraph] typically do not collect packages on a project-level, but globally. Such
      * packages can be stored in this property.
      */
-    val sharedPackages: Set<Package> = sortedSetOf()
+    val sharedPackages: Set<Package> = emptySet()
 )

analyzer/src/main/kotlin/managers/Bower.kt

@@ -258,7 +258,7 @@ class Bower(
                 scopeDependencies = sortedSetOf(dependenciesScope, devDependenciesScope)
             )
 
-            return listOf(ProjectAnalyzerResult(project, packages.values.toSortedSet()))
+            return listOf(ProjectAnalyzerResult(project, packages.values.toSet()))
         }
     }
 

analyzer/src/main/kotlin/managers/Bundler.kt

@@ -205,7 +205,7 @@ class Bundler(
             val allProjectDeps = groupedDeps.values.flatten().toSet()
             val hasBundlerDep = BUNDLER_GEM_NAME in allProjectDeps
 
-            val packages = gemSpecs.values.mapNotNullTo(sortedSetOf()) { gemSpec ->
+            val packages = gemSpecs.values.mapNotNullTo(mutableSetOf()) { gemSpec ->
                 getPackageFromGemspec(gemSpec).takeUnless { gemSpec.name == BUNDLER_GEM_NAME && !hasBundlerDep }
             }
 

analyzer/src/main/kotlin/managers/Cargo.kt

@@ -237,7 +237,7 @@ class Cargo(
 
         val nonProjectPackages = packages
             .filterNot { isProjectDependency(it.key) }
-            .mapTo(sortedSetOf()) { it.value }
+            .mapTo(mutableSetOf()) { it.value }
 
         return listOf(ProjectAnalyzerResult(project, nonProjectPackages))
     }

analyzer/src/main/kotlin/managers/Carthage.kt

@@ -23,7 +23,6 @@ import com.fasterxml.jackson.module.kotlin.readValue
 
 import java.io.File
 import java.net.URL
-import java.util.SortedSet
 
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManager
@@ -111,10 +110,10 @@ class Carthage(
         )
     }
 
-    private fun parseCarthageDependencies(definitionFile: File): SortedSet<Package> {
+    private fun parseCarthageDependencies(definitionFile: File): Set<Package> {
         val dependencyLines = definitionFile.readLines()
         val workingDir = definitionFile.parent
-        val packages = sortedSetOf<Package>()
+        val packages = mutableSetOf<Package>()
 
         dependencyLines.forEach { line ->
             if (line.isBlank() || line.isComment()) return@forEach

analyzer/src/main/kotlin/managers/CocoaPods.kt

@@ -132,7 +132,7 @@ class CocoaPods(
         val lockfile = workingDir.resolve(LOCKFILE_FILENAME)
 
         val scopes = sortedSetOf<Scope>()
-        val packages = sortedSetOf<Package>()
+        val packages = mutableSetOf<Package>()
         val issues = mutableListOf<OrtIssue>()
 
         if (lockfile.isFile) {

analyzer/src/main/kotlin/managers/Composer.kt

@@ -161,7 +161,7 @@ class Composer(
 
             val project = parseProject(definitionFile, scopes)
 
-            return listOf(ProjectAnalyzerResult(project, packages.values.toSortedSet()))
+            return listOf(ProjectAnalyzerResult(project, packages.values.toSet()))
         }
     }
 

analyzer/src/main/kotlin/managers/Conan.kt

@@ -179,7 +179,7 @@ class Conan(
                         homepageUrl = projectPackage.homepageUrl,
                         scopeDependencies = sortedSetOf(dependenciesScope, devDependenciesScope)
                     ),
-                    packages = packages.values.toSortedSet()
+                    packages = packages.values.toSet()
                 )
             )
         }

analyzer/src/main/kotlin/managers/GoDep.kt

@@ -109,7 +109,7 @@ class GoDep(
         }
 
         val projects = parseProjects(workingDir, gopath)
-        val packages = sortedSetOf<Package>()
+        val packages = mutableSetOf<Package>()
         val packageRefs = mutableListOf<PackageReference>()
 
         for (project in projects) {

analyzer/src/main/kotlin/managers/GoMod.kt

@@ -105,7 +105,7 @@ class GoMod(
             val graph = getModuleGraph(projectDir, moduleInfoForModuleName)
             val projectId = graph.projectId()
             val packageIds = graph.nodes() - projectId
-            val packages = packageIds.mapTo(sortedSetOf()) { moduleInfoForModuleName.getValue(it.name).toPackage() }
+            val packages = packageIds.mapTo(mutableSetOf()) { moduleInfoForModuleName.getValue(it.name).toPackage() }
             val projectVcs = processProjectVcs(projectDir)
 
             val dependenciesScopePackageIds = getTransitiveMainModuleDependencies(projectDir).let { moduleNames ->

analyzer/src/main/kotlin/managers/Gradle.kt

@@ -276,7 +276,7 @@ class Gradle(
                     createAndLogIssue(source = managerName, message = it, severity = Severity.WARNING)
                 }
 
-                listOf(ProjectAnalyzerResult(project, sortedSetOf(), issues))
+                listOf(ProjectAnalyzerResult(project, emptySet(), issues))
             }
         }
     }

analyzer/src/main/kotlin/managers/Maven.kt

@@ -164,7 +164,7 @@ class Maven(
             }
         }
 
-        return listOf(ProjectAnalyzerResult(project, sortedSetOf(), issues))
+        return listOf(ProjectAnalyzerResult(project, emptySet(), issues))
     }
 }
 

analyzer/src/main/kotlin/managers/Npm.kt

@@ -209,7 +209,7 @@ open class Npm(
 
         // TODO: add support for peerDependencies and bundledDependencies.
 
-        return listOf(ProjectAnalyzerResult(project.copy(scopeNames = scopeNames.toSortedSet()), sortedSetOf()))
+        return listOf(ProjectAnalyzerResult(project.copy(scopeNames = scopeNames.toSortedSet()), emptySet()))
     }
 
     private fun parseInstalledModules(rootDirectory: File): Map<String, Package> {

analyzer/src/main/kotlin/managers/Pub.kt

@@ -262,7 +262,7 @@ class Pub(
 
         val project = parseProject(definitionFile, manifest, scopes)
 
-        projectAnalyzerResults += ProjectAnalyzerResult(project, packages.values.toSortedSet(), issues)
+        projectAnalyzerResults += ProjectAnalyzerResult(project, packages.values.toSet(), issues)
 
         return projectAnalyzerResults
     }
@@ -395,7 +395,7 @@ class Pub(
                 .resolveDependencies(listOf(packageFile), labels).run {
                     projectResults.getValue(packageFile).map { result ->
                         val project = result.project.withResolvedScopes(dependencyGraph)
-                        result.copy(project = project, packages = sharedPackages.toSortedSet())
+                        result.copy(project = project, packages = sharedPackages)
                     }
                 }
         }
@@ -422,7 +422,7 @@ class Pub(
                     "implemented."
         )
 
-        return ProjectAnalyzerResult(Project.EMPTY, sortedSetOf(), listOf(issue))
+        return ProjectAnalyzerResult(Project.EMPTY, emptySet(), listOf(issue))
     }
 
     private fun parseProject(definitionFile: File, pubspec: JsonNode, scopes: SortedSet<Scope>): Project {

analyzer/src/main/kotlin/managers/SpdxDocumentFile.kt

@@ -523,7 +523,7 @@ class SpdxDocumentFile(
             scopeDependencies = scopes
         )
 
-        return listOf(ProjectAnalyzerResult(project, packages.toSortedSet()))
+        return listOf(ProjectAnalyzerResult(project, packages))
     }
 
     /**

analyzer/src/main/kotlin/managers/Stack.kt

@@ -221,7 +221,7 @@ class Stack(
             scopeDependencies = scopes
         )
 
-        return listOf(ProjectAnalyzerResult(project, dependencyPackageMap.values.toSortedSet()))
+        return listOf(ProjectAnalyzerResult(project, dependencyPackageMap.values.toSet()))
     }
 
     private fun getPackageUrl(name: String, version: String) =

analyzer/src/main/kotlin/managers/Unmanaged.kt

@@ -113,7 +113,7 @@ class Unmanaged(
         return listOf(
             ProjectAnalyzerResult(
                 project = Project.EMPTY.copy(id = id, vcsProcessed = vcsInfo),
-                packages = sortedSetOf()
+                packages = emptySet()
             )
         )
     }

analyzer/src/main/kotlin/managers/Yarn2.kt

@@ -236,7 +236,7 @@ class Yarn2(
             val allProjects = parseAllPackages(iterator, definitionFile, packagesHeaders, packagedDetails)
             val scopeNames = YarnDependencyType.values().map { it.type }.toSortedSet()
             return allProjects.values.map { project ->
-                ProjectAnalyzerResult(project.copy(scopeNames = scopeNames), sortedSetOf(), issues)
+                ProjectAnalyzerResult(project.copy(scopeNames = scopeNames), emptySet(), issues)
             }.toList()
         }
     }

analyzer/src/main/kotlin/managers/utils/NuGetSupport.kt

@@ -240,7 +240,7 @@ class NuGetSupport(
     fun resolveDependencies(definitionFile: File, directDependenciesOnly: Boolean): ProjectAnalyzerResult {
         val workingDir = definitionFile.parentFile
 
-        val packages = sortedSetOf<Package>()
+        val packages = mutableSetOf<Package>()
 
         val references = reader.getDependencies(definitionFile)
         val referencesByFramework = references.groupBy { it.targetFramework }

analyzer/src/main/kotlin/managers/utils/PythonInspector.kt

@@ -286,8 +286,8 @@ private fun processDeclaredLicenses(id: Identifier, declaredLicenses: SortedSet<
     return declaredLicensesProcessed
 }
 
-internal fun List<PythonInspector.Package>.toOrtPackages(): SortedSet<Package> =
-    groupBy { "${it.name}:${it.version}" }.mapTo(sortedSetOf()) { (_, packages) ->
+internal fun List<PythonInspector.Package>.toOrtPackages(): Set<Package> =
+    groupBy { "${it.name}:${it.version}" }.mapTo(mutableSetOf()) { (_, packages) ->
         // The python inspector currently often contains two entries for a package where the only difference is the
         // download URL. In this case, one package contains the URL of the binary artifact, the other for the source
         // artifact. So take all metadata from the first package except for the artifacts.

analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt

@@ -111,10 +111,10 @@ class AnalyzerResultBuilderTest : WordSpec() {
         )
 
     private val analyzerResult1 = ProjectAnalyzerResult(
-        project1, sortedSetOf(package1), listOf(issue3, issue4)
+        project1, setOf(package1), listOf(issue3, issue4)
     )
     private val analyzerResult2 = ProjectAnalyzerResult(
-        project2, sortedSetOf(package1, package2, package3), listOf(issue4)
+        project2, setOf(package1, package2, package3), listOf(issue4)
     )
 
     init {
@@ -236,10 +236,10 @@ class AnalyzerResultBuilderTest : WordSpec() {
                 val p1 = project1.copy(scopeDependencies = null, scopeNames = sortedSetOf("scope-1"))
                 val p2 = project2.copy(scopeDependencies = null, scopeNames = sortedSetOf("scope-3"))
                 val analyzerResult = AnalyzerResultBuilder()
-                    .addResult(ProjectAnalyzerResult(p1, sortedSetOf()))
+                    .addResult(ProjectAnalyzerResult(p1, emptySet()))
                     .addDependencyGraph(p1.id.type, graph1)
-                    .addResult(ProjectAnalyzerResult(project3, sortedSetOf()))
-                    .addResult(ProjectAnalyzerResult(p2, sortedSetOf()))
+                    .addResult(ProjectAnalyzerResult(project3, emptySet()))
+                    .addResult(ProjectAnalyzerResult(p2, emptySet()))
                     .addDependencyGraph(p2.id.type, graph2)
                     .build()
 
@@ -333,7 +333,7 @@ class AnalyzerResultBuilderTest : WordSpec() {
 
                 val projectAnalyzerResult = ProjectAnalyzerResult(
                     project = project,
-                    packages = sortedSetOf(package1)
+                    packages = setOf(package1)
                 )
 
                 val analyzerResult = AnalyzerResultBuilder().run {

model/src/main/kotlin/Package.kt

@@ -130,7 +130,7 @@ data class Package(
      */
     @JsonInclude(JsonInclude.Include.NON_DEFAULT)
     val isModified: Boolean = false
-) : Comparable<Package> {
+) {
     companion object {
         /**
          * A constant for a [Package] where all properties are empty.
@@ -152,11 +152,6 @@ data class Package(
         )
     }
 
-    /**
-     * A comparison function to sort packages by their identifier.
-     */
-    override fun compareTo(other: Package) = id.compareTo(other.id)
-
     /**
      * Compares this package with [other] and creates a [PackageCurationData] containing the values from this package
      * which are different in [other]. All equal values are set to null. Only the fields present in

model/src/main/kotlin/ProjectAnalyzerResult.kt

@@ -20,8 +20,9 @@
 package org.ossreviewtoolkit.model
 
 import com.fasterxml.jackson.annotation.JsonInclude
+import com.fasterxml.jackson.databind.annotation.JsonSerialize
 
-import java.util.SortedSet
+import org.ossreviewtoolkit.model.utils.PackageSortedSetConverter
 
 /**
  * A class that bundles all information generated during an analysis.
@@ -36,7 +37,8 @@ data class ProjectAnalyzerResult(
     /**
      * The set of identified packages used by the project.
      */
-    val packages: SortedSet<Package>,
+    @JsonSerialize(converter = PackageSortedSetConverter::class)
+    val packages: Set<Package>,
 
     /**
      * The list of issues that occurred during dependency resolution. Defaults to an empty list.

model/src/main/kotlin/utils/SortedSetConverters.kt

@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2022 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
+ */
+
+@file:Suppress("Filename", "MatchingDeclarationName")
+
+package org.ossreviewtoolkit.model.utils
+
+import com.fasterxml.jackson.databind.util.StdConverter
+
+import java.util.SortedSet
+
+import org.ossreviewtoolkit.model.Package
+
+class PackageSortedSetConverter : StdConverter<Set<Package>, SortedSet<Package>>() {
+    override fun convert(value: Set<Package>) = value.toSortedSet(compareBy { it.id })
+}
+
+// TODO: Add more converters to get rid of Comparable implementations that just serve sorted output.

model/src/test/kotlin/utils/DependencyGraphConverterTest.kt

@@ -203,7 +203,7 @@ private fun createAnalyzerResult(vararg projectResults: ProjectAnalyzerResult):
  * Create a [ProjectAnalyzerResult] based on this test project.
  */
 private fun Project.createResult(): ProjectAnalyzerResult {
-    val packages = scopes.flatMap { it.dependencies }.mapTo(sortedSetOf()) { ref ->
+    val packages = scopes.flatMap { it.dependencies }.mapTo(mutableSetOf()) { ref ->
         Package.EMPTY.copy(id = ref.id)
     }