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

Allow plugins to contribute to the default configuration #4315

Merged
merged 7 commits into from Jan 2, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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")
Copy link
Member

Choose a reason for hiding this comment

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

Do you think this path config/config.yml should be configurable? If not, we probably need to add documentation in

Copy link
Member Author

@BraisGabin BraisGabin Dec 2, 2021

Choose a reason for hiding this comment

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

I don't think this could be configurable.

We need to document this. But should we document it right now? Or should we wait to release it first? And probably we can create a tool to generate this files from our annotations. The same that we have already for detekt but working for anyone.

Copy link
Member

Choose a reason for hiding this comment

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

If not configurable, should this be exposed in detekt-api so that other plugin authors could refer to this constant. (the prefixing config/ does look a bit awkward)

.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