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

Lint more cases in collapsible_if, without suggestions #14231

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

samueltardieu
Copy link
Contributor

@samueltardieu samueltardieu commented Feb 16, 2025

Due to its use of Sugg::ast() to combine if conditions together, collapsible_if cannot keep comments located inside the conditions or between the conditions in its suggestions. However, it is still possible to warn about those situations and let the user fix their code by themselves.

If reviewed on GitHub, the second commit of this PR is best looked at side by side, with whitespace differences turned off.

changelog: [collapsible_if]: lint more cases

@rustbot
Copy link
Collaborator

rustbot commented Feb 16, 2025

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 16, 2025
@samueltardieu samueltardieu force-pushed the push-uopxwlslsuwp branch 2 times, most recently from b8d9f37 to a94f321 Compare February 16, 2025 15:32
@samueltardieu
Copy link
Contributor Author

I think I'll add a configuration option to merge if containing comments, as some people might not like it.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Feb 18, 2025
@samueltardieu
Copy link
Contributor Author

Done, with the lint_commented_code option, true by default.

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Feb 18, 2025
Due to its use of `Sugg::ast()` to combine `if` conditions together,
`collapsible_if` cannot keep comments located inside the conditions
or between the conditions in its suggestions. However, it is still
possible to warn about those situations and let the user fix their
code by themselves.

A new configuration option `lint_commented_code`, which is `true` by
default, opts out from this behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants