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

Throw error instead of logging as error in analysis phase #3193

Merged
merged 3 commits into from
Nov 5, 2020
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
@@ -1,5 +1,6 @@
package io.gitlab.arturbosch.detekt.core

import io.github.detekt.psi.absolutePath
import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.FileProcessListener
Expand All @@ -10,9 +11,12 @@ import io.gitlab.arturbosch.detekt.api.RuleSetId
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
import io.gitlab.arturbosch.detekt.api.internal.BaseRule
import io.gitlab.arturbosch.detekt.api.internal.CompilerResources
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.DefaultConfig
import io.gitlab.arturbosch.detekt.core.config.DisabledAutoCorrectConfig
import io.gitlab.arturbosch.detekt.core.config.FailFastConfig
import io.gitlab.arturbosch.detekt.core.config.DefaultConfig
import io.gitlab.arturbosch.detekt.core.rules.IdMapping
import io.gitlab.arturbosch.detekt.core.rules.associateRuleIdsToRuleSetIds
import io.gitlab.arturbosch.detekt.core.rules.isActive
Expand All @@ -22,6 +26,8 @@ import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactoryImpl

private typealias FindingsResult = List<Map<RuleSetId, List<Finding>>>

internal class Analyzer(
private val settings: ProcessingSettings,
private val providers: List<RuleSetProvider>,
Expand Down Expand Up @@ -63,7 +69,7 @@ internal class Analyzer(
ktFiles.map { file ->
processors.forEach { it.onProcess(file, bindingContext) }
val findings = runCatching { analyze(file, bindingContext, compilerResources) }
.onFailure { settings.error(createErrorMessage(file, it), it) }
.onFailure { throwIllegalStateException(file, it) }
.getOrDefault(emptyMap())
processors.forEach { it.onProcessComplete(file, findings, bindingContext) }
findings
Expand All @@ -81,10 +87,7 @@ internal class Analyzer(
val findings = analyze(file, bindingContext, compilerResources)
processors.forEach { it.onProcessComplete(file, findings, bindingContext) }
findings
}.recover {
settings.error(createErrorMessage(file, it), it)
emptyMap()
}
}.recover { throwIllegalStateException(file, it) }
}
return awaitAll(tasks).filterNotNull()
}
Expand Down Expand Up @@ -132,6 +135,22 @@ private fun <T, U, R> Sequence<Pair<T, U>>.mapLeft(transform: (T, U) -> R): Sequ
return this.map { (first, second) -> transform(first, second) to second }
}

private fun MutableMap<String, List<Finding>>.mergeSmells(other: Map<String, List<Finding>>) {
for ((key, findings) in other.entries) {
merge(key, findings) { f1, f2 -> f1.plus(f2) }
}
}

private fun throwIllegalStateException(file: KtFile, error: Throwable): Nothing {
val message = """
Analyzing ${file.absolutePath()} led to an exception.
The original exception message was: ${error.localizedMessage}
Running detekt '${whichDetekt() ?: "unknown"}' on Java '${whichJava()}' on OS '${whichOS()}'
If the exception message does not help, please feel free to create an issue on our GitHub page.
""".trimIndent()
throw IllegalStateException(message, error)
}

