Core handles active rules #6782
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BraisGabin
added
the
breaking change
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6782 +/- ##
=========================================
Coverage 84.94% 84.95%
+ Complexity 3890 3887 -3
=========================================
Files 573 574 +1
Lines 12910 12913 +3
Branches 2445 2443 -2
=========================================
+ Hits 10967 10970 +3
Misses 721 721
Partials 1222 1222 ☔ View full report in Codecov by Sentry. |
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
8aa35d8
to
d5f3264
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
d5f3264
to
a6e84d0
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
a6e84d0
to
c9593e9
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
c9593e9
to
82cdad8
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
82cdad8
to
fe83a9a
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
fe83a9a
to
433e3b8
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
433e3b8
to
24ea51a
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
24ea51a
to
3ccd34c
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
3ccd34c
to
27410f3
Compare
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
27410f3
to
863a91e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
...kt-formatting/src/test/kotlin/io/gitlab/arturbosch/detekt/formatting/AutoCorrectLevelSpec.kt
Fixed
Show fixed
Hide fixed
BraisGabin
force-pushed
the
core-handles-active-rules
branch
from
863a91e
to
54894b1
Compare
marschwar
approved these changes
Jan 12, 2024
| @@ -11,8 +11,7 @@ import io.gitlab.arturbosch.detekt.api.internal.createPathFilters | |||
| import io.gitlab.arturbosch.detekt.core.ProcessingSettings | |||
| import org.jetbrains.kotlin.psi.KtFile | |||
|
|
|||
| fun Config.isActive(): Boolean = | |||
| valueOrDefault(Config.ACTIVE_KEY, true) | |||
| internal fun Config.isActive(default: Boolean): Boolean = valueOrDefault(Config.ACTIVE_KEY, default) | |||
There was a problem hiding this comment.
Could we rename this to isActiveOrDefault?
There was a problem hiding this comment.
Also the location of the method is a bit misleading. The file is RuleSets but it is an extension on Config that is used in the context of rules, rulesets and processors.
There was a problem hiding this comment.
You are right with both comments. I did the changes.
mgroth0
pushed a commit
to mgroth0/detekt
that referenced
this pull request
Feb 11, 2024
* Rules doesn't handle if they are active or not * rename isActive to isActiveOrDefault --------- Co-authored-by: marschwar <marschwar@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Until now core said to all the rules to execute, even if they were disabled, and then the rules decide if they should execute or not looking at their own configuration.
This PR changes that. Now the rule doesn't know if it is active or not. Now the core checks if a rule is active or not and decide what to do. This simplifies a bit more the
RuleAPI. Also, this is not the final PR in this regard. The suppression is right now handled by the rule and I would like to move that tocoretoo. But that will be done later.This comes with a bonus:
Config.emptyis finally "empty". Until now it was empty except for theactiveparameter. That one always returnedtrueinstead of the expectednull.Also, I was looking how to test this and I found out that the tests were there already. That's the reason I created #6781 I was just looking how to test this and refactor those tests when I found out that this was already tested.