Do not override toString() for the Identifier anymore

analyzer/src/main/kotlin/Analyzer.kt

@@ -91,7 +91,7 @@ class Analyzer(private val config: AnalyzerConfiguration) {
             // Log the summary of projects found per package manager.
             managedFiles.forEach { manager, files ->
                 // No need to use curly-braces-syntax for logging here as the log level check is already done above.
-                log.info("$manager projects found in:")
+                log.info("${manager.managerName} projects found in:")
                 files.forEach { file ->
                     log.info("\t${file.toRelativeString(absoluteProjectPath).takeIf { it.isNotEmpty() } ?: "." }")
                 }
@@ -114,7 +114,8 @@ class Analyzer(private val config: AnalyzerConfiguration) {
                                 val curations = provider.getCurationsFor(curatedPackage.pkg.id)
                                 curations.fold(curatedPackage) { cur, packageCuration ->
                                     log.debug {
-                                        "Applying curation '$packageCuration' to package '${curatedPackage.pkg.id}'."
+                                        "Applying curation '$packageCuration' to package " +
+                                                "'${curatedPackage.pkg.id.toCoordinates()}'."
                                     }
                                     packageCuration.apply(cur)
                                 }

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

@@ -87,9 +87,9 @@ class GoDep(name: String, analyzerConfig: AnalyzerConfiguration, repoConfig: Rep
 
         for (project in projects) {
             // parseProjects() made sure that all entries contain these keys
-            val name = project["name"]!!
-            val revision = project["revision"]!!
-            val version = project["version"]!!
+            val name = project.getValue("name")
+            val revision = project.getValue("revision")
+            val version = project.getValue("version")
 
             val errors = mutableListOf<OrtIssue>()
 

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

@@ -304,7 +304,7 @@ class PIP(name: String, analyzerConfig: AnalyzerConfiguration, repoConfig: Repos
                     val body = response.body()?.string()?.trim()
 
                     if (response.code() != HttpURLConnection.HTTP_OK || body.isNullOrEmpty()) {
-                        log.warn { "Unable to retrieve PyPI meta-data for package '${pkg.id}'." }
+                        log.warn { "Unable to retrieve PyPI meta-data for package '${pkg.id.toCoordinates()}'." }
                         if (body != null) {
                             log.warn { "The response was '$body' (code ${response.code()})." }
                         }
@@ -318,7 +318,9 @@ class PIP(name: String, analyzerConfig: AnalyzerConfiguration, repoConfig: Repos
                     } catch (e: IOException) {
                         e.showStackTrace()
 
-                        log.warn { "Unable to parse PyPI meta-data for package '${pkg.id}': ${e.message}" }
+                        log.warn {
+                            "Unable to parse PyPI meta-data for package '${pkg.id.toCoordinates()}': ${e.message}"
+                        }
 
                         // Fall back to returning the original package data.
                         return@use pkg
@@ -353,7 +355,9 @@ class PIP(name: String, analyzerConfig: AnalyzerConfiguration, repoConfig: Repos
                     } catch (e: NullPointerException) {
                         e.showStackTrace()
 
-                        log.warn { "Unable to parse PyPI meta-data for package '${pkg.id}': ${e.message}" }
+                        log.warn {
+                            "Unable to parse PyPI meta-data for package '${pkg.id.toCoordinates()}': ${e.message}"
+                        }
 
                         // Fall back to returning the original package data.
                         pkg

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

@@ -213,7 +213,7 @@ class Stack(name: String, analyzerConfig: AnalyzerConfiguration, repoConfig: Rep
             val body = response.body()?.string()?.trim()
 
             if (response.code() != HttpURLConnection.HTTP_OK || body.isNullOrEmpty()) {
-                log.warn { "Unable to retrieve Hackage meta-data for package '${pkg.id}'." }
+                log.warn { "Unable to retrieve Hackage meta-data for package '${pkg.id.toCoordinates()}'." }
                 if (body != null) {
                     log.warn { "The response was '$body' (code ${response.code()})." }
                 }

cli/src/main/kotlin/commands/DownloaderCommand.kt

@@ -183,7 +183,7 @@ object DownloaderCommand : CommandWithHelp() {
                     } catch (e: IllegalArgumentException) {
                         e.showStackTrace()
 
-                        log.error { "Could not archive '${pkg.id}': ${e.message}" }
+                        log.error { "Could not archive '${pkg.id.toCoordinates()}': ${e.message}" }
                     } finally {
                         val relativePath = outputDir.toPath().relativize(result.downloadDirectory.toPath()).first()
                         File(outputDir, relativePath.toString()).safeDeleteRecursively()
@@ -192,7 +192,7 @@ object DownloaderCommand : CommandWithHelp() {
             } catch (e: DownloadException) {
                 e.showStackTrace()
 
-                log.error { "Could not download '${pkg.id}': ${e.message}" }
+                log.error { "Could not download '${pkg.id.toCoordinates()}': ${e.message}" }
 
                 error = true
             }

downloader/src/main/kotlin/Downloader.kt

@@ -132,7 +132,7 @@ class Downloader {
      * @throws DownloadException In case the download failed.
      */
     fun download(target: Package, outputDirectory: File, allowMovingRevisions: Boolean = false): DownloadResult {
-        log.info { "Trying to download source code for '${target.id}'." }
+        log.info { "Trying to download source code for '${target.id.toCoordinates()}'." }
 
         val targetDir = File(outputDirectory, target.id.toPath()).apply { safeMkdirs() }
 
@@ -172,7 +172,7 @@ class Downloader {
                 throw DownloadException(message)
             }
         } catch (e: DownloadException) {
-            log.debug { "VCS download failed for '${target.id}': ${e.message}" }
+            log.debug { "VCS download failed for '${target.id.toCoordinates()}': ${e.message}" }
 
             // Clean up any left-over files.
             targetDir.safeDeleteRecursively()
@@ -186,7 +186,7 @@ class Downloader {
         try {
             return downloadSourceArtifact(target, targetDir)
         } catch (e: DownloadException) {
-            log.debug { "Source artifact download failed for '${target.id}': ${e.message}" }
+            log.debug { "Source artifact download failed for '${target.id.toCoordinates()}': ${e.message}" }
 
             // Clean up any left-over files.
             targetDir.safeDeleteRecursively()
@@ -201,7 +201,7 @@ class Downloader {
             try {
                 return downloadPomArtifact(target, targetDir)
             } catch (e: DownloadException) {
-                log.debug { "POM artifact download failed for '${target.id}': ${e.message}" }
+                log.debug { "POM artifact download failed for '${target.id.toCoordinates()}': ${e.message}" }
 
                 // Clean up any left-over files.
                 targetDir.safeDeleteRecursively()
@@ -218,11 +218,11 @@ class Downloader {
 
     private fun downloadFromVcs(target: Package, outputDirectory: File, allowMovingRevisions: Boolean): DownloadResult {
         log.info {
-            "Trying to download '${target.id}' sources to '${outputDirectory.absolutePath}' from VCS..."
+            "Trying to download '${target.id.toCoordinates()}' sources to '${outputDirectory.absolutePath}' from VCS..."
         }
 
         if (target.vcsProcessed.url.isBlank()) {
-            throw DownloadException("No VCS URL provided for '${target.id}'.")
+            throw DownloadException("No VCS URL provided for '${target.id.toCoordinates()}'.")
         }
 
         if (target.vcsProcessed != target.vcs) {
@@ -295,11 +295,11 @@ class Downloader {
 
     private fun downloadSourceArtifact(target: Package, outputDirectory: File): DownloadResult {
         log.info {
-            "Trying to download source artifact for '${target.id}' from ${target.sourceArtifact.url}..."
+            "Trying to download source artifact for '${target.id.toCoordinates()}' from ${target.sourceArtifact.url}..."
         }
 
         if (target.sourceArtifact.url.isBlank()) {
-            throw DownloadException("No source artifact URL provided for '${target.id}'.")
+            throw DownloadException("No source artifact URL provided for '${target.id.toCoordinates()}'.")
         }
 
         val startTime = Instant.now()
@@ -363,7 +363,8 @@ class Downloader {
         }
 
         log.info {
-            "Successfully downloaded source artifact for '${target.id}' to '${outputDirectory.absolutePath}'..."
+            "Successfully downloaded source artifact for '${target.id.toCoordinates()}' to " +
+                    "'${outputDirectory.absolutePath}'..."
         }
 
         return DownloadResult(startTime, outputDirectory, sourceArtifact = target.sourceArtifact)
@@ -374,7 +375,7 @@ class Downloader {
         val pomUrl = target.binaryArtifact.url.replaceAfterLast('/', pomFilename)
 
         log.info {
-            "Trying to download POM artifact for '${target.id}' from $pomUrl..."
+            "Trying to download POM artifact for '${target.id.toCoordinates()}' from $pomUrl..."
         }
 
         return try {
@@ -391,11 +392,11 @@ class Downloader {
 
             DownloadResult(startTime, outputDirectory, sourceArtifact = pomArtifact)
         } catch (e: FileNotFoundException) {
-            throw DownloadException("Failed to download the Maven POM for '${target.id}'.")
+            throw DownloadException("Failed to download the Maven POM for '${target.id.toCoordinates()}'.")
         } catch (e: MalformedURLException) {
             // TODO: Investigate why the binary artifact URL is actually empty and update
             // the implementation according to root cause.
-            throw DownloadException("Failed to download the Maven POM for '${target.id}'.")
+            throw DownloadException("Failed to download the Maven POM for '${target.id.toCoordinates()}'.")
         }
     }
 }

model/src/main/kotlin/AnalyzerResult.kt

@@ -121,9 +121,10 @@ class AnalyzerResultBuilder {
 
                     val error = OrtIssue(
                             source = "analyzer",
-                            message = "Multiple projects with the same id '${existingProject.id}' found. Not adding " +
-                                    "the project defined in $incomingDefinitionFileUrl to the analyzer results " +
-                                    "as it duplicates the project defined in $existingDefinitionFileUrl."
+                            message = "Multiple projects with the same id '${existingProject.id.toCoordinates()}' " +
+                                    "found. Not adding the project defined in $incomingDefinitionFileUrl to the " +
+                                    "analyzer results as it duplicates the project defined in " +
+                                    "$existingDefinitionFileUrl."
                     )
 
                     log.error { error.message }

model/src/main/kotlin/Identifier.kt

@@ -20,9 +20,7 @@
 package com.here.ort.model
 
 import com.fasterxml.jackson.annotation.JsonCreator
-import com.fasterxml.jackson.core.JsonGenerator
-import com.fasterxml.jackson.databind.SerializerProvider
-import com.fasterxml.jackson.databind.ser.std.StdSerializer
+import com.fasterxml.jackson.annotation.JsonValue
 
 import com.here.ort.utils.encodeOrUnknown
 
@@ -86,7 +84,7 @@ data class Identifier(
         }
     }
 
-    override fun compareTo(other: Identifier) = toString().compareTo(other.toString())
+    override fun compareTo(other: Identifier) = toCoordinates().compareTo(other.toCoordinates())
 
     /**
      * Return whether this [Identifier] is likely to belong any of the organizations mentioned in [names].
@@ -133,18 +131,24 @@ data class Identifier(
     }
 
     /**
-     * Create a path based on the properties of the [Identifier]. All properties are encoded using [encodeOrUnknown].
+     * Create Maven-like coordinates based on the properties of the [Identifier].
      */
-    fun toPath() = components.joinToString("/") { it.encodeOrUnknown() }
-
-    // TODO: Consider using a PURL here, see https://github.com/package-url/purl-spec#purl.
     // TODO: We probably want to already sanitize the individual properties, also in other classes, but Kotlin does not
     // seem to offer a generic / elegant way to do so.
-    override fun toString() = components.joinToString(":") { it.trim().filterNot { it < ' ' } }
-}
+    @JsonValue
+    fun toCoordinates() = components.joinToString(":") { it.trim().filterNot { it < ' ' } }
 
-class IdentifierToStringSerializer : StdSerializer<Identifier>(Identifier::class.java) {
-    override fun serialize(value: Identifier, gen: JsonGenerator, provider: SerializerProvider) {
-        gen.writeString(value.toString())
-    }
+    /**
+     * Create a file system path based on the properties of the [Identifier]. All properties are encoded using
+     * [encodeOrUnknown].
+     */
+    fun toPath() = components.joinToString("/") { it.encodeOrUnknown() }
+
+    /**
+     * Create package URL ("purl") based on the properties of the [Identifier] as specified at
+     * https://github.com/package-url/purl-spec.
+     */
+    // TODO: This is a preliminary implementation as some open questions remain, see e.g.
+    // https://github.com/package-url/purl-spec/issues/33.
+    fun toPurl() = "pkg://$type/$namespace/$name@$version"
 }

model/src/main/kotlin/Mappers.kt

@@ -40,7 +40,6 @@ private val ortModelModule = SimpleModule("OrtModelModule").apply {
     addDeserializer(VcsInfo::class.java, VcsInfoDeserializer())
 
     addSerializer(OrtIssue::class.java, OrtIssueSerializer())
-    addSerializer(Identifier::class.java, IdentifierToStringSerializer())
 }
 
 val PROPERTY_NAMING_STRATEGY = PropertyNamingStrategy.SNAKE_CASE as PropertyNamingStrategy.PropertyNamingStrategyBase

model/src/main/kotlin/Package.kt

@@ -118,7 +118,9 @@ data class Package(
      * [PackageCurationData] are compared.
      */
     fun diff(other: Package): PackageCurationData {
-        require(id == other.id) { "Cannot diff packages with different ids: '$id' vs '${other.id}'" }
+        require(id == other.id) {
+            "Cannot diff packages with different ids: '${id.toCoordinates()}' vs. '${other.id.toCoordinates()}'"
+        }
 
         return PackageCurationData(
                 declaredLicenses = declaredLicenses.takeIf { it != other.declaredLicenses },

model/src/main/kotlin/PackageCuration.kt

@@ -43,8 +43,8 @@ data class PackageCuration(
      */
     fun apply(curatedPackage: CuratedPackage): CuratedPackage {
         if (!id.matches(curatedPackage.pkg.id)) {
-            throw IllegalArgumentException(
-                    "Package curation identifier '$id' does not match package identifier '${curatedPackage.pkg.id}'.")
+            throw IllegalArgumentException("Package curation identifier '${id.toCoordinates()}' does not match " +
+                    "package identifier '${curatedPackage.pkg.id.toCoordinates()}'.")
         }
 
         return data.apply(curatedPackage)

model/src/test/kotlin/IdentifierTest.kt

@@ -40,7 +40,7 @@ class IdentifierTest : StringSpec() {
             )
 
             mapping.forEach { identifier, stringRepresentation ->
-                identifier.toString() shouldBe stringRepresentation
+                identifier.toCoordinates() shouldBe stringRepresentation
             }
         }
 

model/src/test/kotlin/OrtResultTest.kt

@@ -38,7 +38,7 @@ class OrtResultTest : WordSpec({
             val result = resultFile.readValue<OrtResult>()
 
             val pkg = Package.EMPTY.copy(id = Identifier("Maven:com.typesafe.akka:akka-stream_2.12:2.5.6"))
-            result.collectAllDependencies(pkg, 1).map { it.id.toString() } shouldBe expectedDependencies
+            result.collectAllDependencies(pkg, 1).map { it.id.toCoordinates() } shouldBe expectedDependencies
         }
     }
 })

model/src/test/kotlin/PackageCurationTest.kt

@@ -72,7 +72,7 @@ class PackageCurationTest : StringSpec() {
             val curatedPkg = curation.apply(pkg.toCuratedPackage())
 
             curatedPkg.pkg.apply {
-                id.toString() shouldBe pkg.id.toString()
+                id.toCoordinates() shouldBe pkg.id.toCoordinates()
                 declaredLicenses shouldBe curation.data.declaredLicenses
                 concludedLicense shouldBe curation.data.concludedLicense
                 description shouldBe curation.data.description
@@ -123,7 +123,7 @@ class PackageCurationTest : StringSpec() {
             val curatedPkg = curation.apply(pkg.toCuratedPackage())
 
             curatedPkg.pkg.apply {
-                id.toString() shouldBe pkg.id.toString()
+                id.toCoordinates() shouldBe pkg.id.toCoordinates()
                 declaredLicenses shouldBe pkg.declaredLicenses
                 concludedLicense shouldBe pkg.concludedLicense
                 description shouldBe pkg.description

model/src/test/kotlin/ProjectTest.kt

@@ -42,7 +42,7 @@ class ProjectTest : WordSpec({
                     File("../analyzer/src/funTest/assets/projects/synthetic/gradle-expected-output-lib.yml")
             val project = analyzerResultsFile.readValue<ProjectAnalyzerResult>().project
 
-            project.collectDependencies().map { it.id.toString() } shouldBe expectedDependencies
+            project.collectDependencies().map { it.id.toCoordinates() } shouldBe expectedDependencies
         }
 
         "get no dependencies for a depth of 0" {
@@ -64,7 +64,7 @@ class ProjectTest : WordSpec({
                     File("../analyzer/src/funTest/assets/projects/synthetic/gradle-expected-output-lib.yml")
             val project = analyzerResultsFile.readValue<ProjectAnalyzerResult>().project
 
-            project.collectDependencies(maxDepth = 1).map { it.id.toString() } shouldBe expectedDependencies
+            project.collectDependencies(maxDepth = 1).map { it.id.toCoordinates() } shouldBe expectedDependencies
         }
     }