Implement ignoreAnnotated as a core feature

[BraisGabin]

Discussion here: #4068

This id the first step for a more advance way of suppress findings. It implements the most common request one and probably the easiest: ignoreAnnotated. And I implemented it in a way that adding more should be easy.

The main problem that I see with this implementation is the discoverability. As an user this is an interesting feature but it is really difficult to find it. For sure we should add somerhing in our documentation but I think that it would not be enought. Any ideas here?

In this PR I'm removing some default values. For that reason I created #4101 before. We don't want those default values, they are library specific.

And I changed LongParameterList a bit too. It was using the same configuration to do two things: 1) ignore a function completely for an annotation and 2) ignore the annotated parameter for the total count. So I removed the first part of that feature to relay on the common one and changed the second one to use ignoreAnnotatedParameter instead of ignoreAnnotated. This is a breaking change.

Right now I'm just treating the annotations as a name, I'm not using the full qualifier. Do you think that it is necesary to do that? I think that we could keep it like this for now and we could change it later if someone miss that. We should not have problem extending it.

Closes #2231
Closes #4062

[codecov]

Codecov Report

Merging #4102 (27b24fa) into main (f920504) will increase coverage by 0.01%.
The diff coverage is 86.20%.

❗ Current head 27b24fa differs from pull request most recent head 1a8f3d8. Consider uploading reports for the commit 1a8f3d8 to get more accurate results

@@             Coverage Diff              @@
##               main    #4102      +/-   ##
============================================
+ Coverage     83.45%   83.46%   +0.01%     
+ Complexity     3185     3177       -8     
============================================
  Files           463      465       +2     
  Lines          9095     9103       +8     
  Branches       1768     1774       +6     
============================================
+ Hits           7590     7598       +8     
  Misses          571      571              
  Partials        934      934              

Impacted Files	Coverage Δ
...b/arturbosch/detekt/rules/complexity/LongMethod.kt	88.63% <ø> (+1.13%)	⬆️
...bosch/detekt/rules/complexity/LongParameterList.kt	83.33% <50.00%> (-1.12%)	⬇️
...otlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt	80.95% <80.00%> (-0.30%)	⬇️
...ch/detekt/core/suppressors/AnnotationSuppressor.kt	90.00% <90.00%> (ø)
...ab/arturbosch/detekt/core/config/ValidateConfig.kt	100.00% <100.00%> (ø)
.../arturbosch/detekt/core/suppressors/Suppressors.kt	100.00% <100.00%> (ø)
...tlab/arturbosch/detekt/rules/bugs/LateinitUsage.kt	90.00% <100.00%> (ø)
...b/arturbosch/detekt/rules/naming/FunctionNaming.kt	100.00% <100.00%> (ø)
...etekt/rules/style/FunctionOnlyReturningConstant.kt	94.87% <100.00%> (ø)
...sch/detekt/rules/style/UnnecessaryAbstractClass.kt	82.92% <100.00%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e265df...1a8f3d8. Read the comment docs.

[cortinico]

Great work! I believe on the discoverability we can just have a page in the documentation and link it in the release notes

[marschwar]

I like where this is headed.

[marschwar]

If I read this correctly, the list of suppressors would be generated for each and every file even though it only depends on the rule. This will create a lot of unnecessary objects in the process.

[BraisGabin]

Yes. But the same happens right now with the rules. We create all the rules for each file. Changing this is a big refactor. It will be fixed in 2.0

[picklebento]

We only have one Suppressor instance in the returned list - Would you consider using the decorator pattern in case we have more Suppressors or use List<Suppressor>?

[BraisGabin]

I don't know what do you mean by "decorator pattern".

Right now there is only one suppressor but my idea is to have more once we merge this. There is a short list of examples at #4068

[picklebento]

open abstract class Suppressor : Predicate<Finding> {
}

operator fun Suppressor.plus(suppressor: Suppressor): Suppressor {
    return object : Suppressor() {
        override fun test(t: Finding) = this@plus.test(t) || suppressor.test(t)
    }
}

object DefaultSuppressor : Suppressor() {
    override fun test(t: Finding) = true
}

I meant like this where we can compose suppressors in a chain (Probably should be called as a chain of responsibility). The benefit of so is that we can simplify the suppressor implementation that to the Analyzer, the suppressor function is always a simple function (Finding) -> Boolean.

[BraisGabin]

I think that I prefer to handle this as a List. The good part is that all of these isn't an public API so we could change it if we see problems with the current implementation.

[IgorMaineti]

Hello! Are there any plans on releasing a new version of detekt with this change?

[cortinico]

Hello! Are there any plans on releasing a new version of detekt with this change?

I guess we can kickoff the 1.19.x release process (with a RC1) sometime this weekend or so 👍