Add support for local suppression inside formatting #5876
Conversation
| io.gitlab.arturbosch.detekt.test.assertThat(findings).hasSize(2) | ||
| io.gitlab.arturbosch.detekt.test.assertThat(findings[0]).hasSourceLocation(7, 8) | ||
| io.gitlab.arturbosch.detekt.test.assertThat(findings[1]).hasSourceLocation(13, 12) |
There was a problem hiding this comment.
I have a follow-up PR to clean all the detekt-formatting from using non detekt's assertThat function here: cortinico@d9a4ea9
|
Can someone take a look at this one? |
cortinico
requested review from
chao2zhang,
3flex and
BraisGabin
and removed request for
chao2zhang and
3flex
| @@ -41,32 +36,6 @@ abstract class FormattingRule(config: Config) : Rule(config) { | |||
| val runAsLateAsPossible | |||
| get() = RunAsLateAsPossible in wrapping.visitorModifiers | |||
|
|
|||
| private val emit = { offset: Int, message: String, canBeAutoCorrected: Boolean -> | |||
| val (line, column) = positionByOffset(offset) | |||
| val location = Location( | |||
There was a problem hiding this comment.
Would you mind adding a unit test for the location if such test has not existed
There was a problem hiding this comment.
I've updated the tests to follow the logic here (i.e. previously we had a test that was checking file suppression as valid and local suprresion as not. Now the test checks that both suppressions are valid).
Codecov Report
@@ Coverage Diff @@
## main #5876 +/- ##
============================================
- Coverage 84.59% 84.46% -0.14%
Complexity 3784 3784
============================================
Files 546 546
Lines 12944 12923 -21
Branches 2273 2268 -5
============================================
- Hits 10950 10915 -35
- Misses 861 877 +16
+ Partials 1133 1131 -2
... and 6 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Fixes #5547
Fixes #5566
Related to #1999 also
This introduced the possibility to do more fine grained suppression on
detekt-formattingrules. Specifically I'm dropping the custom logic we apply to theEntitywe receive and just pass over thePsiElementthat KtLint passes us.Some of the location in the tests changed due to this, so this is a disruptive change at least for baselines (as the signature also changes). If we decide to merge it we should flag it in the release notes.
At the same time with KtLint you could only suppress at the
@file:level so, aside from baseline, this won't disrupt other workflows.