Skip to content

Add collapse-let-chains option to collapsible_if lint #14455

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

Closed

Conversation

samueltardieu
Copy link
Contributor

Since collapsible_if is an early lint, it is not possible to automatically check whether the let_chains unstable rustc feature is enabled or not, as features are not available in the EarlyContext object.

For this reason, the collapse-let-chains needs to be enabled explicitly. This can be reexamined later when the let_chains feature is stabilized.

changelog: [collapsible_if]: add collapse-let-chains option

r? ghost

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 23, 2025
@samueltardieu samueltardieu force-pushed the push-xyxtzlqyuwxy branch 3 times, most recently from cfb1562 to bec2183 Compare March 25, 2025 18:50
github-merge-queue bot pushed a commit that referenced this pull request Mar 25, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
This PR enables the new ability to collapse `if` statements containing
comments (without losing them) in Clippy sources, excluding tests and
lintcheck, where the default behaviour (no collapsing in presence of
comments) is preserved.

To be applied after #14231. When it is applied, #14455 will be marked as
ready for review, then #14228 afterwards.

changelog: none

r? ghost
Since `collapsible_if` is an early lint, it is not possible to
automatically check whether the `let_chains` unstable rustc feature is
enabled or not, as features are not available in the `EarlyContext`
object.

For this reason, the `collapse-let-chains` needs to be enabled
explicitly. This can be reexamined later when the `let_chains` feature
is stabilized.
@samueltardieu samueltardieu marked this pull request as ready for review March 25, 2025 19:43
@samueltardieu
Copy link
Contributor Author

samueltardieu commented Mar 25, 2025

r? @flip1995

(this is the logical succession to #14451 and will be followed by #14228 to end the 4 PR series)

@flip1995
Copy link
Member

How difficult would it be to turn this into a late lint pass?

@samueltardieu
Copy link
Contributor Author

How difficult would it be to turn this into a late lint pass?

I'll check when I get a chance. Originally, the lint used Sugg::ast() to get suggestions, but this is no longer the case since I know insert, remove, or replace parts of the source code, instead of reconstructing it. It should make it easier to now convert it into a late lint pass.

@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 Mar 26, 2025
@samueltardieu
Copy link
Contributor Author

@flip1995 I have opened #14481 with a late lint pass version. I'll close this one and #14228 since they are covered by the new PR.

github-merge-queue bot pushed a commit that referenced this pull request Mar 28, 2025
Until `if let` chains are stabilized, we do not collapse them together
or with other `if` expressions unless the `let_chains` feature is
enabled. This is the case for example in Clippy sources.

This was made possible by converting the `collapsible_if` to a late lint
to get access to the set of enabled features. This allows this PR to
supersede #14455 and no longer require an additional configuration
option.

The three commits are, in order:
- a conversion of the existing early lint to a late lint, with no new
feature or tests changes
- the addition of the `let_chains` feature detection and action, and
tests
- the application of the enhanced lint to Clippy sources (136 files
modified)

changelog: [`collapsible_if`]: recognize the rust compiler `let_chains`
feature

r? @flip1995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants