New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds ForbiddenSuppress rule #4899
Conversation
6c66b9a
to
f8a363e
Compare
f8a363e
to
e9d6e91
Compare
Codecov Report
@@ Coverage Diff @@
## main #4899 +/- ##
============================================
+ Coverage 84.80% 84.82% +0.02%
- Complexity 3488 3511 +23
============================================
Files 495 497 +2
Lines 11464 11549 +85
Branches 2122 2139 +17
============================================
+ Hits 9722 9797 +75
- Misses 685 687 +2
- Partials 1057 1065 +8
Continue to review full report at Codecov.
|
detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff 👍 Thanks for taking the time to send this over
:)
override val issue = Issue( | ||
javaClass.simpleName, | ||
Severity.Maintainability, | ||
"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be left empty 👍
detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
Outdated
Show resolved
Hide resolved
detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
Outdated
Show resolved
Hide resolved
.find { it is KtStringTemplateExpression } | ||
?.children | ||
?.find { it is KtLiteralStringTemplateEntry } | ||
val text = stringTemplate?.text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be chained with the rest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I extracted that because I was following along debugging the code, will add to the chain.
848c6f8
to
cdac21c
Compare
cdac21c
to
7aac082
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I second Nicos comments.
Thanks for this awesome contribution! 👍
* This rule allows to set a list of rules whose suppression is forbidden. This can be used to discourage abuse of the | ||
* `Suppress` and `SuppressWarnings` annotations. Detekt will report suppression of all forbidden rules. This rule is | ||
* not capable of reporting suppression of itself, as that's a language feature with precedence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor formatting and grammar
* This rule allows to set a list of rules whose suppression is forbidden. This can be used to discourage abuse of the | |
* `Suppress` and `SuppressWarnings` annotations. Detekt will report suppression of all forbidden rules. This rule is | |
* not capable of reporting suppression of itself, as that's a language feature with precedence. | |
* This rule allows to set a list of rules whose suppression is forbidden. | |
* This can be used to discourage the abuse of the `Suppress` and `SuppressWarnings` annotations. | |
* Detekt will report the suppression of all forbidden rules. | |
* This rule is not capable of reporting the suppression of itself, as that's a language feature with precedence. |
7aac082
to
e439cc7
Compare
This rule is based on discussion detekt#4890. Checks for suppression of one or more rules that have been decided to never be suppressed within a code base. For example, if we've set MaximumLineLength to a value, every file must be compliant, and if there is a special case where compliance is discouraged, we can simply exclude this file within YML configuration. This rule cannot prevent itself from being suppressed, but should serve to discourage the over use of `@Suppress` annotations.
e439cc7
to
d0c9ffc
Compare
@cortinico @schalkms addressed your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thank you for sending this
This rule is based on discussion #4890. It has been added to the "style"
rule set, since I believe it is opinionated and usage would vary across code
bases.
Checks for suppression of one or more rules that have been decided to
never be suppressed within a code base. For example, if we've set
MaximumLineLength to a value, every file must be compliant, and if there
is a special case where compliance is discouraged, we can simply exclude
this file within YML configuration.
This rule cannot prevent itself from being suppressed, but should serve
to discourage the over use of
@Suppress
annotations.