internal fun ProcessingSpec.workaroundConfiguration(config: Config): Config = with(configSpec) {
var declaredConfig: Config? = when {
configPaths.isNotEmpty() -> config
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,10 @@
package io.gitlab.arturbosch.detekt.core

import io.github.detekt.psi.absolutePath
import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.api.RuleSetId
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 org.jetbrains.kotlin.psi.KtFile
import java.nio.file.Files
import java.nio.file.Path

fun Path.exists(): Boolean = Files.exists(this)
fun Path.isFile(): Boolean = Files.isRegularFile(this)
fun Path.isDirectory(): Boolean = Files.isDirectory(this)

fun MutableMap<String, List<Finding>>.mergeSmells(other: Map<String, List<Finding>>) {
for ((key, findings) in other.entries) {
merge(key, findings) { f1, f2 -> f1.plus(f2) }
}
}

typealias FindingsResult = List<Map<RuleSetId, List<Finding>>>

fun createErrorMessage(file: KtFile, error: Throwable): String =
"Analyzing '${file.absolutePath()}' led to an exception.\n" +
"The original exception message was: ${error.localizedMessage}\n" +
"Running detekt '${whichDetekt() ?: "unknown"}' on Java '${whichJava()}' on OS '${whichOS()}'.\n" +
"If the exception message does not help, please feel free to create an issue on our GitHub page."

val NL: String = System.lineSeparator()
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ internal interface Lifecycle {
}

val result = measure(Phase.Analyzer) {
val detektor = Analyzer(settings, ruleSets, processors)
val analyzer = Analyzer(settings, ruleSets, processors)
processors.forEach { it.onStart(filesToAnalyze, bindingContext) }
val findings: Map<RuleSetId, List<Finding>> = detektor.run(filesToAnalyze, bindingContext)
val findings: Map<RuleSetId, List<Finding>> = analyzer.run(filesToAnalyze, bindingContext)
val result: Detektion = DetektResult(findings.toSortedMap())
processors.forEach { it.onFinish(filesToAnalyze, result, bindingContext) }
result
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package io.gitlab.arturbosch.detekt.core

import io.github.detekt.test.utils.compileForTest
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.test.yamlConfig
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.jetbrains.kotlin.psi.KtFile
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
import java.util.concurrent.CompletionException

class AnalyzerSpec : Spek({

describe("exceptions during analyze()") {

it("analyze successfully when config has correct value type in config") {
val testFile = path.resolve("Test.kt")
val settings = createProcessingSettings(testFile, yamlConfig("configs/config-value-type-correct.yml"))
val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider()), emptyList())

assertThat(settings.use { analyzer.run(listOf(compileForTest(testFile))) }).isEmpty()
}

it("throw error explicitly when config has wrong value type in config") {
val testFile = path.resolve("Test.kt")
val settings = createProcessingSettings(testFile, yamlConfig("configs/config-value-type-wrong.yml"))
val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider()), emptyList())

assertThatThrownBy {
settings.use { analyzer.run(listOf(compileForTest(testFile))) }
}.isInstanceOf(IllegalStateException::class.java)
}

it("throw error explicitly in parallel when config has wrong value in config") {
val testFile = path.resolve("Test.kt")
val settings = createProcessingSettings(
inputPath = testFile,
config = yamlConfig("configs/config-value-type-wrong.yml"),
spec = createNullLoggingSpec {
execution {
parallelParsing = true
parallelAnalysis = true
}
}
)
val analyzer = Analyzer(settings, listOf(StyleRuleSetProvider()), emptyList())

assertThatThrownBy { settings.use { analyzer.run(listOf(compileForTest(testFile))) } }
.isInstanceOf(CompletionException::class.java)
.hasCauseInstanceOf(IllegalStateException::class.java)
}
}
})

private class StyleRuleSetProvider : RuleSetProvider {
override val ruleSetId: String = "style"
override fun instance(config: Config) = RuleSet(ruleSetId, listOf(MaxLineLength(config)))
}

private class MaxLineLength(config: Config) : Rule(config) {
override val issue = Issue(this::class.java.simpleName, Severity.Style, "", Debt.FIVE_MINS)
private val lengthThreshold: Int = valueOrDefault("maxLineLength", 100)
override fun visitKtFile(file: KtFile) {
super.visitKtFile(file)
for (line in file.text.splitToSequence(NL)) {
if (line.length > lengthThreshold) {
report(CodeSmell(issue, Entity.atPackageOrFirstDecl(file), issue.description))
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
style:
MaxLineLength:
active: true
maxLineLength: 120
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
style:
MaxLineLength:
active: true
maxLineLength: 'abc'