Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove dependencies on discouraged-comment-location rule #2371

Merged
merged 7 commits into from
Nov 22, 2023

Conversation

paul-dingemans
Copy link
Collaborator

@paul-dingemans paul-dingemans commented Nov 22, 2023

Description

Rule discouraged-comment-location intended to make implementation of rules easier. By disallowing those locations, the rules did not need to take them into accound.

Disabling the discouraged-comment-location rule however also forced users to disable other rules as well (for example see #2338). With the number of rules that depended on discouraged-comment-location and the number of locations which were forbidden, this became more and more a useability problem for certain users. Disabling the discouraged-comment-location rule for one specific type of comment location was not possible. So only choiche was to disable the entire rule and all rules that depend on it.

The discouraged-comment-location rule still exists to avoid a breaking change. But it no longer provides any functionality anymore. The logic of this rule has been moved to normal rule classes in case a discouraged comment location was added for one specific rule only. Comment locations that are more generic (e.g. used by at least two different rules) are moved to separate rules as this provides more clarity about rule dependencies.

Rules below no longer depend on any other rule to handle disallowed comment location. If needed, the rules provide custom checks:

  • chain-method-continuation
  • if-else-wrapping

New rules below are added to check for disallowed comment locations in constructs that are used by multiple rules:

  • type-argument/type-argument-list
  • type-projection/type-parameter-list
  • value-argument/value-argument-list
  • value-parameter/value-parameter-list
    Rules that previously relied explicitly or implicitly on the discouraged-comment-location rule now explicitly depend on rules above. If rules above are disabled, it is now more clear why a rule has a dependency on that rule as it is more specific.

To provide above, following functionality was added as well:

  • Add utility methods afterCodeSibling, beforeCodeSibling and betweenCodeSiblings to ASTNodeExtensions
  • KtlintAssertThat now provides a more consistent interface for building rule assertion function which are depending on another rule. For some rules, the editorconfig properties regarding setting code style to ktlint_official have been removed as this code style is by default.

Closes #2367

Checklist

Before submitting the PR, please check following (checks which are not relevant may be ignored):

  • Commit message are well written. In addition to a short title, the commit message also explain why a change is made.
  • At least one commit message contains a reference Closes #<xxx> or Fixes #<xxx> (replace<xxx> with issue number)
  • Tests are added
  • KtLint format has been applied on source code itself and violations are fixed
  • CHANGELOG.md is updated
  • PR description added

Documentation is updated. See difference between snapshot and release documentation

…tweenCodeSiblings` to ASTNodeExtensions

- Break dependency between `if-else-wrapping` and `discouraged-comment-location`
The `discouraged-comment-location` rule contained several unrelated comment locations. When disabling the rule, ktlint might throw an exception when another rule depends on it. It is understandable that users do not instantly understand the relation between for example the `if-else-wrapping` and the `discouraged-comment-location` rule. So rules now have to emit warnings themselves whenever comments can not be processed.

For the `type-argument/type-argument-list`, `type-projection/type-parameter-list`, `value-argument/value-argument-list`, `value-parameter/value-parameter-list` new rules have been created to disallow comments. As those rules are more specific, it is more reasonable why specific rules depend on those rules.
Logic of this rule has been moved to normal rule classes in case a discouraged comment location was added for one specific rule only. Comment locations that are more generic (e.g. used by at least two different rules) are moved to separate rules as this provides more clarity about rule dependencies.

Logic from this class has been removed, except for what is needed to avoid a breaking change.
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependencies on discouraged-comment-location rule
1 participant