Move suppressions to core
#7101
Conversation
BraisGabin
force-pushed
the
suppression-to-core
branch
from
1a85f12
to
2cd837c
Compare
BraisGabin
force-pushed
the
suppression-to-core
branch
from
2cd837c
to
9b1c51b
Compare
BraisGabin
force-pushed
the
suppression-to-core
branch
from
9b1c51b
to
8d45e2d
Compare
BraisGabin
force-pushed
the
suppression-to-core
branch
from
8d45e2d
to
b1bc6bd
Compare
BraisGabin
force-pushed
the
suppression-to-core
branch
from
b1bc6bd
to
c0ff4b8
Compare
| @@ -25,12 +27,12 @@ fun Rule.compileAndLint(@Language("kotlin") content: String): List<Finding> { | |||
|
|
|||
| fun Rule.lint(@Language("kotlin") content: String): List<Finding> { | |||
| val ktFile = compileContentForTest(content) | |||
| return visitFile(ktFile) | |||
| return visitFile(ktFile).filterSuppressed(this) | |||
There was a problem hiding this comment.
I don't like to add this here too much but I think that for now it's the simpler solution.
Why do we need this? Right now we have some tests on the rules that check that when an annotation is set the finding is not raised. Those tests are testing two things. From one side they are testing that the suppression works, that's tested already somewhere else so it should not be tested. But they are also testing that the KtElement that they provide is correct or that the configured alias is correct. We could refactor those tests but the tests would be more difficult to follow (I tried). For that reason I decide to keep this here.
The good part is that, if we want, we could refactor those tests and remove this later.
There was a problem hiding this comment.
Step-by.step is the way to go here. As you said, I don't think it's ideal approach, though sufficient for now.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7101 +/- ##
============================================
- Coverage 84.77% 84.75% -0.03%
+ Complexity 3993 3992 -1
============================================
Files 578 578
Lines 12138 12027 -111
Branches 2484 2485 +1
============================================
- Hits 10290 10193 -97
+ Misses 624 606 -18
- Partials 1224 1228 +4 ☔ View full report in Codecov by Sentry. |
BraisGabin
force-pushed
the
suppression-to-core
branch
from
c0ff4b8
to
360ec92
Compare
BraisGabin
force-pushed
the
suppression-to-core
branch
from
7c76aa3
to
8804306
Compare
|
@detekt/maintainers could I get a review here? |
| @@ -101,15 +98,9 @@ open class Rule( | |||
|
|
|||
| /** | |||
| * Reports a single code smell finding. | |||
There was a problem hiding this comment.
At this point, detekt does not report, bur rather add it to the findings list.
| @@ -25,12 +27,12 @@ fun Rule.compileAndLint(@Language("kotlin") content: String): List<Finding> { | |||
|
|
|||
| fun Rule.lint(@Language("kotlin") content: String): List<Finding> { | |||
| val ktFile = compileContentForTest(content) | |||
| return visitFile(ktFile) | |||
| return visitFile(ktFile).filterSuppressed(this) | |||
There was a problem hiding this comment.
Step-by.step is the way to go here. As you said, I don't think it's ideal approach, though sufficient for now.
detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt
Show resolved
Hide resolved
All the suppression logic should be handled by the
core. Rule shouldn't know about that. This PR moves that logic tocoreand also moves the functions that are no longer used inapitocoreto keepapias clean as possible.