Replace KtLintMultiRule

[marschwar]

This relates to #5192 and removes the last MutliRule usage in the detekt code base.

Unfortunately I had to change the documentation generation mechanism here as well as it did not work when the rule list has a function call attached such as RuleSet("formatting", listOf(Rule1(config).sorted()).

[codecov]

Codecov Report

Merging #6094 (0d76036) into main (6f166ec) will decrease coverage by 0.49%.
The diff coverage is 96.90%.

@@             Coverage Diff              @@
##               main    #6094      +/-   ##
============================================
- Coverage     84.94%   84.45%   -0.49%     
+ Complexity     4026     3988      -38     
============================================
  Files           569      568       -1     
  Lines         13498    13416      -82     
  Branches       2381     2366      -15     
============================================
- Hits          11466    11331     -135     
- Misses          869      935      +66     
+ Partials       1163     1150      -13     

Impacted Files	Coverage Δ
...t/generator/collection/RuleSetProviderCollector.kt	78.88% <66.66%> (+0.47%)	⬆️
...arturbosch/detekt/formatting/FormattingProvider.kt	100.00% <100.00%> (ø)
...osch/detekt/formatting/FormattingRuleComparator.kt	100.00% <100.00%> (ø)
...sch/detekt/generator/collection/DetektCollector.kt	75.00% <100.00%> (-0.76%)	⬇️

... and 16 files with indirect coverage changes

[marschwar]

Coming back to your comment @chao2zhang

The reason behind this is because there are internal orders to execute ktlint rules, hence we need to extract the ordering logic into Detekt's Analyzer.

Looking at the analyzer it seems to me that the execution order of the rules within a rule set is the same as the order returned by the rule set provider. I have not tried to write a test for it though. Do you think there needs to be some more work around that?

[marschwar]

This property was seemingly unused. What am I missing?

[chao2zhang]

See #6094 (comment)

[chao2zhang]

So this Kotlin property is not used in production

• It is used to generate the docs
• It is used to generate the config: https://github.com/detekt/detekt/blob/main/detekt-formatting/src/main/resources/config/config.yml
• Hence the actual config property "autoCorrect" was previously used

[marschwar]

Thank you. I have reverted the change.

[BraisGabin]

This can be merged after 1.23.0, right? As it is a big change and it doesn't provide any feature for the user.

[marschwar]

Yes, it is just a prerequisite for the deprecation removal and for whatever we end up going with the formatting rule set.

[BraisGabin]

😍 LGTM!

[chao2zhang]

Coming back to your comment @chao2zhang

The reason behind this is because there are internal orders to execute ktlint rules, hence we need to extract the ordering logic into Detekt's Analyzer.

Looking at the analyzer it seems to me that the execution order of the rules within a rule set is the same as the order returned by the rule set provider. I have not tried to write a test for it though. Do you think there needs to be some more work around that?

• The ordering logic can be found in Analyzer - So the order is actually determined by FormattingProvider.kt's autoCorrect config.

• Before this PR, all ktlint rules are supposed to run ahead of other detekt rules, simply because is io.gitlab.arturbosch.detekt.api.MultiRule -> rule.rules.any { it.autoCorrect }

• For maximum compatibility, we should still consider maintaining this behavior though

[chao2zhang]

Ktlint also defines VisitorModifier.RunAfterRule, and we should respect that

[marschwar]

Yap, that is described in #5259

[chao2zhang]

😭. Would you mind if I contribute to this PR as well?

[marschwar]

I do not mind at all. But I would not want to fix the RunAfterRule issue with this pr or do you see them as strongly connected?

[chao2zhang]

It seems that we agreed that this PR can be merged post 1.23
Let me just open a new PR post the merge of this PR.

[marschwar]

• The ordering logic can be found in Analyzer - So the order is actually determined by FormattingProvider.kt's autoCorrect config.

• Before this PR, all ktlint rules are supposed to run ahead of other detekt rules, simply because is io.gitlab.arturbosch.detekt.api.MultiRule -> rule.rules.any { it.autoCorrect }

• For maximum compatibility, we should still consider maintaining this behavior though

Thank you for pointing this out. I can see why rules that might change the source code (autocorrect==true) should run first. That totally makes sense. But why must the other formatting rules run before all other rules? Honestly this feels more like a coincidence to me.