Skip to content

Fix suggestion for collapsible_if and collapsible_else_if when the inner if is enclosed in parentheses #15304

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

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

Conversation

folkertdev
Copy link
Contributor

changelog: [collapsible_else_if]: fix suggestion when inner if as wrapped in parentheses
changelog: [collapsible_if]: fix suggestion when inner if as wrapped in parentheses

fixes #15303

I'm sure this is a bit dirty, but don't currently see a better way.

@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 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 Jul 17, 2025
@folkertdev folkertdev changed the title Collapsible if in parens Fix suggestion for collapsible_if and collapsible_else_if when the inner if is enclosed in parentheses Jul 17, 2025
@folkertdev folkertdev force-pushed the collapsible-if-in-parens branch 2 times, most recently from 585b21b to 93591d5 Compare July 17, 2025 18:07
@folkertdev folkertdev force-pushed the collapsible-if-in-parens branch from 93591d5 to ad64c81 Compare July 17, 2025 18:17
Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have a small suggestion to improve the code, otherwise this looks mergeworthy.

let start = span.shrink_to_lo();
let end = span.shrink_to_hi();

loop {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could simplify this loop: first we create a function that returns Option<Span>, trimming the outermost parens, then this can be a while let loop, updating the span.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tricky thing there is that we need to update 3 variables, so it gets kind of verbose. Anyway, I've now solved it using recursion, lmk what you think.

@folkertdev folkertdev force-pushed the collapsible-if-in-parens branch from ad64c81 to 052a4b0 Compare July 17, 2025 22:08
@profetia
Copy link
Contributor

Does blocks also leads to the same FN? If so, it would be great to also include fix for that too

@folkertdev
Copy link
Contributor Author

These lints don't fire when the inner if is wrapped in a block, e.g. these fail because the lint is not found

fn in_brackets() {
    if true {
        {if true {
            println!("In parens, linted");
        }}
    }
    //~^^^^^ collapsible_if
}

fn in_brackets() {
    let x = "hello";
    let y = "world";

    if x == "hello" {
        print!("Hello ");
    } else {
        {if y == "world" { println!("world") } else { println!("!") }}
    }
    //~^^^ collapsible_else_if
}

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.

incorrect fix for collapsable_if and collapsable_else_if when inner if is wrapped in parens
4 participants