refactor: Avoid the logger to leak into the public API

Use the new Log4j Kotlin API extension property to avoid the logger to leak into the public API via (public) companion objects. Also, find a better way to log from top-level functions by using `MethodHandles.lookup()` (introduced in JDK 7) to get the name of the Kotlin-internal class that is generated to hold top-level functions. SLF4J describes this as an idiomatic way to get a logger, see [1]. [1]: https://www.slf4j.org/faq.html#declaration_pattern Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Oct 7, 2023 · 29cba89 · 29cba89
1 parent 5cef1f8
commit 29cba89
Show file tree

Hide file tree

Showing 141 changed files with 265 additions and 411 deletions.
diff --git a/advisor/src/main/kotlin/AdviceProvider.kt b/advisor/src/main/kotlin/AdviceProvider.kt
@@ -19,8 +19,6 @@
 
 package org.ossreviewtoolkit.advisor
 
-import org.apache.logging.log4j.kotlin.Logging
-
 import org.ossreviewtoolkit.model.AdvisorDetails
 import org.ossreviewtoolkit.model.AdvisorResult
 import org.ossreviewtoolkit.model.Package
@@ -31,8 +29,6 @@ import org.ossreviewtoolkit.model.Package
  * or code analysis results.
  */
 abstract class AdviceProvider(val providerName: String) {
-    private companion object : Logging
-
     /**
      * For a given set of [Package]s, retrieve findings and return a map that associates packages with [AdvisorResult]s.
      */

diff --git a/advisor/src/main/kotlin/Advisor.kt b/advisor/src/main/kotlin/Advisor.kt
@@ -25,7 +25,7 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.async
 import kotlinx.coroutines.withContext
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.model.AdvisorRecord
 import org.ossreviewtoolkit.model.AdvisorResult
@@ -45,7 +45,7 @@ class Advisor(
     private val providerFactories: List<AdviceProviderFactory>,
     private val config: AdvisorConfiguration
 ) {
-    companion object : Logging {
+    companion object {
         /**
          * All [advice provider factories][AdviceProviderFactory] available in the classpath, associated by their names.
          */

diff --git a/advisor/src/main/kotlin/advisors/GitHubDefects.kt b/advisor/src/main/kotlin/advisors/GitHubDefects.kt
@@ -31,7 +31,7 @@ import kotlinx.coroutines.asCoroutineDispatcher
 import kotlinx.coroutines.async
 import kotlinx.coroutines.withContext
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
@@ -81,7 +81,7 @@ import org.ossreviewtoolkit.utils.ort.showStackTrace
  * suitable for production usage.
  */
 class GitHubDefects(name: String, config: GitHubDefectsConfiguration) : AdviceProvider(name) {
-    companion object : Logging {
+    companion object {
         /**
          * The default number of parallel requests executed by this advisor implementation. This value is used if the
          * corresponding property in the configuration is unspecified. It is chosen rather arbitrarily.

diff --git a/advisor/src/main/kotlin/advisors/NexusIq.kt b/advisor/src/main/kotlin/advisors/NexusIq.kt
@@ -23,7 +23,7 @@ import java.net.URI
 import java.time.Duration
 import java.time.Instant
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
@@ -65,8 +65,6 @@ private val READ_TIMEOUT = Duration.ofSeconds(60)
  * A wrapper for [Nexus IQ Server](https://help.sonatype.com/iqserver) security vulnerability data.
  */
 class NexusIq(name: String, private val config: NexusIqConfiguration) : AdviceProvider(name) {
-    private companion object : Logging
-
     class Factory : AbstractAdviceProviderFactory<NexusIq>("NexusIQ") {
         override fun create(config: AdvisorConfiguration) = NexusIq(type, config.forProvider { nexusIq })
     }

diff --git a/advisor/src/main/kotlin/advisors/OssIndex.kt b/advisor/src/main/kotlin/advisors/OssIndex.kt
@@ -22,7 +22,7 @@ package org.ossreviewtoolkit.advisor.advisors
 import java.net.URI
 import java.time.Instant
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
@@ -53,8 +53,6 @@ private const val BULK_REQUEST_SIZE = 128
  * A wrapper for Sonatype's [OSS Index](https://ossindex.sonatype.org/) security vulnerability data.
  */
 class OssIndex(name: String, config: OssIndexConfiguration) : AdviceProvider(name) {
-    private companion object : Logging
-
     class Factory : AbstractAdviceProviderFactory<OssIndex>("OssIndex") {
         override fun create(config: AdvisorConfiguration) =
             OssIndex(type, config.forProvider { ossIndex ?: OssIndexConfiguration() })

diff --git a/advisor/src/main/kotlin/advisors/Osv.kt b/advisor/src/main/kotlin/advisors/Osv.kt
@@ -24,7 +24,7 @@ import java.time.Instant
 import kotlinx.serialization.json.JsonPrimitive
 import kotlinx.serialization.json.contentOrNull
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
@@ -53,8 +53,6 @@ import us.springett.cvss.Cvss
  * An advice provider that obtains vulnerability information from Open Source Vulnerabilities (https://osv.dev/).
  */
 class Osv(name: String, config: OsvConfiguration) : AdviceProvider(name) {
-    internal companion object : Logging
-
     class Factory : AbstractAdviceProviderFactory<Osv>("OSV") {
         override fun create(config: AdvisorConfiguration) =
             // OSV does not require any dedicated configuration to be present.
@@ -184,7 +182,7 @@ private fun Vulnerability.toOrtVulnerability(): org.ossreviewtoolkit.model.Vulne
             // Work around for https://github.com/stevespringett/cvss-calculator/issues/56.
             it.score.substringBefore("/") to "${cvss.calculateScore().baseScore}"
         } ?: run {
-            Osv.logger.debug { "Could not parse CVSS vector '${it.score}'." }
+            logger.debug { "Could not parse CVSS vector '${it.score}'." }
             null to it.score
         }
     } ?: (null to null)
@@ -202,7 +200,7 @@ private fun Vulnerability.toOrtVulnerability(): org.ossreviewtoolkit.model.Vulne
         val url = reference.url.trim().let { if (it.startsWith("://")) "https$it" else it }
 
         url.toUri().onFailure {
-            Osv.logger.debug { "Could not parse reference URL for vulnerability '$id': ${it.message}." }
+            logger.debug { "Could not parse reference URL for vulnerability '$id': ${it.message}." }
         }.map {
             VulnerabilityReference(
                 url = it,

diff --git a/advisor/src/main/kotlin/advisors/VulnerableCode.kt b/advisor/src/main/kotlin/advisors/VulnerableCode.kt
@@ -22,8 +22,6 @@ package org.ossreviewtoolkit.advisor.advisors
 import java.net.URI
 import java.time.Instant
 
-import org.apache.logging.log4j.kotlin.Logging
-
 import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
 import org.ossreviewtoolkit.clients.vulnerablecode.VulnerableCodeService
@@ -56,8 +54,6 @@ private const val BULK_REQUEST_SIZE = 100
  * [VulnerableCode][https://github.com/nexB/vulnerablecode] instance.
  */
 class VulnerableCode(name: String, config: VulnerableCodeConfiguration) : AdviceProvider(name) {
-    private companion object : Logging
-
     class Factory : AbstractAdviceProviderFactory<VulnerableCode>("VulnerableCode") {
         override fun create(config: AdvisorConfiguration) = VulnerableCode(type, config.forProvider { vulnerableCode })
     }

diff --git a/analyzer/src/main/kotlin/Analyzer.kt b/analyzer/src/main/kotlin/Analyzer.kt
@@ -34,7 +34,7 @@ import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.analyzer.PackageManager.Companion.excludes
 import org.ossreviewtoolkit.downloader.VersionControlSystem
@@ -57,8 +57,6 @@ import org.ossreviewtoolkit.utils.ort.Environment
  * The class to run the analysis. The signatures of public functions in this class define the library API.
  */
 class Analyzer(private val config: AnalyzerConfiguration, private val labels: Map<String, String> = emptyMap()) {
-    internal companion object : Logging
-
     data class ManagedFileInfo(
         val absoluteProjectPath: File,
         val managedFiles: Map<PackageManager, List<File>>,
@@ -303,7 +301,7 @@ private class PackageManagerRunner(
                 val remaining = mustRunAfter - finishedPackageManagers
 
                 if (remaining.isNotEmpty()) {
-                    Analyzer.logger.info {
+                    logger.info {
                         "${manager.managerName} is waiting for the following package managers to complete: " +
                             remaining.joinToString(postfix = ".")
                     }
@@ -317,12 +315,12 @@ private class PackageManagerRunner(
     }
 
     private suspend fun run() {
-        Analyzer.logger.info { "Starting ${manager.managerName} analysis." }
+        logger.info { "Starting ${manager.managerName} analysis." }
 
         withContext(Dispatchers.IO) {
             val result = manager.resolveDependencies(definitionFiles, labels)
 
-            Analyzer.logger.info { "Finished ${manager.managerName} analysis." }
+            logger.info { "Finished ${manager.managerName} analysis." }
 
             onResult(result)
         }

diff --git a/analyzer/src/main/kotlin/AnalyzerResultBuilder.kt b/analyzer/src/main/kotlin/AnalyzerResultBuilder.kt
@@ -19,8 +19,6 @@
 
 package org.ossreviewtoolkit.analyzer
 
-import org.apache.logging.log4j.kotlin.Logging
-
 import org.ossreviewtoolkit.analyzer.managers.utils.PackageManagerDependencyHandler
 import org.ossreviewtoolkit.model.AnalyzerResult
 import org.ossreviewtoolkit.model.DependencyGraph
@@ -37,8 +35,6 @@ import org.ossreviewtoolkit.model.utils.convertToDependencyGraph
 import org.ossreviewtoolkit.utils.common.getDuplicates
 
 class AnalyzerResultBuilder {
-    private companion object : Logging
-
     private val projects = mutableSetOf<Project>()
     private val packages = mutableSetOf<Package>()
     private val issues = mutableMapOf<Identifier, List<Issue>>()

diff --git a/analyzer/src/main/kotlin/PackageManager.kt b/analyzer/src/main/kotlin/PackageManager.kt
@@ -30,7 +30,7 @@ import java.nio.file.attribute.BasicFileAttributes
 import kotlin.io.path.invariantSeparatorsPathString
 import kotlin.time.measureTime
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 import org.apache.maven.project.ProjectBuildingException
 
 import org.ossreviewtoolkit.downloader.VcsHost
@@ -69,7 +69,7 @@ abstract class PackageManager(
     val analyzerConfig: AnalyzerConfiguration,
     val repoConfig: RepositoryConfiguration
 ) {
-    companion object : Logging {
+    companion object {
         /**
          * All [package manager factories][PackageManagerFactory] available in the classpath, associated by their names.
          */

diff --git a/analyzer/src/main/kotlin/managers/GoDep.kt b/analyzer/src/main/kotlin/managers/GoDep.kt
@@ -28,7 +28,7 @@ import java.net.URI
 import kotlin.io.path.copyToRecursively
 import kotlin.io.path.createDirectories
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManager
@@ -84,8 +84,6 @@ class GoDep(
     analyzerConfig: AnalyzerConfiguration,
     repoConfig: RepositoryConfiguration
 ) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig), CommandLineTool {
-    private companion object : Logging
-
     class Factory : AbstractPackageManagerFactory<GoDep>("GoDep") {
         override val globsForDefinitionFiles = listOf("Gopkg.toml", *GO_LEGACY_MANIFESTS.keys.toTypedArray())
 

diff --git a/analyzer/src/main/kotlin/managers/GoMod.kt b/analyzer/src/main/kotlin/managers/GoMod.kt
@@ -27,7 +27,7 @@ import kotlinx.serialization.json.Json
 import kotlinx.serialization.json.decodeFromStream
 import kotlinx.serialization.json.decodeToSequence
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManager
@@ -68,7 +68,7 @@ class GoMod(
     analyzerConfig: AnalyzerConfiguration,
     repoConfig: RepositoryConfiguration
 ) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig), CommandLineTool {
-    companion object : Logging {
+    companion object {
         const val DEFAULT_GO_PROXY = "https://proxy.golang.org"
 
         val JSON = Json { ignoreUnknownKeys = true }

diff --git a/analyzer/src/main/kotlin/managers/Maven.kt b/analyzer/src/main/kotlin/managers/Maven.kt
@@ -21,7 +21,6 @@ package org.ossreviewtoolkit.analyzer.managers
 
 import java.io.File
 
-import org.apache.logging.log4j.kotlin.Logging
 import org.apache.maven.project.ProjectBuildingResult
 
 import org.eclipse.aether.artifact.Artifact
@@ -56,8 +55,6 @@ class Maven(
     analyzerConfig: AnalyzerConfiguration,
     repoConfig: RepositoryConfiguration
 ) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig) {
-    private companion object : Logging
-
     class Factory : AbstractPackageManagerFactory<Maven>("Maven") {
         override val globsForDefinitionFiles = listOf("pom.xml")
 

diff --git a/analyzer/src/main/kotlin/managers/Sbt.kt b/analyzer/src/main/kotlin/managers/Sbt.kt
@@ -21,12 +21,14 @@ package org.ossreviewtoolkit.analyzer.managers
 
 import java.io.File
 import java.io.IOException
+import java.lang.invoke.MethodHandles
 import java.nio.file.StandardCopyOption
 import java.util.Properties
 
 import kotlin.io.path.moveTo
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
+import org.apache.logging.log4j.kotlin.loggerOf
 
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManager
@@ -53,7 +55,7 @@ class Sbt(
     analyzerConfig: AnalyzerConfiguration,
     repoConfig: RepositoryConfiguration
 ) : PackageManager(name, analysisRoot, analyzerConfig, repoConfig), CommandLineTool {
-    companion object : Logging {
+    companion object {
         // See https://github.com/sbt/sbt/blob/v1.5.1/launcher-package/integration-test/src/test/scala/RunnerTest.scala#L9.
         private const val SBT_VERSION_PATTERN = "\\d(\\.\\d+){2}(-\\w+)?"
 
@@ -232,13 +234,15 @@ class Sbt(
         throw NotImplementedError()
 }
 
+private val logger = loggerOf(MethodHandles.lookup().lookupClass())
+
 private fun moveGeneratedPom(pomFile: File): File {
     val targetDirParent = pomFile.absoluteFile.parentFile.searchUpwardsForSubdirectory("target") ?: return pomFile
     val targetFilename = pomFile.relativeTo(targetDirParent).invariantSeparatorsPath.replace('/', '-')
     val targetFile = targetDirParent.resolve(targetFilename)
 
     if (runCatching { pomFile.toPath().moveTo(targetFile.toPath(), StandardCopyOption.ATOMIC_MOVE) }.isFailure) {
-        Sbt.logger.error { "Moving '${pomFile.absolutePath}' to '${targetFile.absolutePath}' failed." }
+        logger.error { "Moving '${pomFile.absolutePath}' to '${targetFile.absolutePath}' failed." }
         return pomFile
     }
 

diff --git a/analyzer/src/main/kotlin/managers/utils/Graph.kt b/analyzer/src/main/kotlin/managers/utils/Graph.kt
@@ -21,7 +21,7 @@ package org.ossreviewtoolkit.analyzer.managers.utils
 
 import java.util.LinkedList
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.model.Identifier
 import org.ossreviewtoolkit.model.PackageLinkage
@@ -32,8 +32,6 @@ import org.ossreviewtoolkit.model.PackageReference
  * map whose keys are package identifiers and whose values are the identifiers of packages these packages depend on.
  */
 internal class Graph private constructor(private val nodeMap: MutableMap<Identifier, MutableSet<Identifier>>) {
-    private companion object : Logging
-
     constructor() : this(mutableMapOf())
 
     /**

diff --git a/analyzer/src/main/kotlin/managers/utils/MavenDependencyHandler.kt b/analyzer/src/main/kotlin/managers/utils/MavenDependencyHandler.kt
@@ -19,7 +19,7 @@
 
 package org.ossreviewtoolkit.analyzer.managers.utils
 
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 import org.apache.maven.project.MavenProject
 
 import org.eclipse.aether.graph.DependencyNode
@@ -55,8 +55,6 @@ class MavenDependencyHandler(
      */
     private val sbtMode: Boolean
 ) : DependencyHandler<DependencyNode> {
-    private companion object : Logging
-
     /**
      * A set of identifiers that are known to point to local projects. This is updated for packages that are resolved
      * to projects.

diff --git a/analyzer/src/main/kotlin/managers/utils/MavenLogger.kt b/analyzer/src/main/kotlin/managers/utils/MavenLogger.kt
@@ -20,7 +20,7 @@
 package org.ossreviewtoolkit.analyzer.managers.utils
 
 import org.apache.logging.log4j.Level
-import org.apache.logging.log4j.kotlin.Logging
+import org.apache.logging.log4j.kotlin.logger
 
 import org.codehaus.plexus.logging.AbstractLogger
 import org.codehaus.plexus.logging.Logger
@@ -45,8 +45,6 @@ private fun toPlexusLoggerLevel(level: Level) =
  * appropriate log levels.
  */
 class MavenLogger(level: Level) : AbstractLogger(toPlexusLoggerLevel(level), "MavenLogger") {
-    private companion object : Logging
-
     override fun getChildLogger(name: String?) = this
 
     override fun debug(message: String, throwable: Throwable?) = logger.debug(message, throwable)