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

Warn about enabled rules that are not going to run #5226

Merged
merged 5 commits into from Sep 10, 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 @@ -52,6 +52,9 @@ internal class Analyzer(
} else {
runSync(ktFiles, bindingContext, compilerResources)
}
if (bindingContext == BindingContext.EMPTY) {
warnAboutEnabledRequiresTypeResolutionRules(settings::info)
}

val findingsPerRuleSet = HashMap<RuleSetId, List<Finding>>()
for (findings in findingsPerFile) {
Expand Down Expand Up @@ -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 }
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
.flatMap { (ruleSet, _) -> ruleSet.rules.asSequence() }
.filter { rule -> (rule as? Rule)?.active == true }
.filter { rule -> rule::class.hasAnnotation<RequiresTypeResolution>() }
.forEach { rule ->
log("The rule '${rule.ruleId}' requires type resolution but it was run without it.")
}
}
}

private fun filterSuppressedFindings(rule: BaseRule, bindingContext: BindingContext): List<Finding> {
Expand Down
@@ -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
Expand Down Expand Up @@ -44,13 +45,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))) } }
Expand All @@ -65,70 +65,101 @@ 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()).isEqualTo(
"The rule 'RequiresTypeResolutionMaxLineLength' requires type resolution but it was run without it.\n"
)
}

@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()).isEqualTo(
"The rule 'RequiresTypeResolutionMaxLineLength' requires type resolution but it was run without it.\n"
)
}

@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()
}
}
}
Expand Down
Expand Up @@ -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
Expand All @@ -18,16 +19,36 @@ fun createProcessingSettings(
inputPath: Path? = null,
config: Config = Config.empty,
reportPaths: Collection<ReportsSpec.Report> = emptyList(),
spec: ProcessingSpec = createNullLoggingSpec {
outputChannel: PrintStream = NullPrintStream(),
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
init: ProcessingSpecBuilder.() -> Unit = { /* no-op */ },
): ProcessingSettings {
val spec = ProcessingSpec {
project {
inputPaths = inputPath?.let(::listOf).orEmpty()
inputPaths = listOfNotNull(inputPath)
}
logging {
this.outputChannel = outputChannel
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)
}
) = 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()
Expand All @@ -42,7 +63,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() {
Expand Down
Expand Up @@ -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()
}
Expand Down
@@ -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
Expand All @@ -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
}
)
}