refactor(plugins)!: Move all ALL properties to Plugin implementat…

…ions For some plugin types, the conventional `ALL` property was not defined in the actual plugin implementation, but in a class managing the plugin implementations. Resolve this inconsistency by generally defining the `ALL` properties in the belonging plugin implementation to prepare for an upcoming change that inspects plugin implementations. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Nov 24, 2023 · b76b7a7 · b76b7a7
1 parent 5d2fb19
commit b76b7a7
Show file tree

Hide file tree

Showing 17 changed files with 61 additions and 58 deletions.
diff --git a/advisor/src/main/kotlin/AdviceProviderFactory.kt b/advisor/src/main/kotlin/AdviceProviderFactory.kt
@@ -21,13 +21,21 @@ package org.ossreviewtoolkit.advisor
 
 import java.util.ServiceLoader
 
+import org.ossreviewtoolkit.utils.common.Plugin
 import org.ossreviewtoolkit.utils.common.TypedConfigurablePluginFactory
 
 /**
  * A common abstract class for use with [ServiceLoader] that all [AdviceProviderFactory] classes need to implement.
  */
 abstract class AdviceProviderFactory<CONFIG>(override val type: String) :
     TypedConfigurablePluginFactory<CONFIG, AdviceProvider> {
+    companion object {
+        /**
+         * All [advice provider factories][AdviceProviderFactory] available in the classpath, associated by their names.
+         */
+        val ALL by lazy { Plugin.getAll<AdviceProviderFactory<*>>() }
+    }
+
     /**
      * Return the provider's type here to allow Clikt to display something meaningful when listing the advisors which
      * are enabled by default via their factories.

diff --git a/advisor/src/main/kotlin/Advisor.kt b/advisor/src/main/kotlin/Advisor.kt
@@ -34,7 +34,6 @@ import org.ossreviewtoolkit.model.Identifier
 import org.ossreviewtoolkit.model.OrtResult
 import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.config.AdvisorConfiguration
-import org.ossreviewtoolkit.utils.common.Plugin
 import org.ossreviewtoolkit.utils.ort.Environment
 
 /**
@@ -45,13 +44,6 @@ class Advisor(
     private val providerFactories: List<AdviceProviderFactory<*>>,
     private val config: AdvisorConfiguration
 ) {
-    companion object {
-        /**
-         * All [advice provider factories][AdviceProviderFactory] available in the classpath, associated by their names.
-         */
-        val ALL by lazy { Plugin.getAll<AdviceProviderFactory<*>>() }
-    }
-
     /**
      * Query the [advice providers][providerFactories] and add the result to the provided [ortResult]. Excluded packages
      * can optionally be [skipped][skipExcluded].

diff --git a/analyzer/src/funTest/kotlin/PackageManagerFunTest.kt b/analyzer/src/funTest/kotlin/PackageManagerFunTest.kt
@@ -85,8 +85,8 @@ class PackageManagerFunTest : WordSpec({
 
             // The test project contains at least one file per package manager, so the result should also contain an
             // entry for each package manager.
-            val unmanagedPackageManagerFactory = PackageManager.ALL.getValue("Unmanaged")
-            managedFiles.keys shouldContainExactlyInAnyOrder PackageManager.ENABLED_BY_DEFAULT.filterNot {
+            val unmanagedPackageManagerFactory = PackageManagerFactory.ALL.getValue("Unmanaged")
+            managedFiles.keys shouldContainExactlyInAnyOrder PackageManagerFactory.ENABLED_BY_DEFAULT.filterNot {
                 it == unmanagedPackageManagerFactory
             }
 
@@ -144,9 +144,9 @@ class PackageManagerFunTest : WordSpec({
             val managedFiles = PackageManager.findManagedFiles(
                 projectDir,
                 setOf(
-                    PackageManager.ALL.getValue("Gradle"),
-                    PackageManager.ALL.getValue("Pip"),
-                    PackageManager.ALL.getValue("Sbt")
+                    PackageManagerFactory.ALL.getValue("Gradle"),
+                    PackageManagerFactory.ALL.getValue("Pip"),
+                    PackageManagerFactory.ALL.getValue("Sbt")
                 )
             )
 

diff --git a/analyzer/src/main/kotlin/Analyzer.kt b/analyzer/src/main/kotlin/Analyzer.kt
@@ -71,7 +71,7 @@ class Analyzer(private val config: AnalyzerConfiguration, private val labels: Ma
     @JvmOverloads
     fun findManagedFiles(
         absoluteProjectPath: File,
-        packageManagers: Collection<PackageManagerFactory> = PackageManager.ENABLED_BY_DEFAULT,
+        packageManagers: Collection<PackageManagerFactory> = PackageManagerFactory.ENABLED_BY_DEFAULT,
         repositoryConfiguration: RepositoryConfiguration = RepositoryConfiguration()
     ): ManagedFileInfo {
         require(absoluteProjectPath.isAbsolute)
@@ -111,7 +111,7 @@ class Analyzer(private val config: AnalyzerConfiguration, private val labels: Ma
         }
 
         if (!hasOnlyManagedDirs) {
-            val unmanagedPackageManagerFactory = PackageManager.ALL["Unmanaged"]
+            val unmanagedPackageManagerFactory = PackageManagerFactory.ALL["Unmanaged"]
             distinctPackageManagers.find { it == unmanagedPackageManagerFactory }
                 ?.create(absoluteProjectPath, config, repositoryConfiguration)
                 ?.also { managedFiles[it] = listOf(absoluteProjectPath) }

diff --git a/analyzer/src/main/kotlin/Extensions.kt b/analyzer/src/main/kotlin/Extensions.kt
@@ -31,16 +31,16 @@ import org.ossreviewtoolkit.utils.common.alsoIfNull
  */
 fun AnalyzerConfiguration.determineEnabledPackageManagers(): Set<PackageManagerFactory> {
     val enabled = enabledPackageManagers?.mapNotNull { name ->
-        PackageManager.ALL[name].alsoIfNull {
+        PackageManagerFactory.ALL[name].alsoIfNull {
             logger.error {
                 "Package manager '$name' is configured to be enabled but is not available in the classpath. It must " +
-                    "be one of: ${PackageManager.ALL.keys.joinToString()}."
+                    "be one of: ${PackageManagerFactory.ALL.keys.joinToString()}."
             }
         }
-    } ?: PackageManager.ENABLED_BY_DEFAULT
+    } ?: PackageManagerFactory.ENABLED_BY_DEFAULT
 
     val disabled = disabledPackageManagers?.mapNotNull { name ->
-        PackageManager.ALL[name].alsoIfNull {
+        PackageManagerFactory.ALL[name].alsoIfNull {
             logger.warn {
                 "Package manager '$name' is configured to be disabled but is not available in the classpath."
             }

diff --git a/analyzer/src/main/kotlin/PackageManager.kt b/analyzer/src/main/kotlin/PackageManager.kt
@@ -42,7 +42,6 @@ import org.ossreviewtoolkit.model.config.PackageManagerConfiguration
 import org.ossreviewtoolkit.model.config.RepositoryConfiguration
 import org.ossreviewtoolkit.model.createAndLogIssue
 import org.ossreviewtoolkit.utils.common.Options
-import org.ossreviewtoolkit.utils.common.Plugin
 import org.ossreviewtoolkit.utils.common.VCS_DIRECTORIES
 import org.ossreviewtoolkit.utils.common.collectMessages
 import org.ossreviewtoolkit.utils.common.isSymbolicLink
@@ -65,16 +64,6 @@ abstract class PackageManager(
     val repoConfig: RepositoryConfiguration
 ) {
     companion object {
-        /**
-         * All [package manager factories][PackageManagerFactory] available in the classpath, associated by their names.
-         */
-        val ALL by lazy { Plugin.getAll<PackageManagerFactory>() }
-
-        /**
-         * The available [package manager factories][PackageManagerFactory] that are enabled by default.
-         */
-        val ENABLED_BY_DEFAULT by lazy { ALL.values.filter { it.isEnabledByDefault } }
-
         private val PACKAGE_MANAGER_DIRECTORIES = setOf(
             // Ignore intermediate build system directories.
             ".gradle",
@@ -100,7 +89,7 @@ abstract class PackageManager(
          */
         fun findManagedFiles(
             directory: File,
-            packageManagers: Collection<PackageManagerFactory> = ENABLED_BY_DEFAULT,
+            packageManagers: Collection<PackageManagerFactory> = PackageManagerFactory.ENABLED_BY_DEFAULT,
             excludes: Excludes = Excludes.EMPTY
         ): ManagedProjectFiles {
             require(directory.isDirectory) {

diff --git a/analyzer/src/main/kotlin/PackageManagerFactory.kt b/analyzer/src/main/kotlin/PackageManagerFactory.kt
@@ -32,6 +32,18 @@ import org.ossreviewtoolkit.utils.common.Plugin
  * A common interface for use with [ServiceLoader] that all [AbstractPackageManagerFactory] classes need to implement.
  */
 interface PackageManagerFactory : Plugin {
+    companion object {
+        /**
+         * All [package manager factories][PackageManagerFactory] available in the classpath, associated by their names.
+         */
+        val ALL by lazy { Plugin.getAll<PackageManagerFactory>() }
+
+        /**
+         * The available [package manager factories][PackageManagerFactory] that are enabled by default.
+         */
+        val ENABLED_BY_DEFAULT by lazy { ALL.values.filter { it.isEnabledByDefault } }
+    }
+
     /**
      * The glob matchers for all definition files.
      */

diff --git a/analyzer/src/testFixtures/kotlin/Extensions.kt b/analyzer/src/testFixtures/kotlin/Extensions.kt
@@ -128,7 +128,7 @@ fun ProjectAnalyzerResult.withInvariantIssues() = copy(
 fun Spec.analyze(
     projectDir: File,
     allowDynamicVersions: Boolean = false,
-    packageManagers: Collection<PackageManagerFactory> = PackageManager.ENABLED_BY_DEFAULT
+    packageManagers: Collection<PackageManagerFactory> = PackageManagerFactory.ENABLED_BY_DEFAULT
 ): OrtResult {
     val config = AnalyzerConfiguration(allowDynamicVersions)
     val analyzer = Analyzer(config)
@@ -141,7 +141,7 @@ fun Spec.create(
     managerName: String,
     analyzerConfig: AnalyzerConfiguration,
     repoConfig: RepositoryConfiguration = RepositoryConfiguration()
-) = PackageManager.ALL.getValue(managerName).create(USER_DIR, analyzerConfig, repoConfig)
+) = PackageManagerFactory.ALL.getValue(managerName).create(USER_DIR, analyzerConfig, repoConfig)
 
 fun Spec.create(
     managerName: String,

diff --git a/cli/src/funTest/kotlin/AnalyzerFunTest.kt b/cli/src/funTest/kotlin/AnalyzerFunTest.kt
@@ -27,7 +27,7 @@ import io.kotest.matchers.should
 import java.util.concurrent.TimeUnit
 
 import org.ossreviewtoolkit.analyzer.Analyzer
-import org.ossreviewtoolkit.analyzer.PackageManager
+import org.ossreviewtoolkit.analyzer.PackageManagerFactory
 import org.ossreviewtoolkit.analyzer.managers.analyze
 import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.VcsInfo
@@ -70,7 +70,7 @@ class AnalyzerFunTest : WordSpec({
             val repoConfig = RepositoryConfiguration()
 
             val analyzer = Analyzer(analyzerConfig)
-            val gradleFactory = PackageManager.ALL.getValue("Gradle")
+            val gradleFactory = PackageManagerFactory.ALL.getValue("Gradle")
             val gradle = gradleFactory.create(inputDir, analyzerConfig, repoConfig)
             val info = Analyzer.ManagedFileInfo(
                 inputDir,

diff --git a/cli/src/funTest/kotlin/OrtMainFunTest.kt b/cli/src/funTest/kotlin/OrtMainFunTest.kt
@@ -34,7 +34,7 @@ import io.kotest.matchers.shouldBe
 
 import java.io.File
 
-import org.ossreviewtoolkit.analyzer.PackageManager
+import org.ossreviewtoolkit.analyzer.PackageManagerFactory
 import org.ossreviewtoolkit.model.OrtResult
 import org.ossreviewtoolkit.model.config.OrtConfiguration
 import org.ossreviewtoolkit.model.config.OrtConfigurationWrapper
@@ -101,7 +101,7 @@ class OrtMainFunTest : StringSpec() {
         }
 
         "Disabling only Gradle works" {
-            val expectedPackageManagers = PackageManager.ENABLED_BY_DEFAULT.filterNot { it.type == "Gradle" }
+            val expectedPackageManagers = PackageManagerFactory.ENABLED_BY_DEFAULT.filterNot { it.type == "Gradle" }
             val markerLine = "The following ${expectedPackageManagers.size} package manager(s) are enabled:"
             val inputDir = tempdir()
 

diff --git a/helper-cli/src/main/kotlin/utils/Extensions.kt b/helper-cli/src/main/kotlin/utils/Extensions.kt
@@ -32,7 +32,7 @@ import kotlin.io.path.createTempDirectory
 
 import org.jetbrains.exposed.sql.transactions.TransactionManager
 
-import org.ossreviewtoolkit.analyzer.PackageManager
+import org.ossreviewtoolkit.analyzer.PackageManagerFactory
 import org.ossreviewtoolkit.downloader.Downloader
 import org.ossreviewtoolkit.model.ArtifactProvenance
 import org.ossreviewtoolkit.model.Identifier
@@ -254,7 +254,7 @@ internal fun OrtResult.getScanIssues(omitExcluded: Boolean = false): List<Issue>
  */
 internal fun OrtResult.getRepositoryPathExcludes(): RepositoryPathExcludes {
     fun isDefinitionsFile(pathExclude: PathExclude) =
-        PackageManager.ENABLED_BY_DEFAULT.any {
+        PackageManagerFactory.ENABLED_BY_DEFAULT.any {
             it.matchersForDefinitionFiles.any { matcher ->
                 pathExclude.pattern.endsWith(matcher.toString())
             }

diff --git a/plugins/commands/advisor/src/main/kotlin/AdvisorCommand.kt b/plugins/commands/advisor/src/main/kotlin/AdvisorCommand.kt
@@ -38,6 +38,7 @@ import kotlin.time.toKotlinDuration
 
 import kotlinx.coroutines.runBlocking
 
+import org.ossreviewtoolkit.advisor.AdviceProviderFactory
 import org.ossreviewtoolkit.advisor.Advisor
 import org.ossreviewtoolkit.model.FileFormat
 import org.ossreviewtoolkit.model.utils.DefaultResolutionProvider
@@ -96,9 +97,10 @@ class AdvisorCommand : OrtCommand(
 
     private val providerFactories by option(
         "--advisors", "-a",
-        help = "The comma-separated advisors to use, any of ${Advisor.ALL.keys}."
+        help = "The comma-separated advisors to use, any of ${AdviceProviderFactory.ALL.keys}."
     ).convert { name ->
-        Advisor.ALL[name] ?: throw BadParameterValue("Advisor '$name' is not one of ${Advisor.ALL.keys}.")
+        AdviceProviderFactory.ALL[name]
+            ?: throw BadParameterValue("Advisor '$name' is not one of ${AdviceProviderFactory.ALL.keys}.")
     }.split(",").required()
 
     private val skipExcluded by option(

diff --git a/plugins/commands/scanner/src/main/kotlin/ScannerCommand.kt b/plugins/commands/scanner/src/main/kotlin/ScannerCommand.kt
@@ -59,7 +59,6 @@ import org.ossreviewtoolkit.plugins.commands.api.utils.readOrtResult
 import org.ossreviewtoolkit.plugins.commands.api.utils.writeOrtResult
 import org.ossreviewtoolkit.scanner.ScanStorages
 import org.ossreviewtoolkit.scanner.Scanner
-import org.ossreviewtoolkit.scanner.ScannerWrapper
 import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
 import org.ossreviewtoolkit.scanner.provenance.DefaultNestedProvenanceResolver
 import org.ossreviewtoolkit.scanner.provenance.DefaultPackageProvenanceResolver
@@ -113,15 +112,15 @@ class ScannerCommand : OrtCommand(
 
     private val scanners by option(
         "--scanners", "-s",
-        help = "A comma-separated list of scanners to use.\nPossible values are: ${ScannerWrapper.ALL.keys}"
+        help = "A comma-separated list of scanners to use.\nPossible values are: ${ScannerWrapperFactory.ALL.keys}"
     ).convertToScannerWrapperFactories()
-        .default(listOfNotNull(ScannerWrapper.ALL.values.singleOrNull() ?: ScannerWrapper.ALL["ScanCode"]))
+        .default(listOfNotNull(ScannerWrapperFactory.ALL.let { it.values.singleOrNull() ?: it["ScanCode"] }))
 
     private val projectScanners by option(
         "--project-scanners",
         help = "A comma-separated list of scanners to use for scanning the source code of projects. By default, " +
             "projects and packages are scanned with the same scanners as specified by '--scanners'.\n" +
-            "Possible values are: ${ScannerWrapper.ALL.keys}"
+            "Possible values are: ${ScannerWrapperFactory.ALL.keys}"
     ).convertToScannerWrapperFactories()
 
     private val packageTypes by option(
@@ -249,7 +248,7 @@ class ScannerCommand : OrtCommand(
 private fun RawOption.convertToScannerWrapperFactories() =
     convert { scannerNames ->
         scannerNames.split(",").map { name ->
-            ScannerWrapper.ALL[name]
-                ?: throw BadParameterValue("Scanner '$name' is not one of ${ScannerWrapper.ALL.keys}.")
+            ScannerWrapperFactory.ALL[name]
+                ?: throw BadParameterValue("Scanner '$name' is not one of ${ScannerWrapperFactory.ALL.keys}.")
         }
     }
diff --git a/plugins/package-managers/pub/src/main/kotlin/Pub.kt b/plugins/package-managers/pub/src/main/kotlin/Pub.kt
@@ -30,6 +30,7 @@ import org.apache.logging.log4j.kotlin.logger
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManager
 import org.ossreviewtoolkit.analyzer.PackageManagerDependencyResult
+import org.ossreviewtoolkit.analyzer.PackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManagerResult
 import org.ossreviewtoolkit.analyzer.managers.utils.PackageManagerDependencyHandler
 import org.ossreviewtoolkit.analyzer.parseAuthorString
@@ -121,7 +122,7 @@ class Pub(
         ) = Pub(type, analysisRoot, analyzerConfig, repoConfig)
     }
 
-    private val gradleFactory = ALL["Gradle"]
+    private val gradleFactory = PackageManagerFactory.ALL["Gradle"]
 
     private data class ParsePackagesResult(
         val packages: Map<Identifier, Package>,

diff --git a/scanner/src/main/kotlin/ScannerWrapper.kt b/scanner/src/main/kotlin/ScannerWrapper.kt
@@ -29,19 +29,11 @@ import org.ossreviewtoolkit.model.ScanSummary
 import org.ossreviewtoolkit.model.ScannerDetails
 import org.ossreviewtoolkit.model.config.PluginConfiguration
 import org.ossreviewtoolkit.model.config.ScannerConfiguration
-import org.ossreviewtoolkit.utils.common.Plugin
 
 /**
  * The base interface for all types of scanners.
  */
 sealed interface ScannerWrapper {
-    companion object {
-        /**
-         * All [scanner wrapper factories][ScannerWrapperFactory] available in the classpath, associated by their names.
-         */
-        val ALL by lazy { Plugin.getAll<ScannerWrapperFactory<*>>() }
-    }
-
     /**
      * The name of the scanner.
      */

diff --git a/scanner/src/main/kotlin/ScannerWrapperFactory.kt b/scanner/src/main/kotlin/ScannerWrapperFactory.kt
@@ -22,13 +22,21 @@ package org.ossreviewtoolkit.scanner
 import java.util.ServiceLoader
 
 import org.ossreviewtoolkit.utils.common.Options
+import org.ossreviewtoolkit.utils.common.Plugin
 import org.ossreviewtoolkit.utils.common.TypedConfigurablePluginFactory
 
 /**
  * A common abstract class for use with [ServiceLoader] that all [ScannerWrapperFactory] classes need to implement.
  */
 abstract class ScannerWrapperFactory<CONFIG>(override val type: String) :
     TypedConfigurablePluginFactory<CONFIG, ScannerWrapper> {
+    companion object {
+        /**
+         * All [scanner wrapper factories][ScannerWrapperFactory] available in the classpath, associated by their names.
+         */
+        val ALL by lazy { Plugin.getAll<ScannerWrapperFactory<*>>() }
+    }
+
     override fun create(options: Options, secrets: Options): ScannerWrapper {
         val (wrapperConfig, filteredOptions) = ScannerWrapperConfig.create(options)
         return create(parseConfig(filteredOptions, secrets), wrapperConfig)

diff --git a/scanner/src/main/kotlin/storages/ClearlyDefinedStorage.kt b/scanner/src/main/kotlin/storages/ClearlyDefinedStorage.kt
@@ -52,7 +52,7 @@ import org.ossreviewtoolkit.model.utils.toClearlyDefinedSourceLocation
 import org.ossreviewtoolkit.scanner.CommandLinePathScannerWrapper
 import org.ossreviewtoolkit.scanner.ScanResultsStorage
 import org.ossreviewtoolkit.scanner.ScanStorageException
-import org.ossreviewtoolkit.scanner.ScannerWrapper
+import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
 import org.ossreviewtoolkit.scanner.storages.utils.getScanCodeDetails
 import org.ossreviewtoolkit.utils.common.AlphaNumericComparator
 import org.ossreviewtoolkit.utils.common.collectMessages
@@ -110,7 +110,7 @@ class ClearlyDefinedStorage(
 
             val supportedScanners = toolVersionsByName.mapNotNull { (name, versions) ->
                 // For the ClearlyDefined tool names see https://github.com/clearlydefined/service#tool-name-registry.
-                ScannerWrapper.ALL[name]?.let { factory ->
+                ScannerWrapperFactory.ALL[name]?.let { factory ->
                     val scanner = factory.create(emptyMap(), emptyMap())
                     (scanner as? CommandLinePathScannerWrapper)?.let { cliScanner -> cliScanner to versions.last() }
                 }.also { factory ->