Run same rule multiple times with different configuration #7263

BraisGabin · 2024-05-06T19:20:47Z

With this PR we finally are able to run the same run multiple times with different configurations.

The most important part to discuss, I think, is if we like how those new instances are configured.

Right now if you want to create a new instance of the same rule you should add on your detekt.yml something like this:

styles:
  ForbiddenImport/compose:
    active: true
    ...

Close #6738

cortinico · 2024-05-06T22:25:34Z

And it produces this output:

The output has rule ids like [ForbiddenImport/AtTestCode] while in your config you have ForbiddenImport/2 and /3. I assume that's a mistake

cortinico

I really like this :D

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt

cortinico · 2024-05-06T22:30:57Z

config/detekt/detekt.yml

@@ -1,3 +1,6 @@
+config:
+  validation: false


The validation code doesn't know about this type of config keys yet. I need to implement that. I disabled it just to make it work for now. As I said this is still not ready to merge. I just wanted to share it to show that this is near to be done.

detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Rule.kt

BraisGabin · 2024-05-07T05:19:13Z

And it produces this output:

The output has rule ids like [ForbiddenImport/AtTestCode] while in your config you have ForbiddenImport/2 and /3. I assume that's a mistake

Indeed. Copy&paste error. I fixed it now.

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/rules/SingleRuleProvider.kt

github-advanced-security

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt

codecov · 2024-06-21T12:18:16Z

Codecov Report

Attention: Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 84.50%. Comparing base (4fa634a) to head (41ea7be).

Files	Patch %	Lines
...otlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt	86.66%	0 Missing and 2 partials ⚠️
...io/gitlab/arturbosch/detekt/core/rules/RuleSets.kt	33.33%	1 Missing and 1 partial ⚠️
...fig/validation/InvalidPropertiesConfigValidator.kt	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7263      +/-   ##
============================================
- Coverage     84.50%   84.50%   -0.01%     
- Complexity     4160     4163       +3     
============================================
  Files           572      572              
  Lines         11890    11902      +12     
  Branches       2461     2465       +4     
============================================
+ Hits          10048    10058      +10     
- Misses          590      591       +1     
- Partials       1252     1253       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

BraisGabin · 2024-06-22T08:00:35Z

@detekt/maintainers This PR is ready to review and I think that this one should get multiple eyes on it.

cortinico

LGTM 👍 We need docs for this also :)

TWiStErRob · 2024-06-23T12:34:10Z

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt

@@ -145,6 +154,11 @@ internal class Analyzer(
    }
 }

+internal fun extractRuleName(key: String): Rule.Name? =
+    runCatching { Rule.Name(key.split("/", limit = 2).first()) }.getOrNull()


Is there a way to get better DX here? In what cases can this throw and return null? If I read it correctly it'll simply ignore the rule. Should we fail when config is incorrect rather than let the user scratch their head why things are "not working". I guess the Rule.Name validation was there before, but now the runCatching can fail due to 2 reasons. Consider to get back "original" behavior:

Suggested change

runCatching { Rule.Name(key.split("/", limit = 2).first()) }.getOrNull()

val name = key.split("/", limit = 2).first()

return runCatching { Rule.Name(name) }.getOrNull()

Are we not allowing / in original rule id? Also instead of runCatching { Rule.Name(key.split("/", limit = 2).first()) }.getOrNull() can we do Rule.Name(key.split("/", limit = 2).firstOrNull()

TWiStErRob · 2024-06-23T12:38:31Z

detekt-core/src/test/resources/config_validation/rule-id.yml

+  active: true
+  WildcardImport:
+    active: true
+  WildcardImport/extra:


(Probably the wrong place to flag it, but wanted to start a thread.)

Is this syntax suppress-compatible, i.e. can I suppress a specific instance of a rule? Can IDEA handle it?

Will this work and only suppress the extra instance?

@file:Suppress("WildcardImport/extra")

Will this work and suppress all the instances?

@file:Suppress("WildcardImport")

How can I suppress only the default instance, and not the extras?

@file:Suppress("WildcardImport/default")

(Same for baseline)

@file:Suppress("WildcardImport") This only suppress the default one (if it exists)

So there's no syntax to suppress all of them? (e.g. if there's 3 set up, and all 3 flag the same thing, need to list 3 IDs?)

Yes. It you can give them the same alias to those 3 and use that.

I don't see a real scenario where suppressing all the rules of the same name is a good idea. But if someone end up with an issue like that we could implement wildcards RuleName/* or something like that. But I prefer not to do that until a real use case appears.

TWiStErRob · 2024-06-23T12:41:10Z

detekt-tooling/src/main/kotlin/io/github/detekt/tooling/api/spec/RulesSpec.kt

