Skip to content

Commit

Permalink
Make findings a List<Finding2> instead of a `Map<RuleSet.Id, List…
Browse files Browse the repository at this point in the history
…<Finding2>>` (#6884)

* Simplify code

* Refactor

* Remove Map from Detektion

* Update detekt-compiler-plugin/src/main/kotlin/io/github/detekt/compiler/plugin/internal/MessageCollectorExtensions.kt

Co-authored-by: Nicola Corti <corti.nico@gmail.com>

---------

Co-authored-by: Nicola Corti <corti.nico@gmail.com>
  • Loading branch information
BraisGabin and cortinico committed Apr 7, 2024
1 parent f0e7d91 commit dda3a11
Show file tree
Hide file tree
Showing 34 changed files with 84 additions and 184 deletions.
10 changes: 5 additions & 5 deletions detekt-api/api/detekt-api.api
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public class io/gitlab/arturbosch/detekt/api/DetektVisitor : org/jetbrains/kotli
public abstract interface class io/gitlab/arturbosch/detekt/api/Detektion : org/jetbrains/kotlin/com/intellij/openapi/util/UserDataHolder {
public abstract fun add (Lio/gitlab/arturbosch/detekt/api/Notification;)V
public abstract fun add (Lio/gitlab/arturbosch/detekt/api/ProjectMetric;)V
public abstract fun getFindings ()Ljava/util/Map;
public abstract fun getFindings ()Ljava/util/List;
public abstract fun getMetrics ()Ljava/util/Collection;
public abstract fun getNotifications ()Ljava/util/Collection;
}
Expand Down Expand Up @@ -149,7 +149,7 @@ public final class io/gitlab/arturbosch/detekt/api/Extension$DefaultImpls {
public abstract interface class io/gitlab/arturbosch/detekt/api/FileProcessListener : io/gitlab/arturbosch/detekt/api/Extension {
public abstract fun onFinish (Ljava/util/List;Lio/gitlab/arturbosch/detekt/api/Detektion;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public abstract fun onProcess (Lorg/jetbrains/kotlin/psi/KtFile;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public abstract fun onProcessComplete (Lorg/jetbrains/kotlin/psi/KtFile;Ljava/util/Map;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public abstract fun onProcessComplete (Lorg/jetbrains/kotlin/psi/KtFile;Ljava/util/List;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public abstract fun onStart (Ljava/util/List;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
}

Expand All @@ -159,7 +159,7 @@ public final class io/gitlab/arturbosch/detekt/api/FileProcessListener$DefaultIm
public static fun init (Lio/gitlab/arturbosch/detekt/api/FileProcessListener;Lio/gitlab/arturbosch/detekt/api/SetupContext;)V
public static fun onFinish (Lio/gitlab/arturbosch/detekt/api/FileProcessListener;Ljava/util/List;Lio/gitlab/arturbosch/detekt/api/Detektion;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public static fun onProcess (Lio/gitlab/arturbosch/detekt/api/FileProcessListener;Lorg/jetbrains/kotlin/psi/KtFile;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public static fun onProcessComplete (Lio/gitlab/arturbosch/detekt/api/FileProcessListener;Lorg/jetbrains/kotlin/psi/KtFile;Ljava/util/Map;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public static fun onProcessComplete (Lio/gitlab/arturbosch/detekt/api/FileProcessListener;Lorg/jetbrains/kotlin/psi/KtFile;Ljava/util/List;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
public static fun onStart (Lio/gitlab/arturbosch/detekt/api/FileProcessListener;Ljava/util/List;Lorg/jetbrains/kotlin/resolve/BindingContext;)V
}

Expand Down Expand Up @@ -254,7 +254,7 @@ public abstract interface class io/gitlab/arturbosch/detekt/api/PropertiesAware
public abstract interface class io/gitlab/arturbosch/detekt/api/ReportingExtension : io/gitlab/arturbosch/detekt/api/Extension {
public abstract fun onFinalResult (Lio/gitlab/arturbosch/detekt/api/Detektion;)V
public abstract fun onRawResult (Lio/gitlab/arturbosch/detekt/api/Detektion;)V
public abstract fun transformFindings (Ljava/util/Map;)Ljava/util/Map;
public abstract fun transformFindings (Ljava/util/List;)Ljava/util/List;
}

public final class io/gitlab/arturbosch/detekt/api/ReportingExtension$DefaultImpls {
Expand All @@ -263,7 +263,7 @@ public final class io/gitlab/arturbosch/detekt/api/ReportingExtension$DefaultImp
public static fun init (Lio/gitlab/arturbosch/detekt/api/ReportingExtension;Lio/gitlab/arturbosch/detekt/api/SetupContext;)V
public static fun onFinalResult (Lio/gitlab/arturbosch/detekt/api/ReportingExtension;Lio/gitlab/arturbosch/detekt/api/Detektion;)V
public static fun onRawResult (Lio/gitlab/arturbosch/detekt/api/ReportingExtension;Lio/gitlab/arturbosch/detekt/api/Detektion;)V
public static fun transformFindings (Lio/gitlab/arturbosch/detekt/api/ReportingExtension;Ljava/util/Map;)Ljava/util/Map;
public static fun transformFindings (Lio/gitlab/arturbosch/detekt/api/ReportingExtension;Ljava/util/List;)Ljava/util/List;
}

public abstract interface annotation class io/gitlab/arturbosch/detekt/api/RequiresTypeResolution : java/lang/annotation/Annotation {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import org.jetbrains.kotlin.com.intellij.openapi.util.UserDataHolder
* which needs to be transferred from the detekt engine to the user.
*/
interface Detektion : UserDataHolder {
val findings: Map<RuleSet.Id, List<Finding2>>
val findings: List<Finding2>
val notifications: Collection<Notification>
val metrics: Collection<ProjectMetric>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ interface FileProcessListener : Extension {
* Called when processing of a file completes.
* This method is called from a thread pool thread. Heavy computations allowed.
*/
fun onProcessComplete(file: KtFile, findings: Map<RuleSet.Id, List<Finding2>>, bindingContext: BindingContext) {}
fun onProcessComplete(file: KtFile, findings: List<Finding2>, bindingContext: BindingContext) {}

/**
* Mainly use this method to save computed metrics from KtFile's to the {@link Detektion} container.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface ReportingExtension : Extension {
/**
* Allows to transform the reported findings e.g. apply custom filtering.
*/
fun transformFindings(findings: Map<RuleSet.Id, List<Finding2>>): Map<RuleSet.Id, List<Finding2>> = findings
fun transformFindings(findings: List<Finding2>): List<Finding2> = findings

/**
* Is called after all extensions's [transformFindings] were called.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.Notification
import io.gitlab.arturbosch.detekt.api.ProjectMetric
import io.gitlab.arturbosch.detekt.api.RuleSet
import org.jetbrains.kotlin.com.intellij.openapi.util.Key
import org.jetbrains.kotlin.com.intellij.openapi.util.UserDataHolderBase

Expand All @@ -14,7 +13,7 @@ class TestDetektion(
notifications: List<Notification> = emptyList(),
) : Detektion, UserDataHolderBase() {

override val findings: Map<RuleSet.Id, List<Finding2>> = findings.groupBy { it.ruleInfo.ruleSetId }
override val findings: List<Finding2> = findings.toList()
override val metrics: Collection<ProjectMetric> get() = _metrics
override val notifications: List<Notification> get() = _notifications

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fun MessageCollector.error(msg: String) {
}

fun MessageCollector.reportFindings(result: Detektion) {
for (finding in result.findings.values.flatten()) {
result.findings.forEach { finding ->
val (message, location) = finding.renderAsCompilerWarningMessage()
warn(message, location)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import io.gitlab.arturbosch.detekt.api.internal.isSuppressedBy
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.rules.associateRuleIdsToRuleSetIds
import io.gitlab.arturbosch.detekt.core.suppressors.buildSuppressors
import io.gitlab.arturbosch.detekt.core.util.isActiveOrDefault
import io.gitlab.arturbosch.detekt.core.util.shouldAnalyzeFile
Expand All @@ -28,8 +27,6 @@ import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactoryImpl
import kotlin.reflect.full.hasAnnotation

private typealias FindingsResult = List<Map<RuleSet.Id, List<Finding2>>>

internal class Analyzer(
private val settings: ProcessingSettings,
private val providers: List<RuleSetProvider>,
Expand All @@ -38,39 +35,31 @@ internal class Analyzer(
fun run(
ktFiles: Collection<KtFile>,
bindingContext: BindingContext = BindingContext.EMPTY
): Map<RuleSet.Id, List<Finding2>> {
): List<Finding2> {
val languageVersionSettings = settings.environment.configuration.languageVersionSettings

val dataFlowValueFactory = DataFlowValueFactoryImpl(languageVersionSettings)
val compilerResources = CompilerResources(languageVersionSettings, dataFlowValueFactory)
val findingsPerFile: FindingsResult =
if (settings.spec.executionSpec.parallelAnalysis) {
runAsync(ktFiles, bindingContext, compilerResources)
} else {
runSync(ktFiles, bindingContext, compilerResources)
}

if (bindingContext == BindingContext.EMPTY) {
warnAboutEnabledRequiresTypeResolutionRules()
}

val findingsPerRuleSet = HashMap<RuleSet.Id, List<Finding2>>()
for (findings in findingsPerFile) {
findingsPerRuleSet.mergeSmells(findings)
return if (settings.spec.executionSpec.parallelAnalysis) {
runAsync(ktFiles, bindingContext, compilerResources)
} else {
runSync(ktFiles, bindingContext, compilerResources)
}
return findingsPerRuleSet
}

private fun runSync(
ktFiles: Collection<KtFile>,
bindingContext: BindingContext,
compilerResources: CompilerResources
): FindingsResult =
ktFiles.map { file ->
): List<Finding2> =
ktFiles.flatMap { file ->
processors.forEach { it.onProcess(file, bindingContext) }
val findings = runCatching { analyze(file, bindingContext, compilerResources) }
.onFailure { throwIllegalStateException(file, it) }
.getOrDefault(emptyMap())
.getOrDefault(emptyList())
processors.forEach { it.onProcessComplete(file, findings, bindingContext) }
findings
}
Expand All @@ -79,34 +68,29 @@ internal class Analyzer(
ktFiles: Collection<KtFile>,
bindingContext: BindingContext,
compilerResources: CompilerResources
): FindingsResult {
): List<Finding2> {
val service = settings.taskPool
val tasks: TaskList<Map<RuleSet.Id, List<Finding2>>?> = ktFiles.map { file ->
val tasks: TaskList<List<Finding2>?> = ktFiles.map { file ->
service.task {
processors.forEach { it.onProcess(file, bindingContext) }
val findings = analyze(file, bindingContext, compilerResources)
processors.forEach { it.onProcessComplete(file, findings, bindingContext) }
findings
}.recover { throwIllegalStateException(file, it) }
}
return awaitAll(tasks).filterNotNull()
return awaitAll(tasks).filterNotNull().flatten()
}

private fun analyze(
file: KtFile,
bindingContext: BindingContext,
compilerResources: CompilerResources
): Map<RuleSet.Id, List<Finding2>> {
): List<Finding2> {
val activeRuleSetsToRuleSetConfigs = providers.asSequence()
.map { it to settings.config.subConfig(it.ruleSetId.value) }
.filter { (_, ruleSetConfig) -> ruleSetConfig.isActiveOrDefault(true) }
.map { (provider, ruleSetConfig) -> provider.instance() to ruleSetConfig }
.filter { (_, ruleSetConfig) -> ruleSetConfig.shouldAnalyzeFile(file) }
.toList()

val ruleIdsToRuleSetIds = associateRuleIdsToRuleSetIds(
activeRuleSetsToRuleSetConfigs.map { (ruleSet, _) -> ruleSet }
)

val (correctableRules, otherRules) = activeRuleSetsToRuleSetConfigs
.flatMap { (ruleSet, ruleSetConfig) ->
Expand All @@ -126,26 +110,11 @@ internal class Analyzer(
}
.partition { (_, rule) -> rule.autoCorrect }

val result = HashMap<RuleSet.Id, MutableList<Finding2>>()

fun executeRules(rules: List<Pair<Finding2.RuleInfo, Rule>>) {
for ((ruleInfo, rule) in rules) {
val findings = rule.visitFile(file, bindingContext, compilerResources)
.filterSuppressedFindings(rule, bindingContext)
for (finding in findings) {
val mappedRuleSet = checkNotNull(ruleIdsToRuleSetIds[rule.ruleId]) {
"Mapping for '${rule.ruleId}' expected."
}
result.computeIfAbsent(mappedRuleSet) { mutableListOf() }
.add(finding.toFinding2(ruleInfo, rule.computeSeverity()))
}
}
return (correctableRules + otherRules).flatMap { (ruleInfo, rule) ->
rule.visitFile(file, bindingContext, compilerResources)
.filterSuppressedFindings(rule, bindingContext)
.map { it.toFinding2(ruleInfo, rule.computeSeverity()) }
}

executeRules(correctableRules)
executeRules(otherRules)

return result
}

private fun warnAboutEnabledRequiresTypeResolutionRules() {
Expand Down Expand Up @@ -176,12 +145,6 @@ private fun List<Finding>.filterSuppressedFindings(rule: Rule, bindingContext: B
}
}

private fun MutableMap<RuleSet.Id, List<Finding2>>.mergeSmells(other: Map<RuleSet.Id, List<Finding2>>) {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ package io.gitlab.arturbosch.detekt.core

import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.RuleSet

class DelegatingResult(
result: Detektion,
override val findings: Map<RuleSet.Id, List<Finding2>>
override val findings: List<Finding2>
) : Detektion by result
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.Notification
import io.gitlab.arturbosch.detekt.api.ProjectMetric
import io.gitlab.arturbosch.detekt.api.RuleSet
import org.jetbrains.kotlin.com.intellij.openapi.util.UserDataHolderBase

@Suppress("DataClassShouldBeImmutable")
data class DetektResult(override val findings: Map<RuleSet.Id, List<Finding2>>) : Detektion, UserDataHolderBase() {
data class DetektResult(override val findings: List<Finding2>) : Detektion, UserDataHolderBase() {

private val _notifications = ArrayList<Notification>()
override val notifications: Collection<Notification> = _notifications
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ package io.gitlab.arturbosch.detekt.core.baseline
import io.github.detekt.tooling.api.Baseline
import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.RuleSet

internal class BaselineFilteredResult(
result: Detektion,
private val baseline: Baseline,
) : Detektion by result {

override val findings: Map<RuleSet.Id, List<Finding2>> = result.findings
.mapValues { (_, findings) -> findings.filterNot { baseline.contains(it.baselineId) } }
override val findings: List<Finding2> = result.findings.filterNot { baseline.contains(it.baselineId) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package io.gitlab.arturbosch.detekt.core.baseline

import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.ReportingExtension
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.SetupContext
import io.gitlab.arturbosch.detekt.api.getOrNull
import io.gitlab.arturbosch.detekt.core.DetektResult
Expand All @@ -20,7 +19,7 @@ class BaselineResultMapping : ReportingExtension {
createBaseline = context.getOrNull(DETEKT_BASELINE_CREATION_KEY) ?: false
}

override fun transformFindings(findings: Map<RuleSet.Id, List<Finding2>>): Map<RuleSet.Id, List<Finding2>> {
override fun transformFindings(findings: List<Finding2>): List<Finding2> {
val baselineFile = baselineFile
require(!createBaseline || (createBaseline && baselineFile != null)) {
"Invalid baseline options invariant."
Expand All @@ -29,14 +28,13 @@ class BaselineResultMapping : ReportingExtension {
return baselineFile?.let { findings.transformWithBaseline(it) } ?: findings
}

private fun Map<RuleSet.Id, List<Finding2>>.transformWithBaseline(
private fun List<Finding2>.transformWithBaseline(
baselinePath: Path,
): Map<RuleSet.Id, List<Finding2>> {
): List<Finding2> {
val facade = BaselineFacade()
val flatten = this.flatMap { it.value }

if (createBaseline) {
facade.createOrUpdate(baselinePath, flatten)
facade.createOrUpdate(baselinePath, this)
}

return facade.transformResult(baselinePath, DetektResult(this)).findings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ private fun Detektion.checkForIssuesWithSeverity(config: Config, minSeverity: Se

private fun Detektion.computeIssueCount(config: Config, minSeverity: Severity): Int =
filterAutoCorrectedIssues(config)
.flatMap { it.value }
.count { it.severity.isAtLeast(minSeverity) }

private fun Severity.isAtLeast(severity: Severity): Boolean = this.ordinal <= severity.ordinal
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.OutputReport
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.internal.BuiltInOutputReport

internal fun defaultReportMapping(report: OutputReport) =
Expand Down Expand Up @@ -35,21 +34,16 @@ private val messageReplacementRegex = Regex("\\s+")

fun Config.excludeCorrectable(): Boolean = subConfig(BUILD).valueOrDefault(EXCLUDE_CORRECTABLE, false)

fun Detektion.filterEmptyIssues(config: Config): Map<RuleSet.Id, List<Finding2>> {
fun Detektion.filterEmptyIssues(config: Config): List<Finding2> {
return this
.filterAutoCorrectedIssues(config)
.filter { it.value.isNotEmpty() }
}

fun Detektion.filterAutoCorrectedIssues(config: Config): Map<RuleSet.Id, List<Finding2>> {
fun Detektion.filterAutoCorrectedIssues(config: Config): List<Finding2> {
if (!config.excludeCorrectable()) {
return findings
}
val filteredFindings = HashMap<RuleSet.Id, List<Finding2>>()
findings.forEach { (ruleSetId, findingsList) ->
filteredFindings[ruleSetId] = findingsList.filter { finding -> !finding.autoCorrectEnabled }
}
return filteredFindings
return findings.filter { finding -> !finding.autoCorrectEnabled }
}

private fun Finding2.truncatedMessage(): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.ConsoleReport
import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.core.reporting.filterEmptyIssues

abstract class AbstractFindingsReport : ConsoleReport() {
Expand All @@ -25,5 +24,5 @@ abstract class AbstractFindingsReport : ConsoleReport() {
return render(findings)
}

abstract fun render(findings: Map<RuleSet.Id, List<Finding2>>): String
abstract fun render(findings: List<Finding2>): String
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.gitlab.arturbosch.detekt.core.reporting.console

import io.gitlab.arturbosch.detekt.api.Finding2
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.core.reporting.printFindings

/**
Expand All @@ -12,10 +11,8 @@ class FileBasedFindingsReport : AbstractFindingsReport() {

override val id: String = "FileBasedFindingsReport"

override fun render(findings: Map<RuleSet.Id, List<Finding2>>): String {
val findingsPerFile = findings.values
.flatten()
.groupBy { it.entity.location.filePath.absolutePath.toString() }
override fun render(findings: List<Finding2>): String {
val findingsPerFile = findings.groupBy { it.entity.location.filePath.absolutePath.toString() }
return printFindings(findingsPerFile)
}
}

0 comments on commit dda3a11

Please sign in to comment.