Preventing rules from being suppressed

[gfreivasc]

I'm seeing a situation arise where some files in a code base I work on are being pushed with @Suppress annotations to avoid actually making the code compliant. I understand why it makes sense to suppress some rules in specific scenarios, but what I'm dealing with is not that such scenarios.

i.e.: We have MaximumLineLength set to 100 accordingly to android guidelines. Some classes have recently been pushed with @file:Suppress("MaximumLineLength") just to avoid dealing with the problem. They are existing classes which were not compliant before, but the policy we were following was to fix those lint issues whenever we had to touch these classes.

This is one rule which I'd be totally fine with disallowing suppression altogether. In the few (but possible) files where we'd want to ignore it, we could manually exclude it in the config YML instead, to further discourage it. My question is: is there a way to prevent specific rules from being suppressed?

[schalkms]

You could actually write a detekt rule, which detects Suppress annotations for given rules.
This new detekt rule could even include a config option passing the rule names.
With that in place, this new rule flags suppressions for the passed rule names.

The rule could look like this, for instance:

SuppressionViolation:
  active: true
  forRules: ["MagicNumber", ...]

This idea sounds awesome. However, a seasoned detekt user could suppress the rule, which exactly flags this suppression. 😉
This case could maybe be prevented in my opinion.

What do you think about this idea? @gfreivasc

---

It should be easy to implement this rule, since you could check this repo, which includes many rules that check annotations.

[schalkms]

The new rule approach is the way to go here. Hence, there exists also no API in detekt for that special use case, currently.

[BraisGabin]

Something like grep -r --include="*.kt" "@Suppress" . will provide you with all the usages of @Suppress if you then add | wc -l you will get the number of suppress. You hardcode that number on your CI and if the number goes up you fail the pipeline. But now anyone could change that number...

[schalkms]

You could write-protect the script file then. As I've said, it depends on how far you want to go down this rabbit hole.

[gfreivasc]

Keeping those things in central places, like the detekt YML, a build script or script file would be better specifically because these files are easier to watch and can be write-protected. This discussion has given me enough ideas to work with, and it seems a new rule would be the best approach at first.

[schalkms]

Thank you for the discussion! Glad that we could help. 😊

@gfreivasc if you could contribute this rule back to the main detekt repo, the community will be glad for yet another awesome rule.

[BraisGabin]

Really great topic! I had/have similar sitiations here are my 2 cents:

This is not something that detekt nor other tool can fix. It's part of the team culture. This are the tings that I do to prevent or lower this things:

• Configure detekt with the team. Each time that I configure detekt in a team I spend 2 months tweaking its configuration. It's vital that all the team give feedback. In this period I allow anyone to add anything new to the baseline. Detekt is not here to block your work.
  • If the team doesn't like one rule we disable it.
  • If a threshold feels too low, we move it up.
• Now, during 2 months, your team was using detektBaseline all the time. They don't think about suppressing any issue, they just want to add it to the baseline. This isn't ideal but it's way better than suppressing. At least you have a list of things that are wrong.
  • It's way easier to spot a change on the baseline than a @Suppress in a .kt.
• Your team adds thing to the baseline, you know what's going on. You now which rules are the problematic onces. Yesterday I was talking about moving up the threshold for LongMethod because of Compose. It's way better to lower a bit the standards than force something that the team will not comply.
• If the problems are related with formatting, remove detekt-formatting. Add ktlint directly. Teach the team about ./gradlew ktlintFormat. And then teach them about the git-hook.
• Monitor the number of lines in your baseline and report those metrics to the team regularly (I do it weekly). Make them feel that it is an important metric and that you need to lower it.
• On the PR ask why someone added something to the baseline instead of fixing it. Usually there are some good reasons, fix those issues.
  • Those issues can be a false positive on detekt or a missing configuration. Make your teamate be part of the loop, encorage them to open an issue.

They are some random ideas. Sorry they aren't well structured but I hope that some of them could help you :)