Introduce reporting extensions #2755
Conversation
Allows to pass arbitrary data to detekt extensions.
Reporting extensions provide interception points to filter findings and execute logic on detekt's result container.
Codecov Report
@@ Coverage Diff @@
## master #2755 +/- ##
============================================
+ Coverage 80.44% 80.51% +0.07%
- Complexity 2303 2314 +11
============================================
Files 378 384 +6
Lines 6949 6960 +11
Branches 1262 1260 -2
============================================
+ Hits 5590 5604 +14
+ Misses 729 728 -1
+ Partials 630 628 -2
Continue to review full report at Codecov.
|
| inline fun <reified T : Any> PropertiesAware.getOrNull(key: String): T? { | ||
| val value = properties[key] | ||
| if (value != null) { | ||
| return value.safeAs() ?: error("No value of type ''${T::class} for key '$key'.") | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
If we allow null values in the Map how are we going to know the difference between a value that it's there but it's null and a value that it's not there?
I vote for don't allo null as a possible value in the Map.
detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/PropertiesAwareSpec.kt
Show resolved
Hide resolved
detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/ExtensionsSpec.kt
Outdated
Show resolved
Hide resolved
detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettings.kt
Outdated
Show resolved
Hide resolved
| /** | ||
| * Is called after all extensions's [transformFindings] were called. | ||
| */ | ||
| fun onFinalResult(result: Detektion) { |
There was a problem hiding this comment.
I think that we should make Detektion a immutable class. Otherwise we are allowing to edit it here and in onRawResult. I know, this is out of the scope of this PR but we could add it to the architecture issue. What do you think?
There was a problem hiding this comment.
Yes, please feel free to write it down in the issue.
| @OptIn(UnstableApi::class) | ||
| class BaselineResultMapping : ReportingExtension { | ||
|
|
||
| private var baselineFile: Path? = null |
There was a problem hiding this comment.
The baseline file can be null.
| } | ||
| } |
There was a problem hiding this comment.
Yes, still I extracted the long lambda to an function to make it more clear :).
| val reportPaths: Collection<ReportPath> = | ||
| context.getOrNull(DETEKT_OUTPUT_REPORT_PATHS_KEY) ?: emptyList() | ||
| reports = reportPaths.associateBy { it.kind } | ||
| settings = context as? ProcessingSettings ?: error("ProcessingSettings expected.") |
There was a problem hiding this comment.
🤔 this cast is a bit strange, isn't it?
There was a problem hiding this comment.
Yes! I'm not sure how to handle this. It needs the plugin classloader. An extension which loads other extensions.
Feels kinda wrong.
I will convert it to a plain core class again.
| @@ -13,6 +13,8 @@ import kotlin.system.measureTimeMillis | |||
| @OptIn(UnstableApi::class) | |||
| class OutputFacade : ReportingExtension { | |||
|
|
|||
| override val priority: Int = Int.MIN_VALUE | |||
There was a problem hiding this comment.
Why is this necessary? OutputFacade only overrides onFinalResult and in that state no one should change the Detektion (for that reason I was talking about Detektion as an immutable class)
There was a problem hiding this comment.
However Detektion is still mutable. So it might be that anyone may add additional ProjectMetric's and expect them to be printed on the console.
With running the output facade last we will ensure that everything stored will be printed.
…sion An extensions which loads other extensions feels kind of wrong. Reporting is a core logic and shouldn't be optional.
This PR introduces the concept of a
ReportingExtension.With this extension the user can listen to results and manipulate them.
The first two implementations are
BaselineFacadeandOutputFacade.With this concept the baseline and the reporting features do not need special treatment in the
Runnerclass or the Intellij or Sonarqube plugin but are just loaded from the detekt core engine and executed.No more code like:
Extracting the "special @Suppress" implemention to a
ReportingExtensionis conceivable in the future.This PR also introduces a concept to share data between extensions and tooling frondends (cli, gradle, sonar, intellij) via the
PropertiesAwareinterface.It allows to retrieve data in a type safe way:
This lightweight key-value storage reduces the need for extra parameters for the
ProcessingSettingsclass.I've marked both new api interfaces as
@UnstableApifor now.#2680