Skip to content

Code not formatted inside body of if with certain condition #5583

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
arxanas opened this issue Nov 3, 2022 · 3 comments
Closed

Code not formatted inside body of if with certain condition #5583

arxanas opened this issue Nov 3, 2022 · 3 comments

Comments

@arxanas
Copy link

arxanas commented Nov 3, 2022

Repro case: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ad64f0e25dfb5ae14d9693020ed0ff07

Code
// Tested with 1.5.1-nightly (2022-11-01 11ebe65)
impl RebasePlanBuilder {
    fn make_rebase_plan_for_current_commit() {
        if acc.any(|| {
            matches!(foo,
            Merge{
                commit_oid,replacement_commit_oid: _, new_parents:_ } if commit_oid == current_commit)
        }) {
            // No formatting in the above `matches!`, which is maybe understandable.
            // But formatting inside this block seems to be inhibited too?
            foo{bar}
        }
    }

    fn make_rebase_plan_for_current_commit() {
        if || {
            matches!(foo,
            Merge{
                commit_oid,replacement_commit_oid: _, new_parents:_ } if commit_oid == current_commit)
        } {
            // Okay
            foo { bar }
        }
    }

    fn make_rebase_plan_for_current_commit() {
        if acc.any(|| {
            matches!(foo,
            Merge{
                commit_oid,replacement_commit_oid: _, new_parents } if commit_oid == current_commit)
        }) {
            // Okay
            foo { bar }
        }
    }
}

In the first fn, the body of the if-statement is not formatted. Actual behavior: stays as foo{bar}. Expected behavior: becomes foo { bar }. But by removing the acc.any() call around the condition, the body starts being formatted again. The complexity (or presence of macros) in the if-condition shouldn't affect whether the body is formatted.

@ytmimi
Copy link
Contributor

ytmimi commented Nov 3, 2022

Thanks for reaching out.

There are two issues at play here. First rustfmt doesn't handle matches! macros all that well when there's a guard clause (#5547). Failing to format the matches! leads to the line with if commit_oid == current_commit exceeding the max_width of 100, which then triggers #3863. Failing to format the chained method call acc.any() leads to rustfmt being unable to format the if-block.

This boils down to a max_width issue, and if you'd like some immediate relief you can bump up the max_width e.g. max_width=110

For some additional info rustfmt operates on the AST nodes of your program and when it can't format part of that node it fails to format the entire node and leaves the code unchanged. In this case an ast::Expr node with a kind field of ast::ExprKind::If is the node in question, and because we fail to format the conditional expression for the reasons mentioned above we fail to format the if-block.

@ytmimi ytmimi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2022
@arxanas
Copy link
Author

arxanas commented Nov 3, 2022

@ytmimi is there a way to get rustfmt to emit a warning when a node fails to be formatted due to max_width? I suspected as much, but I couldn't find an obvious way to confirm.

@ytmimi
Copy link
Contributor

ytmimi commented Nov 3, 2022

You can try setting --config=error_on_line_overflow=true when invoking rustfmt directly or adding error_on_line_overflow = true to your rustfmt.toml

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

No branches or pull requests

2 participants