Skip to content

Commit

Permalink
Allow plugins to contribute to the default configuration (#4315)
Browse files Browse the repository at this point in the history
* Add missing tests

* Remove deprecated code

* Get Spec in DefaultConfigProvider

* Remove DefaultConfig

* Concat configurations

* Apply suggestions from code review

Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Chao Zhang <zhangchao6865@gmail.com>

Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>
Co-authored-by: Chao Zhang <zhangchao6865@gmail.com>
  • Loading branch information
3 people committed Jan 2, 2022
1 parent d993ce6 commit a7184d1
Show file tree
Hide file tree
Showing 21 changed files with 203 additions and 71 deletions.
Expand Up @@ -20,7 +20,7 @@ class CliRunner : DetektCli {

val specialRunner = when {
arguments.showVersion -> VersionPrinter(outputChannel)
arguments.generateConfig -> ConfigExporter(arguments, outputChannel)
arguments.generateConfig -> ConfigExporter(arguments, outputChannel, errorChannel)
arguments.printAst -> AstPrinter(arguments, outputChannel)
else -> null
}
Expand Down

This file was deleted.

Expand Up @@ -54,7 +54,7 @@ fun buildRunner(
val arguments = parseArguments(args)
return when {
arguments.showVersion -> VersionPrinter(outputPrinter)
arguments.generateConfig -> ConfigExporter(arguments, outputPrinter)
arguments.generateConfig -> ConfigExporter(arguments, outputPrinter, errorPrinter)
arguments.printAst -> AstPrinter(arguments, outputPrinter)
else -> Runner(arguments, outputPrinter, errorPrinter)
}
Expand Down
Expand Up @@ -2,16 +2,19 @@ package io.gitlab.arturbosch.detekt.cli.runners

import io.github.detekt.tooling.api.DefaultConfigurationProvider
import io.gitlab.arturbosch.detekt.cli.CliArgs
import io.gitlab.arturbosch.detekt.cli.createSpec
import java.nio.file.Paths

class ConfigExporter(
private val arguments: CliArgs,
private val outputPrinter: Appendable
private val outputPrinter: Appendable,
private val errorPrinter: Appendable,
) : Executable {

override fun execute() {
val configPath = Paths.get(arguments.config ?: "detekt.yml")
DefaultConfigurationProvider.load().copy(configPath)
val spec = arguments.createSpec(outputPrinter, errorPrinter)
DefaultConfigurationProvider.load(spec).copy(configPath)
outputPrinter.appendLine("Successfully copied default config to ${configPath.toAbsolutePath()}")
}
}
Expand Up @@ -16,7 +16,7 @@ class ConfigExporterSpec : Spek({
val tmpConfig = createTempFileForTest("ConfigPrinterSpec", ".yml")
val cliArgs = parseArguments(arrayOf("--config", tmpConfig.toString()))

ConfigExporter(cliArgs, NullPrintStream()).execute()
ConfigExporter(cliArgs, NullPrintStream(), NullPrintStream()).execute()

assertThat(Files.readAllLines(tmpConfig)).isNotEmpty
}
Expand Down
Expand Up @@ -15,12 +15,12 @@ import io.gitlab.arturbosch.detekt.api.internal.whichDetekt
import io.gitlab.arturbosch.detekt.api.internal.whichJava
import io.gitlab.arturbosch.detekt.api.internal.whichOS
import io.gitlab.arturbosch.detekt.core.config.AllRulesConfig
import io.gitlab.arturbosch.detekt.core.config.DefaultConfig
import io.gitlab.arturbosch.detekt.core.config.DisabledAutoCorrectConfig
import io.gitlab.arturbosch.detekt.core.rules.associateRuleIdsToRuleSetIds
import io.gitlab.arturbosch.detekt.core.rules.isActive
import io.gitlab.arturbosch.detekt.core.rules.shouldAnalyzeFile
import io.gitlab.arturbosch.detekt.core.suppressors.getSuppressors
import io.gitlab.arturbosch.detekt.core.tooling.getDefaultConfiguration
import org.jetbrains.kotlin.config.languageVersionSettings
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.resolve.BindingContext
Expand Down Expand Up @@ -171,13 +171,13 @@ internal fun ProcessingSpec.workaroundConfiguration(config: Config): Config = wi
}

if (rulesSpec.activateAllRules) {
val defaultConfig = DefaultConfig.newInstance()
val defaultConfig = getDefaultConfiguration()
declaredConfig = AllRulesConfig(declaredConfig ?: defaultConfig, defaultConfig)
}

if (!rulesSpec.autoCorrect) {
declaredConfig = DisabledAutoCorrectConfig(declaredConfig ?: DefaultConfig.newInstance())
declaredConfig = DisabledAutoCorrectConfig(declaredConfig ?: getDefaultConfiguration())
}

return declaredConfig ?: DefaultConfig.newInstance()
return declaredConfig ?: getDefaultConfiguration()
}
@@ -1,6 +1,7 @@
package io.gitlab.arturbosch.detekt.core.config

import io.github.detekt.tooling.api.InvalidConfig
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.ConfigValidator
import io.gitlab.arturbosch.detekt.api.Notification
import io.gitlab.arturbosch.detekt.core.NL
Expand All @@ -9,12 +10,13 @@ import io.gitlab.arturbosch.detekt.core.extensions.loadExtensions
import io.gitlab.arturbosch.detekt.core.reporting.red
import io.gitlab.arturbosch.detekt.core.reporting.yellow

internal fun checkConfiguration(settings: ProcessingSettings) {
internal fun checkConfiguration(settings: ProcessingSettings, baseline: Config) {
val props = settings.config.subConfig("config")
val shouldValidate = props.valueOrDefault("validation", true)

if (shouldValidate) {
val validators = loadExtensions<ConfigValidator>(settings) + DefaultPropertiesConfigValidator(settings)
val validators =
loadExtensions<ConfigValidator>(settings) + DefaultPropertiesConfigValidator(settings, baseline)
val notifications = validators.flatMap { it.validate(settings.config) }
notifications.map(Notification::message).forEach(settings::info)
val errors = notifications.filter(Notification::isError)
Expand Down
Expand Up @@ -4,6 +4,7 @@ import io.github.detekt.tooling.api.spec.ConfigSpec
import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.github.detekt.utils.openSafeStream
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.core.tooling.getDefaultConfiguration
import java.net.URI
import java.net.URL
import java.nio.file.FileSystemNotFoundException
Expand All @@ -19,13 +20,13 @@ internal fun ProcessingSpec.loadConfiguration(): Config = with(configSpec) {

if (useDefaultConfig) {
declaredConfig = if (declaredConfig == null) {
DefaultConfig.newInstance()
getDefaultConfiguration()
} else {
CompositeConfig(declaredConfig, DefaultConfig.newInstance())
CompositeConfig(declaredConfig, getDefaultConfiguration())
}
}

return declaredConfig ?: DefaultConfig.newInstance()
return declaredConfig ?: getDefaultConfiguration()
}

private fun parseResourceConfig(urls: Collection<URL>): Config =
Expand Down

This file was deleted.

Expand Up @@ -9,7 +9,8 @@ import io.gitlab.arturbosch.detekt.core.ProcessingSettings
import io.gitlab.arturbosch.detekt.core.rules.RuleSetLocator

class DefaultPropertiesConfigValidator(
private val settings: ProcessingSettings
private val settings: ProcessingSettings,
private val baseline: Config,
) : ConfigValidator {

override fun validate(config: Config): Collection<Notification> {
Expand All @@ -21,6 +22,6 @@ class DefaultPropertiesConfigValidator(
val allExcludes = "$configExcludes,$DEFAULT_PROPERTY_EXCLUDES,$pluginExcludes"
return CommaSeparatedPattern(allExcludes).mapToRegex()
}
return validateConfig(config, DefaultConfig.newInstance(), patterns())
return validateConfig(config, baseline, patterns())
}
}
Expand Up @@ -22,17 +22,22 @@ class AnalysisFacade(
private val spec: ProcessingSpec
) : Detekt {

override fun run(): AnalysisResult = runAnalysis { DefaultLifecycle(it, inputPathsToKtFiles) }
override fun run(): AnalysisResult = runAnalysis {
DefaultLifecycle(spec.getDefaultConfiguration(), it, inputPathsToKtFiles)
}

override fun run(path: Path): AnalysisResult =
runAnalysis { DefaultLifecycle(it, pathToKtFile(path)) }
runAnalysis { DefaultLifecycle(spec.getDefaultConfiguration(), it, pathToKtFile(path)) }

override fun run(sourceCode: String, filename: String): AnalysisResult =
runAnalysis { DefaultLifecycle(it, contentToKtFile(sourceCode, Paths.get(filename))) }
runAnalysis {
DefaultLifecycle(spec.getDefaultConfiguration(), it, contentToKtFile(sourceCode, Paths.get(filename)))
}

override fun run(files: Collection<KtFile>, bindingContext: BindingContext): AnalysisResult =
runAnalysis {
DefaultLifecycle(
spec.getDefaultConfiguration(),
it,
parsingStrategy = { files.toList() },
bindingProvider = { bindingContext }
Expand Down
@@ -1,19 +1,55 @@
package io.gitlab.arturbosch.detekt.core.tooling

import io.github.detekt.tooling.api.DefaultConfigurationProvider
import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.github.detekt.utils.getSafeResourceAsStream
import io.github.detekt.utils.openSafeStream
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.core.config.DefaultConfig
import io.gitlab.arturbosch.detekt.core.config.YamlConfig
import io.gitlab.arturbosch.detekt.core.settings.ExtensionFacade
import java.io.ByteArrayInputStream
import java.io.ByteArrayOutputStream
import java.io.InputStream
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.StandardCopyOption

class DefaultConfigProvider : DefaultConfigurationProvider {
private lateinit var spec: ProcessingSpec

override fun get(): Config = DefaultConfig.newInstance()
override fun init(spec: ProcessingSpec) {
this.spec = spec
}

override fun get(): Config = spec.getDefaultConfiguration()

override fun copy(targetLocation: Path) {
val configUrl = javaClass.getResource("/${DefaultConfig.RESOURCE_NAME}")!!
Files.copy(configUrl.openSafeStream(), targetLocation, StandardCopyOption.REPLACE_EXISTING)
Files.copy(configInputStream(spec), targetLocation, StandardCopyOption.REPLACE_EXISTING)
}
}

private fun configInputStream(spec: ProcessingSpec): InputStream {
val outputStream = ByteArrayOutputStream()

requireNotNull(spec.javaClass.getSafeResourceAsStream("/default-detekt-config.yml"))
.use { it.copyTo(outputStream) }

ExtensionFacade(spec.extensionsSpec).pluginLoader
.getResourcesAsStream("config/config.yml")
.forEach { inputStream ->
outputStream.bufferedWriter().append('\n').flush()
inputStream.use { it.copyTo(outputStream) }
}

return ByteArrayInputStream(outputStream.toByteArray())
}

private fun ClassLoader.getResourcesAsStream(name: String): Sequence<InputStream> {
return getResources(name)
.asSequence()
.map { it.openSafeStream() }
}

fun ProcessingSpec.getDefaultConfiguration(): Config {
return YamlConfig.load(configInputStream(this).reader())
}
@@ -1,5 +1,6 @@
package io.gitlab.arturbosch.detekt.core.tooling

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.FileProcessListener
import io.gitlab.arturbosch.detekt.api.Finding
Expand All @@ -22,6 +23,7 @@ import org.jetbrains.kotlin.resolve.BindingContext

internal interface Lifecycle {

val baselineConfig: Config
val settings: ProcessingSettings
val parsingStrategy: ParsingStrategy
val bindingProvider: (files: List<KtFile>) -> BindingContext
Expand All @@ -32,7 +34,7 @@ internal interface Lifecycle {
private fun <R> measure(phase: Phase, block: () -> R): R = settings.getOrCreateMonitor().measure(phase, block)

fun analyze(): Detektion {
measure(Phase.ValidateConfig) { checkConfiguration(settings) }
measure(Phase.ValidateConfig) { checkConfiguration(settings, baselineConfig) }
val filesToAnalyze = measure(Phase.Parsing) { parsingStrategy.invoke(settings) }
val bindingContext = measure(Phase.Binding) { bindingProvider.invoke(filesToAnalyze) }
val (processors, ruleSets) = measure(Phase.LoadingExtensions) {
Expand All @@ -57,6 +59,7 @@ internal interface Lifecycle {
}

internal class DefaultLifecycle(
override val baselineConfig: Config,
override val settings: ProcessingSettings,
override val parsingStrategy: ParsingStrategy,
override val bindingProvider: (files: List<KtFile>) -> BindingContext =
Expand Down
Expand Up @@ -17,6 +17,7 @@ import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.core.tooling.AnalysisFacade
import io.gitlab.arturbosch.detekt.core.tooling.DefaultLifecycle
import io.gitlab.arturbosch.detekt.core.tooling.inputPathsToKtFiles
import io.mockk.mockk
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.psi.KtAnnotation
import org.jetbrains.kotlin.psi.KtFile
Expand Down Expand Up @@ -56,6 +57,7 @@ class TopLevelAutoCorrectSpec : Spek({

AnalysisFacade(spec).runAnalysis {
DefaultLifecycle(
mockk(),
it,
inputPathsToKtFiles,
processorsProvider = { listOf(contentChangedListener) },
Expand Down
Expand Up @@ -8,7 +8,9 @@ import io.gitlab.arturbosch.detekt.api.Notification
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification
import io.gitlab.arturbosch.detekt.core.createNullLoggingSpec
import io.gitlab.arturbosch.detekt.core.createProcessingSettings
import io.gitlab.arturbosch.detekt.core.tooling.getDefaultConfiguration
import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent
import org.assertj.core.api.Assertions.assertThatCode
import org.spekframework.spek2.Spek
Expand All @@ -19,6 +21,7 @@ class SupportConfigValidationSpec : Spek({
describe("support config validation") {

val testDir by memoized { createTempDirectoryForTest("detekt-sample") }
val spec by memoized { createNullLoggingSpec {} }

it("fails when unknown properties are found") {
val config = yamlConfigFromContent(
Expand All @@ -35,7 +38,7 @@ class SupportConfigValidationSpec : Spek({
"""
)
createProcessingSettings(testDir, config).use {
assertThatCode { checkConfiguration(it) }
assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) }
.isInstanceOf(InvalidConfig::class.java)
.hasMessageContaining("Run failed with 1 invalid config property.")
.hasMessageContaining("my_additional_properties")
Expand All @@ -53,7 +56,7 @@ class SupportConfigValidationSpec : Spek({
"""
)
createProcessingSettings(testDir, config).use {
assertThatCode { checkConfiguration(it) }
assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) }
.isInstanceOf(InvalidConfig::class.java)
.hasMessageContaining("Run failed with 1 invalid config property.")
}
Expand Down Expand Up @@ -82,7 +85,8 @@ class SupportConfigValidationSpec : Spek({
"""
)
createProcessingSettings(testDir, config).use {
assertThatCode { checkConfiguration(it) }.doesNotThrowAnyException()
assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) }
.doesNotThrowAnyException()
}
}
}
Expand Down

0 comments on commit a7184d1

Please sign in to comment.