From e3b61e90f2060599f74f678fafc707af1f38e8e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sun, 21 Aug 2022 01:04:23 +0200 Subject: [PATCH 1/5] Avoid unnecesary usage of null --- .../arturbosch/detekt/core/ProcessingSettingsFactory.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt index 3df4909f54b..c2c077536e8 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt @@ -22,12 +22,14 @@ fun createProcessingSettings( project { inputPaths = inputPath?.let(::listOf).orEmpty() } - } + }, ) = ProcessingSettings(spec, config).apply { register(DETEKT_OUTPUT_REPORT_PATHS_KEY, reportPaths) } -fun createNullLoggingSpec(init: (ProcessingSpecBuilder.() -> Unit)? = null): ProcessingSpec = +fun createNullLoggingSpec( + init: ProcessingSpecBuilder.() -> Unit = { /* no-op */ }, +): ProcessingSpec = ProcessingSpec { logging { outputChannel = NullPrintStream() @@ -42,7 +44,7 @@ fun createNullLoggingSpec(init: (ProcessingSpecBuilder.() -> Unit)? = null): Pro execution { executorService = DirectExecutor() // run in the same thread } - init?.invoke(this) + init.invoke(this) } class DirectExecutor : AbstractExecutorService() { From 3d607ef8ae2dc823288eb41c648cce96fb7718ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sun, 21 Aug 2022 11:00:24 +0200 Subject: [PATCH 2/5] Simplify code --- .../gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt index c2c077536e8..74647a4ae5b 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt @@ -20,7 +20,7 @@ fun createProcessingSettings( reportPaths: Collection = emptyList(), spec: ProcessingSpec = createNullLoggingSpec { project { - inputPaths = inputPath?.let(::listOf).orEmpty() + inputPaths = listOfNotNull(inputPath) } }, ) = ProcessingSettings(spec, config).apply { From c37f3fad6978366c7739ae63db8993098623990e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sun, 21 Aug 2022 11:20:13 +0200 Subject: [PATCH 3/5] Improve createProcessingSettings --- .../arturbosch/detekt/core/AnalyzerSpec.kt | 11 ++++---- .../detekt/core/ProcessingSettingsFactory.kt | 25 ++++++++++++++++--- .../validation/CheckConfigurationSpec.kt | 9 +++---- .../core/settings/EnvironmentFacadeSpec.kt | 11 +++----- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt index 184736c7293..ffdcd78b978 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt @@ -44,13 +44,12 @@ class AnalyzerSpec(val env: KotlinCoreEnvironment) { val settings = createProcessingSettings( inputPath = testFile, config = yamlConfig("configs/config-value-type-wrong.yml"), - spec = createNullLoggingSpec { - execution { - parallelParsing = true - parallelAnalysis = true - } + ) { + execution { + parallelParsing = true + parallelAnalysis = true } - ) + } val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider()), emptyList()) assertThatThrownBy { settings.use { analyzer.run(listOf(compileForTest(testFile))) } } diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt index 74647a4ae5b..1dd94ae3da8 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt @@ -18,13 +18,30 @@ fun createProcessingSettings( inputPath: Path? = null, config: Config = Config.empty, reportPaths: Collection = emptyList(), - spec: ProcessingSpec = createNullLoggingSpec { + init: ProcessingSpecBuilder.() -> Unit = { /* no-op */ }, +): ProcessingSettings { + val spec = ProcessingSpec { project { inputPaths = listOfNotNull(inputPath) } - }, -) = ProcessingSettings(spec, config).apply { - register(DETEKT_OUTPUT_REPORT_PATHS_KEY, reportPaths) + logging { + outputChannel = NullPrintStream() + errorChannel = NullPrintStream() + } + config { + // Use an empty config in tests to be compatible with old ProcessingSettings config default. + // This was in particular done due to all console reports being active with an empty config. + // These outputs are used to assert test conditions. + configPaths = listOf(resourceAsPath("configs/empty.yml")) + } + execution { + executorService = DirectExecutor() // run in the same thread + } + init.invoke(this) + } + return ProcessingSettings(spec, config).apply { + register(DETEKT_OUTPUT_REPORT_PATHS_KEY, reportPaths) + } } fun createNullLoggingSpec( diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt index 43ba281b6ef..ba9f7c41fc7 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt @@ -31,12 +31,11 @@ class CheckConfigurationSpec { createProcessingSettings( testDir, config, - spec = createNullLoggingSpec { - config { - shouldValidateBeforeAnalysis = false - } + ) { + config { + shouldValidateBeforeAnalysis = false } - ).use { + }.use { assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) } .doesNotThrowAnyException() } diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/settings/EnvironmentFacadeSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/settings/EnvironmentFacadeSpec.kt index faadcadadf5..d483cd3d88a 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/settings/EnvironmentFacadeSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/settings/EnvironmentFacadeSpec.kt @@ -1,6 +1,5 @@ package io.gitlab.arturbosch.detekt.core.settings -import io.gitlab.arturbosch.detekt.core.createNullLoggingSpec import io.gitlab.arturbosch.detekt.core.createProcessingSettings import org.assertj.core.api.Assertions.assertThat import org.jetbrains.kotlin.konan.file.File @@ -22,10 +21,8 @@ class EnvironmentFacadeSpec { } } -private fun testSettings(classpath: String) = createProcessingSettings( - spec = createNullLoggingSpec { - compiler { - this.classpath = classpath - } +private fun testSettings(classpath: String) = createProcessingSettings { + compiler { + this.classpath = classpath } -) +} From a4eb3a9aaeea5323f0d617af0cdd0073c09fe720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sun, 21 Aug 2022 11:31:47 +0200 Subject: [PATCH 4/5] Add asserts about output --- .../arturbosch/detekt/core/AnalyzerSpec.kt | 40 ++++++++++++++++--- .../detekt/core/ProcessingSettingsFactory.kt | 4 +- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt index ffdcd78b978..5ed0993ef36 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt @@ -1,5 +1,6 @@ package io.gitlab.arturbosch.detekt.core +import io.github.detekt.test.utils.StringPrintStream import io.github.detekt.test.utils.compileForTest import io.gitlab.arturbosch.detekt.api.CodeSmell import io.gitlab.arturbosch.detekt.api.Config @@ -64,70 +65,97 @@ class AnalyzerSpec(val env: KotlinCoreEnvironment) { @Test fun `no findings`() { val testFile = path.resolve("Test.kt") - val settings = createProcessingSettings(testFile, yamlConfig("configs/config-value-type-correct.yml")) + val output = StringPrintStream() + val settings = createProcessingSettings( + testFile, + yamlConfig("configs/config-value-type-correct.yml"), + outputChannel = output, + ) val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider()), emptyList()) assertThat(settings.use { analyzer.run(listOf(compileForTest(testFile))) }.values.flatten()).isEmpty() + assertThat(output.toString()).isEmpty() } @Test fun `with findings`() { val testFile = path.resolve("Test.kt") - val settings = createProcessingSettings(testFile, yamlConfig("configs/config-value-type-correct.yml")) + val output = StringPrintStream() + val settings = createProcessingSettings( + testFile, + yamlConfig("configs/config-value-type-correct.yml"), + outputChannel = output, + ) val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider(30)), emptyList()) assertThat(settings.use { analyzer.run(listOf(compileForTest(testFile))) }.values.flatten()).hasSize(1) + assertThat(output.toString()).isEmpty() } @Test fun `with findings and context binding`() { val testFile = path.resolve("Test.kt") - val settings = createProcessingSettings(testFile, yamlConfig("configs/config-value-type-correct.yml")) + val output = StringPrintStream() + val settings = createProcessingSettings( + testFile, + yamlConfig("configs/config-value-type-correct.yml"), + outputChannel = output, + ) val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider(30)), emptyList()) val ktFile = compileForTest(testFile) val bindingContext = env.getContextForPaths(listOf(ktFile)) assertThat(settings.use { analyzer.run(listOf(ktFile), bindingContext) }.values.flatten()).hasSize(2) + assertThat(output.toString()).isEmpty() } @Test fun `with findings but ignored`() { val testFile = path.resolve("Test.kt") + val output = StringPrintStream() val settings = createProcessingSettings( testFile, - yamlConfig("configs/config-value-type-correct-ignore-annotated.yml") + yamlConfig("configs/config-value-type-correct-ignore-annotated.yml"), + outputChannel = output, ) val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider(18)), emptyList()) assertThat(settings.use { analyzer.run(listOf(compileForTest(testFile))) }.values.flatten()).isEmpty() + assertThat(output.toString()).isEmpty() } @Test fun `with faulty rule`() { val testFile = path.resolve("Test.kt") + val output = StringPrintStream() val settings = createProcessingSettings( testFile, - yamlConfig("configs/config-value-type-correct.yml") + yamlConfig("configs/config-value-type-correct.yml"), + outputChannel = output, ) val analyzer = Analyzer(settings, listOf(FaultyRuleSetProvider()), emptyList()) assertThatThrownBy { settings.use { analyzer.run(listOf(compileForTest(testFile))) } } .hasCauseInstanceOf(IllegalStateException::class.java) .hasMessageContaining("Location: ${FaultyRule::class.java.name}") + assertThat(output.toString()).isEmpty() } @Test fun `with faulty rule without stack trace`() { val testFile = path.resolve("Test.kt") + val output = StringPrintStream() val settings = createProcessingSettings( testFile, - yamlConfig("configs/config-value-type-correct.yml") + yamlConfig("configs/config-value-type-correct.yml"), + outputChannel = output, ) val analyzer = Analyzer(settings, listOf(FaultyRuleNoStackTraceSetProvider()), emptyList()) assertThatThrownBy { settings.use { analyzer.run(listOf(compileForTest(testFile))) } } .hasCauseInstanceOf(IllegalStateException::class.java) .hasMessageContaining("Location: ${null}") + assertThat(output.toString()).isEmpty() } } } diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt index 1dd94ae3da8..5a9cf1d80f5 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettingsFactory.kt @@ -7,6 +7,7 @@ import io.github.detekt.tooling.api.spec.ReportsSpec import io.github.detekt.tooling.dsl.ProcessingSpecBuilder import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.core.reporting.DETEKT_OUTPUT_REPORT_PATHS_KEY +import java.io.PrintStream import java.nio.file.Path import java.util.concurrent.AbstractExecutorService import java.util.concurrent.TimeUnit @@ -18,6 +19,7 @@ fun createProcessingSettings( inputPath: Path? = null, config: Config = Config.empty, reportPaths: Collection = emptyList(), + outputChannel: PrintStream = NullPrintStream(), init: ProcessingSpecBuilder.() -> Unit = { /* no-op */ }, ): ProcessingSettings { val spec = ProcessingSpec { @@ -25,7 +27,7 @@ fun createProcessingSettings( inputPaths = listOfNotNull(inputPath) } logging { - outputChannel = NullPrintStream() + this.outputChannel = outputChannel errorChannel = NullPrintStream() } config { From c0706e70047815eda7dcd90994f5d391fe951ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Tue, 16 Aug 2022 21:18:08 +0200 Subject: [PATCH 5/5] Warn about enabled rules that are not going to run --- .../io/gitlab/arturbosch/detekt/core/Analyzer.kt | 16 ++++++++++++++++ .../arturbosch/detekt/core/AnalyzerSpec.kt | 8 ++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt index 47b3bf2d3c4..44cc294f5bd 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt @@ -52,6 +52,9 @@ internal class Analyzer( } else { runSync(ktFiles, bindingContext, compilerResources) } + if (bindingContext == BindingContext.EMPTY) { + warnAboutEnabledRequiresTypeResolutionRules(settings::info) + } val findingsPerRuleSet = HashMap>() for (findings in findingsPerFile) { @@ -140,6 +143,19 @@ internal class Analyzer( return result } + + private fun warnAboutEnabledRequiresTypeResolutionRules(log: (String) -> Unit) { + providers.asSequence() + .map { it to config.subConfig(it.ruleSetId) } + .filter { (_, ruleSetConfig) -> ruleSetConfig.isActive() } + .map { (provider, ruleSetConfig) -> provider.instance(ruleSetConfig) to ruleSetConfig } + .flatMap { (ruleSet, _) -> ruleSet.rules.asSequence() } + .filter { rule -> (rule as? Rule)?.active == true } + .filter { rule -> rule::class.hasAnnotation() } + .forEach { rule -> + log("The rule '${rule.ruleId}' requires type resolution but it was run without it.") + } + } } private fun filterSuppressedFindings(rule: BaseRule, bindingContext: BindingContext): List { diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt index 5ed0993ef36..952149dfcca 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt @@ -74,7 +74,9 @@ class AnalyzerSpec(val env: KotlinCoreEnvironment) { val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider()), emptyList()) assertThat(settings.use { analyzer.run(listOf(compileForTest(testFile))) }.values.flatten()).isEmpty() - assertThat(output.toString()).isEmpty() + assertThat(output.toString()).isEqualTo( + "The rule 'RequiresTypeResolutionMaxLineLength' requires type resolution but it was run without it.\n" + ) } @Test @@ -89,7 +91,9 @@ class AnalyzerSpec(val env: KotlinCoreEnvironment) { val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider(30)), emptyList()) assertThat(settings.use { analyzer.run(listOf(compileForTest(testFile))) }.values.flatten()).hasSize(1) - assertThat(output.toString()).isEmpty() + assertThat(output.toString()).isEqualTo( + "The rule 'RequiresTypeResolutionMaxLineLength' requires type resolution but it was run without it.\n" + ) } @Test