Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(scanner)!: Use the configurable plugin API for scanner wrappers #7673

Merged
merged 1 commit into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
import org.ossreviewtoolkit.model.config.RepositoryConfiguration
import org.ossreviewtoolkit.plugins.commands.api.OrtCommand
import org.ossreviewtoolkit.scanner.CommandLinePathScannerWrapper
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.utils.common.CommandLineTool
import org.ossreviewtoolkit.utils.spdx.scanCodeLicenseTextDir

Expand Down Expand Up @@ -88,8 +89,8 @@ class RequirementsCommand : OrtCommand(
logger.debug { "$it is a $category." }
it.getDeclaredConstructor(
String::class.java,
Map::class.java
).newInstance("", emptyMap<String, String>())
ScannerMatcherConfig::class.java
).newInstance("", ScannerMatcherConfig.EMPTY)
}

VersionControlSystem::class.java.isAssignableFrom(it) -> {
Expand Down
4 changes: 2 additions & 2 deletions plugins/commands/scanner/src/main/kotlin/ScannerCommand.kt
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,10 @@ class ScannerCommand : OrtCommand(
): OrtResult {
val packageScannerWrappers = scannerWrapperFactories
.takeIf { PackageType.PACKAGE in packageTypes }.orEmpty()
.map { it.create(ortConfig.scanner.options?.get(it.type).orEmpty()) }
.map { it.create(ortConfig.scanner.options?.get(it.type).orEmpty(), emptyMap()) }
val projectScannerWrappers = projectScannerWrapperFactories
.takeIf { PackageType.PROJECT in packageTypes }.orEmpty()
.map { it.create(ortConfig.scanner.options?.get(it.type).orEmpty()) }
.map { it.create(ortConfig.scanner.options?.get(it.type).orEmpty(), emptyMap()) }

if (projectScannerWrappers.isNotEmpty()) {
echo("Scanning projects with:")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ package org.ossreviewtoolkit.plugins.scanners.askalono

import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.scanners.AbstractPathScannerWrapperFunTest

class AskalonoFunTest : AbstractPathScannerWrapperFunTest() {
override val scanner = Askalono("Askalono", emptyMap())
override val scanner = Askalono("Askalono", ScannerMatcherConfig.EMPTY)

override val expectedFileLicenses = listOf(
LicenseFinding("Apache-2.0", TextLocation("LICENSE", TextLocation.UNKNOWN_LINE), 1.0f)
Expand Down
12 changes: 8 additions & 4 deletions plugins/scanners/askalono/src/main/kotlin/Askalono.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import org.ossreviewtoolkit.scanner.CommandLinePathScannerWrapper
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.scanner.ScanException
import org.ossreviewtoolkit.scanner.ScannerMatcher
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.Os
Expand All @@ -44,14 +45,17 @@ private const val CONFIDENCE_NOTICE = "Confidence threshold not high enough for

private val JSON = Json { ignoreUnknownKeys = true }

class Askalono internal constructor(name: String, private val options: Options) : CommandLinePathScannerWrapper(name) {
class Factory : ScannerWrapperFactory<Askalono>("Askalono") {
override fun create(options: Options) = Askalono(type, options)
class Askalono internal constructor(name: String, private val matcherConfig: ScannerMatcherConfig) :
CommandLinePathScannerWrapper(name) {
class Factory : ScannerWrapperFactory<Unit>("Askalono") {
override fun create(config: Unit, matcherConfig: ScannerMatcherConfig) = Askalono(type, matcherConfig)

override fun parseConfig(options: Options, secrets: Options) = Unit
}

override val configuration = ""

override val matcher by lazy { ScannerMatcher.create(details, options) }
override val matcher by lazy { ScannerMatcher.create(details, matcherConfig) }

override fun command(workingDir: File?) =
listOfNotNull(workingDir, if (Os.isWindows) "askalono.exe" else "askalono").joinToString(File.separator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ package org.ossreviewtoolkit.plugins.scanners.boyterlc

import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.scanners.AbstractPathScannerWrapperFunTest

class BoyterLcFunTest : AbstractPathScannerWrapperFunTest() {
override val scanner = BoyterLc("BoyterLc", emptyMap())
override val scanner = BoyterLc("BoyterLc", ScannerMatcherConfig.EMPTY)

override val expectedFileLicenses = listOf(
LicenseFinding("Apache-2.0", TextLocation("LICENSE", TextLocation.UNKNOWN_LINE), 0.98388565f),
Expand Down
12 changes: 8 additions & 4 deletions plugins/scanners/boyterlc/src/main/kotlin/BoyterLc.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import org.ossreviewtoolkit.scanner.CommandLinePathScannerWrapper
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.scanner.ScanException
import org.ossreviewtoolkit.scanner.ScannerMatcher
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.Os
Expand All @@ -43,21 +44,24 @@ import org.ossreviewtoolkit.utils.ort.createOrtTempDir

private val JSON = Json { ignoreUnknownKeys = true }

class BoyterLc internal constructor(name: String, private val options: Options) : CommandLinePathScannerWrapper(name) {
class BoyterLc internal constructor(name: String, private val matcherConfig: ScannerMatcherConfig) :
CommandLinePathScannerWrapper(name) {
companion object {
val CONFIGURATION_OPTIONS = listOf(
"--confidence", "0.95", // Cut-off value to only get most relevant matches.
"--format", "json"
)
}

class Factory : ScannerWrapperFactory<BoyterLc>("BoyterLc") {
override fun create(options: Options) = BoyterLc(type, options)
class Factory : ScannerWrapperFactory<Unit>("BoyterLc") {
override fun create(config: Unit, matcherConfig: ScannerMatcherConfig) = BoyterLc(type, matcherConfig)

override fun parseConfig(options: Options, secrets: Options) = Unit
}

override val configuration = CONFIGURATION_OPTIONS.joinToString(" ")

override val matcher by lazy { ScannerMatcher.create(details, options) }
override val matcher by lazy { ScannerMatcher.create(details, matcherConfig) }

override fun command(workingDir: File?) =
listOfNotNull(workingDir, if (Os.isWindows) "lc.exe" else "lc").joinToString(File.separator)
Expand Down
9 changes: 6 additions & 3 deletions plugins/scanners/fossid/src/main/kotlin/FossId.kt
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import org.ossreviewtoolkit.scanner.PackageScannerWrapper
import org.ossreviewtoolkit.scanner.ProvenanceScannerWrapper
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.scanner.ScannerMatcher
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.enumSetOf
Expand Down Expand Up @@ -168,14 +169,16 @@ class FossId internal constructor(
)
}

class Factory : ScannerWrapperFactory<FossId>("FossId") {
override fun create(options: Options) = FossId(type, FossIdConfig.create(options))
class Factory : ScannerWrapperFactory<FossIdConfig>("FossId") {
override fun create(config: FossIdConfig, matcherConfig: ScannerMatcherConfig) = FossId(type, config)

override fun parseConfig(options: Options, secrets: Options) = FossIdConfig.create(options)
}

/**
* The qualifier of a scan when delta scans are enabled.
*/
internal enum class DeltaTag {
enum class DeltaTag {
/**
* Qualifier used when there is no scan and the first one is created.
*/
Expand Down
2 changes: 1 addition & 1 deletion plugins/scanners/fossid/src/main/kotlin/FossIdConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ import org.ossreviewtoolkit.utils.common.Options
*
* every repository URL would be added credentials. Mappings are applied in the order they are defined.
*/
internal data class FossIdConfig(
data class FossIdConfig(
/** The URL where the FossID service is running. */
val serverUrl: String,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import org.apache.logging.log4j.kotlin.logger
* * **deltaTag** (scan code only): If delta scans is enabled, this qualifies the scan as an *origin* scan or a *delta*
* scan.
*/
internal class FossIdNamingProvider(
class FossIdNamingProvider(
private val namingProjectPattern: String?,
private val namingScanPattern: String?,
private val namingConventionVariables: Map<String, String>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import org.ossreviewtoolkit.utils.ort.requestPasswordAuthentication
* The URLs used by FossId can sometimes be different from the normal package URLs. For instance, credentials may need
* to be added, or a different protocol may be used. This class takes care of such mappings.
*/
internal class FossIdUrlProvider private constructor(
class FossIdUrlProvider private constructor(
/**
* The URL mapping. URLs matched by a key [Regex] are replaced by the URL in the value. The replacement string can
* refer to matched groups of the [Regex]. It can also contain the variables [VAR_USERNAME] and [VAR_PASSWORD] to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ package org.ossreviewtoolkit.plugins.scanners.licensee

import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.scanners.AbstractPathScannerWrapperFunTest
import org.ossreviewtoolkit.utils.test.ExpensiveTag

class LicenseeFunTest : AbstractPathScannerWrapperFunTest(setOf(ExpensiveTag)) {
override val scanner = Licensee("Licensee", emptyMap())
override val scanner = Licensee("Licensee", ScannerMatcherConfig.EMPTY)

override val expectedFileLicenses = listOf(
LicenseFinding("Apache-2.0", TextLocation("LICENSE", TextLocation.UNKNOWN_LINE), 100.0f)
Expand Down
12 changes: 8 additions & 4 deletions plugins/scanners/licensee/src/main/kotlin/Licensee.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import org.ossreviewtoolkit.scanner.CommandLinePathScannerWrapper
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.scanner.ScanException
import org.ossreviewtoolkit.scanner.ScannerMatcher
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.Os
Expand All @@ -45,18 +46,21 @@ private val JSON = Json {
namingStrategy = JsonNamingStrategy.SnakeCase
}

class Licensee internal constructor(name: String, private val options: Options) : CommandLinePathScannerWrapper(name) {
class Licensee internal constructor(name: String, private val matcherConfig: ScannerMatcherConfig) :
CommandLinePathScannerWrapper(name) {
companion object {
val CONFIGURATION_OPTIONS = listOf("--json")
}

class Factory : ScannerWrapperFactory<Licensee>("Licensee") {
override fun create(options: Options) = Licensee(type, options)
class Factory : ScannerWrapperFactory<Unit>("Licensee") {
override fun create(config: Unit, matcherConfig: ScannerMatcherConfig) = Licensee(type, matcherConfig)

override fun parseConfig(options: Options, secrets: Options) = Unit
}

override val configuration = CONFIGURATION_OPTIONS.joinToString(" ")

override val matcher by lazy { ScannerMatcher.create(details, options) }
override val matcher by lazy { ScannerMatcher.create(details, matcherConfig) }

override fun command(workingDir: File?) =
listOfNotNull(workingDir, if (Os.isWindows) "licensee.bat" else "licensee").joinToString(File.separator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ import io.kotest.matchers.string.startWith

import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.scanners.AbstractPathScannerWrapperFunTest
import org.ossreviewtoolkit.utils.ort.createOrtTempDir
import org.ossreviewtoolkit.utils.spdx.getLicenseText
import org.ossreviewtoolkit.utils.test.ExpensiveTag

class ScanCodeScannerFunTest : AbstractPathScannerWrapperFunTest(setOf(ExpensiveTag)) {
override val scanner = ScanCode("ScanCode", emptyMap())
override val scanner = ScanCode("ScanCode", ScanCodeConfig.EMPTY, ScannerMatcherConfig.EMPTY)

override val expectedFileLicenses = listOf(
LicenseFinding("Apache-2.0", TextLocation("LICENSE", 1, 187), 100.0f),
Expand Down
23 changes: 17 additions & 6 deletions plugins/scanners/scancode/src/main/kotlin/ScanCode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.ossreviewtoolkit.scanner.CommandLinePathScannerWrapper
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.scanner.ScanResultsStorage
import org.ossreviewtoolkit.scanner.ScannerMatcher
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.scanner.ScannerWrapperFactory
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.Os
Expand All @@ -57,7 +58,14 @@ import org.semver4j.Semver
* * **"commandLineNonConfig":** Command line options that do not modify the result and should therefore not be
* considered in [configuration], like "--processes". Defaults to [DEFAULT_NON_CONFIGURATION_OPTIONS].
*/
class ScanCode internal constructor(name: String, private val options: Options) : CommandLinePathScannerWrapper(name) {
class ScanCode internal constructor(
name: String,
config: ScanCodeConfig,
private val matcherConfig: ScannerMatcherConfig
) : CommandLinePathScannerWrapper(name) {
// This constructor is required by the `RequirementsCommand`.
constructor(name: String, matcherConfig: ScannerMatcherConfig) : this(name, ScanCodeConfig.EMPTY, matcherConfig)

companion object {
const val SCANNER_NAME = "ScanCode"

Expand Down Expand Up @@ -91,11 +99,14 @@ class ScanCode internal constructor(name: String, private val options: Options)
}
}

class Factory : ScannerWrapperFactory<ScanCode>(SCANNER_NAME) {
override fun create(options: Options) = ScanCode(type, options)
class Factory : ScannerWrapperFactory<ScanCodeConfig>(SCANNER_NAME) {
override fun create(config: ScanCodeConfig, matcherConfig: ScannerMatcherConfig) =
ScanCode(type, config, matcherConfig)

override fun parseConfig(options: Options, secrets: Options) = ScanCodeConfig.create(options)
}

override val matcher by lazy { ScannerMatcher.create(details, options) }
override val matcher by lazy { ScannerMatcher.create(details, matcherConfig) }

override val configuration by lazy {
buildList {
Expand All @@ -104,8 +115,8 @@ class ScanCode internal constructor(name: String, private val options: Options)
}.joinToString(" ")
}

private val configurationOptions = options["commandLine"]?.splitOnWhitespace() ?: DEFAULT_CONFIGURATION_OPTIONS
private val nonConfigurationOptions = options["commandLineNonConfig"]?.splitOnWhitespace()
private val configurationOptions = config.commandLine?.splitOnWhitespace() ?: DEFAULT_CONFIGURATION_OPTIONS
private val nonConfigurationOptions = config.commandLineNonConfig?.splitOnWhitespace()
?: DEFAULT_NON_CONFIGURATION_OPTIONS

internal fun getCommandLineOptions(version: String) =
Expand Down
37 changes: 37 additions & 0 deletions plugins/scanners/scancode/src/main/kotlin/ScanCodeConfig.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (C) 2023 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
*/

package org.ossreviewtoolkit.plugins.scanners.scancode

import org.ossreviewtoolkit.utils.common.Options

data class ScanCodeConfig(
val commandLine: String?,
val commandLineNonConfig: String?
) {
companion object {
val EMPTY = ScanCodeConfig(null, null)

private const val COMMAND_LINE_PROPERTY = "commandLine"
private const val COMMAND_LINE_NON_CONFIG_PROPERTY = "commandLineNonConfig"

fun create(options: Options) =
ScanCodeConfig(options[COMMAND_LINE_PROPERTY], options[COMMAND_LINE_NON_CONFIG_PROPERTY])
Comment on lines +31 to +35
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Strictly speaking IMO the knowledge about the names of the keys in the options map is not something the ScanCodeConfig should need to care about. All existing implementations inline this to parseConfig, which I believe is the better place, as it "glues" options to ScanCodeConfig.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now I see where this probably comes from: The ScanOss implementation already did it that way. However, I'd then prefer to inline this for ScanOss in a preceding commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ScanOss implementation was actually inspired by the FossId config. Looking at the number of properties the FossIdConfig supports in this case there is a clear benefit in moving the parsing to a separate file. What would you think of keeping the implementation as is for now and to discuss in the next dev meeting if we want to align on one approach?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discuss in the next dev meeting

I'm fine with that, it was only a nit anyway. Please put this on the agenda then.

}
}
30 changes: 17 additions & 13 deletions plugins/scanners/scancode/src/test/kotlin/ScanCodeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ import java.io.File

import org.ossreviewtoolkit.model.PackageType
import org.ossreviewtoolkit.scanner.ScanContext
import org.ossreviewtoolkit.scanner.ScannerMatcherConfig
import org.ossreviewtoolkit.utils.common.ProcessCapture

class ScanCodeTest : WordSpec({
val scanner = ScanCode("ScanCode", emptyMap())
val scanner = ScanCode("ScanCode", ScanCodeConfig.EMPTY, ScannerMatcherConfig.EMPTY)

"configuration" should {
"return the default values if the scanner configuration is empty" {
Expand All @@ -48,10 +49,11 @@ class ScanCodeTest : WordSpec({
"return the non-config values from the scanner configuration" {
val scannerWithConfig = ScanCode(
"ScanCode",
mapOf(
"commandLine" to "--command --line",
"commandLineNonConfig" to "--commandLineNonConfig"
)
ScanCodeConfig(
commandLine = "--command --line",
commandLineNonConfig = "--commandLineNonConfig"
),
ScannerMatcherConfig.EMPTY
)

scannerWithConfig.configuration shouldBe "--command --line --json-pp"
Expand All @@ -69,10 +71,11 @@ class ScanCodeTest : WordSpec({
"contain the values from the scanner configuration" {
val scannerWithConfig = ScanCode(
"ScanCode",
mapOf(
"commandLine" to "--command --line",
"commandLineNonConfig" to "--commandLineNonConfig"
)
ScanCodeConfig(
commandLine = "--command --line",
commandLineNonConfig = "--commandLineNonConfig"
),
ScannerMatcherConfig.EMPTY
)

scannerWithConfig.getCommandLineOptions("31.2.4").joinToString(" ") shouldBe
Expand All @@ -82,10 +85,11 @@ class ScanCodeTest : WordSpec({
"be handled correctly when containing multiple spaces" {
val scannerWithConfig = ScanCode(
"ScanCode",
mapOf(
"commandLine" to " --command --line ",
"commandLineNonConfig" to " -n -c "
)
ScanCodeConfig(
commandLine = " --command --line ",
commandLineNonConfig = " -n -c "
),
ScannerMatcherConfig.EMPTY
)

scannerWithConfig.getCommandLineOptions("31.2.4") shouldBe listOf("--command", "--line", "-n", "-c")
Expand Down
Loading
Loading