Skip to content
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

Reuse com.pinterest.ktlint.core.Rule.VisitorModifier.RunAfterRule to keep order of rules #1309

Closed
nulls opened this issue May 27, 2022 · 4 comments · Fixed by #1310
Closed

Comments

@nulls
Copy link
Member

nulls commented May 27, 2022

Ordering was changed in pinterest/ktlint#1478: previously ktlint kept order of rules as ResultSetProvider returned it.
But now ktlink sorts rules by ID.

We found duplication and invalid order in rule id in scope of #1306.
Now prefix of ID which was introduced as tactic solution for ordering, can be removed

@paul-dingemans
Copy link

This PR solves the rule ordering problem by providing a custom RulesetProvider. The RulesetProvider is however going to be removed in next ktlint release 0.46 (pinterest/ktlint#1479). Please implement the Rule.VisitorModifier on those rules which really depend on a specific other rule. The less dependencies exists between rules, the better it is.

@nulls
Copy link
Member Author

nulls commented Jun 1, 2022

@paul-dingemans, I checked the provided PR and I don't see changes are related to RulesetProvider.
As I got, RulesetProvider is a way to provide a set of rules. Are you going to remove it and replace by another approach?

Thanks!

@paul-dingemans
Copy link

@nulls Sorry, I made a misjudgement. It is the VisitorProvider which is being removed in pinterest/ktlint#1479 instead of the RulesetProvider. So if this solution works out for you, you should be okay. Currently there are no plans to change the RulesetProvider interface.

@orchestr7
Copy link
Member

Currently there are no plans to change the RulesetProvider interface.

Good news :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants