Skip to content

Commit

Permalink
refactor: Avoid the logger to leak into the public API
Browse files Browse the repository at this point in the history
Use the new 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]>
  • Loading branch information
sschuberth committed Oct 6, 2023
1 parent f16df4e commit 6b54b7b
Show file tree
Hide file tree
Showing 139 changed files with 232 additions and 399 deletions.
4 changes: 0 additions & 4 deletions advisor/src/main/kotlin/AdviceProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
*/
Expand Down
4 changes: 2 additions & 2 deletions advisor/src/main/kotlin/Advisor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
*/
Expand Down
4 changes: 2 additions & 2 deletions advisor/src/main/kotlin/advisors/GitHubDefects.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions advisor/src/main/kotlin/advisors/NexusIq.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })
}
Expand Down
4 changes: 1 addition & 3 deletions advisor/src/main/kotlin/advisors/OssIndex.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() })
Expand Down
8 changes: 3 additions & 5 deletions advisor/src/main/kotlin/advisors/Osv.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions advisor/src/main/kotlin/advisors/VulnerableCode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })
}
Expand Down
10 changes: 4 additions & 6 deletions analyzer/src/main/kotlin/Analyzer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>>,
Expand Down Expand Up @@ -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 = ".")
}
Expand All @@ -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)
}
Expand Down
4 changes: 0 additions & 4 deletions analyzer/src/main/kotlin/AnalyzerResultBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>>()
Expand Down
4 changes: 2 additions & 2 deletions analyzer/src/main/kotlin/PackageManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down
4 changes: 1 addition & 3 deletions analyzer/src/main/kotlin/managers/GoDep.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())

Expand Down
4 changes: 2 additions & 2 deletions analyzer/src/main/kotlin/managers/GoMod.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand Down
3 changes: 0 additions & 3 deletions analyzer/src/main/kotlin/managers/Maven.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")

Expand Down
4 changes: 2 additions & 2 deletions analyzer/src/main/kotlin/managers/Sbt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ 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.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
import org.ossreviewtoolkit.analyzer.PackageManager
Expand All @@ -53,7 +53,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+)?"

Expand Down
4 changes: 1 addition & 3 deletions analyzer/src/main/kotlin/managers/utils/Graph.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions analyzer/src/main/kotlin/managers/utils/MavenLogger.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions analyzer/src/main/kotlin/managers/utils/MavenSupport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import java.net.URI

import kotlin.time.Duration.Companion.hours

import org.apache.logging.log4j.kotlin.Logging
import org.apache.logging.log4j.kotlin.logger
import org.apache.maven.artifact.repository.LegacyLocalRepositoryManager
import org.apache.maven.bridge.MavenRepositorySystem
import org.apache.maven.execution.DefaultMavenExecutionRequest
Expand Down Expand Up @@ -109,7 +109,7 @@ private val File?.safePath: String
get() = this?.invariantSeparatorsPath ?: "<unknown file>"

class MavenSupport(private val workspaceReader: WorkspaceReader) {
companion object : Logging {
companion object {
private val PACKAGING_TYPES = setOf(
// Core packaging types, see https://maven.apache.org/pom.html#packaging.
"pom", "jar", "maven-plugin", "ejb", "war", "ear", "rar",
Expand Down Expand Up @@ -828,7 +828,7 @@ class MavenSupport(private val workspaceReader: WorkspaceReader) {
* [Medium article](https://medium.com/p/d069d253fe23)
*/
private class HttpsMirrorSelector(private val originalMirrorSelector: MirrorSelector?) : MirrorSelector {
companion object : Logging {
companion object {
private val DISABLED_HTTP_REPOSITORY_URLS = listOf(
"http://jcenter.bintray.com",
"http://repo.maven.apache.org",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ package org.ossreviewtoolkit.analyzer.managers.utils
import kotlin.contracts.ExperimentalContracts
import kotlin.contracts.contract

import org.apache.logging.log4j.kotlin.Logging
import org.apache.logging.log4j.kotlin.logger

import org.ossreviewtoolkit.model.AnalyzerResult
import org.ossreviewtoolkit.model.DependencyGraphNavigator
Expand Down Expand Up @@ -113,8 +113,6 @@ private data class PackageManagerDependency(
val scope: String,
val linkage: PackageLinkage
) {
private companion object : Logging

fun findProjects(analyzerResult: AnalyzerResult): List<Project> =
analyzerResult.projects.filter { it.definitionFilePath == definitionFile }.also { projects ->
if (projects.isEmpty()) {
Expand Down
Loading

0 comments on commit 6b54b7b

Please sign in to comment.