@@ -62,6 +62,6 @@ interface RulesSpec {
        /**
         * Run a single rule.
         */
-        class RestrictToSingleRule(val ruleSetId: RuleSet.Id, val ruleName: Rule.Name) : RunPolicy()
+        class RestrictToSingleRule(val ruleSetId: RuleSet.Id, val ruleId: String) : RunPolicy()


Could this be a value-class that is pre-split, or at least hides the complexity of split("/", 2).first() in a val ruleName: Rule.Name get() =?

Suggested change

class RestrictToSingleRule(val ruleSetId: RuleSet.Id, val ruleId: String) : RunPolicy()

class RestrictToSingleRule(val ruleSetId: RuleSet.Id, val ruleId: Rule.Id) : RunPolicy()

atulgpt · 2024-07-03T11:26:44Z

detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt

+                yamlConfigFromContent(
+                    """
+                        custom:
+                          MaxLineLength/foo:


one TC should be there with MaxLineLength/foo/bar and one with MaxLineLength/(no value after /) I think latter should be error in validation

detekt-ci added rules api core cli gradle-plugin formatting reports suppressors compiler-plugin labels May 6, 2024

vercel bot deployed to Preview May 6, 2024 19:23 View deployment

BraisGabin mentioned this pull request May 6, 2024

Add the option to instantiate the same rule more than once #6738

Open

cortinico reviewed May 6, 2024

View reviewed changes

BraisGabin force-pushed the multiple-instances branch from fe4a059 to b4e7660 Compare May 8, 2024 09:52

vercel bot deployed to Preview May 8, 2024 09:57 View deployment

BraisGabin mentioned this pull request May 19, 2024

Add new id for the rules #7313

Merged

BraisGabin force-pushed the multiple-instances branch 2 times, most recently from ba4ad98 to 1667d70 Compare May 19, 2024 15:03

github-advanced-security bot found potential problems May 19, 2024

View reviewed changes

vercel bot deployed to Preview May 19, 2024 15:09 View deployment

github-advanced-security bot found potential problems May 19, 2024

View reviewed changes

BraisGabin force-pushed the multiple-instances branch from 1667d70 to d4db32d Compare May 19, 2024 15:14

vercel bot deployed to Preview May 19, 2024 15:23 View deployment

BraisGabin force-pushed the multiple-instances branch 2 times, most recently from d325745 to d570d28 Compare May 20, 2024 21:08

vercel bot deployed to Preview May 20, 2024 21:18 View deployment

BraisGabin force-pushed the multiple-instances branch from d570d28 to ab77c23 Compare June 5, 2024 09:52

vercel bot deployed to Preview June 5, 2024 10:02 View deployment

BraisGabin force-pushed the multiple-instances branch from ab77c23 to e438dbc Compare June 5, 2024 10:32

vercel bot deployed to Preview June 5, 2024 10:37 View deployment

BraisGabin mentioned this pull request Jun 16, 2024

Improve single rule provider #7378

Merged

BraisGabin force-pushed the multiple-instances branch from e438dbc to 8b42dd9 Compare June 21, 2024 12:09

github-advanced-security bot found potential problems Jun 21, 2024

View reviewed changes

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Jun 21, 2024

View reviewed changes

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt Fixed Show fixed Hide fixed

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt Fixed Show fixed Hide fixed

vercel bot deployed to Preview June 21, 2024 12:18 View deployment

BraisGabin force-pushed the multiple-instances branch from 8b42dd9 to 72dfe9a Compare June 22, 2024 07:49

BraisGabin added 2 commits June 22, 2024 09:53

Support multiple instances with the same ruleId

01bb7d5

Update config validator

41ea7be

BraisGabin force-pushed the multiple-instances branch from 72dfe9a to 41ea7be Compare June 22, 2024 07:53

BraisGabin removed rules gradle-plugin formatting reports suppressors compiler-plugin labels Jun 22, 2024

BraisGabin added this to the 2.0.0 milestone Jun 22, 2024

BraisGabin marked this pull request as ready for review June 22, 2024 07:57

vercel bot deployed to Preview June 22, 2024 07:58 View deployment

cortinico approved these changes Jun 22, 2024

View reviewed changes

cortinico added the notable changes Marker for notable changes in the changelog label Jun 22, 2024

schalkms approved these changes Jun 22, 2024

View reviewed changes

TWiStErRob reviewed Jun 23, 2024

View reviewed changes

atulgpt reviewed Jul 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run same rule multiple times with different configuration #7263

Run same rule multiple times with different configuration #7263

BraisGabin commented May 6, 2024 •

edited

Loading

cortinico commented May 6, 2024

cortinico left a comment

cortinico May 6, 2024

BraisGabin May 7, 2024

BraisGabin commented May 7, 2024

github-advanced-security bot left a comment

codecov bot commented Jun 21, 2024 •

edited

Loading

BraisGabin commented Jun 22, 2024

cortinico left a comment

TWiStErRob Jun 23, 2024

atulgpt Jul 3, 2024

TWiStErRob Jun 23, 2024 •

edited

Loading

BraisGabin Jun 24, 2024

TWiStErRob Jun 24, 2024

BraisGabin Jun 24, 2024

TWiStErRob Jun 23, 2024 •

edited

Loading

atulgpt Jul 3, 2024

	runCatching { Rule.Name(key.split("/", limit = 2).first()) }.getOrNull()
	val name = key.split("/", limit = 2).first()
	return runCatching { Rule.Name(name) }.getOrNull()

	class RestrictToSingleRule(val ruleSetId: RuleSet.Id, val ruleId: String) : RunPolicy()
	class RestrictToSingleRule(val ruleSetId: RuleSet.Id, val ruleId: Rule.Id) : RunPolicy()

Run same rule multiple times with different configuration #7263

Are you sure you want to change the base?

Run same rule multiple times with different configuration #7263

Conversation

BraisGabin commented May 6, 2024 • edited Loading

cortinico commented May 6, 2024

cortinico left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BraisGabin commented May 7, 2024

github-advanced-security bot left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 21, 2024 • edited Loading

Codecov Report

BraisGabin commented Jun 22, 2024

cortinico left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TWiStErRob Jun 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TWiStErRob Jun 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BraisGabin commented May 6, 2024 •

edited

Loading

codecov bot commented Jun 21, 2024 •

edited

Loading

TWiStErRob Jun 23, 2024 •

edited

Loading

TWiStErRob Jun 23, 2024 •

edited

Loading