Skip to content

"if expression" in implicit return position (?) considered a statement #4351

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
qm3ster opened this issue Jul 24, 2020 · 5 comments
Open
Assignees
Labels
a-matches match arms, patterns, blocks, etc poor-formatting

Comments

@qm3ster
Copy link

qm3ster commented Jul 24, 2020

Input

let x = {let a = true; if a { 1 } else { 2 }};
let y = match () {
    _ => if true { 1 } else { 2 },
};

Output

let x = {
    let a = true;
    if a {
        1
    } else {
        2
    }
};
let y = match () {
    _ => {
        if true {
            1
        } else {
            2
        }
    }
};

Expected output

let x = {
    let a = true;
    if a { 1 } else { 2 } 
};
let y = match () {
    _ => if true { 1 } else { 2 },
};

Meta

  • rustfmt version: rustfmt 1.4.18-nightly (c1e9b7b 2020-06-13)
  • From where did you install rustfmt?: rustup

The "block" case is also what happens in functions, for example:

fn min(perc: u8) -> u8 {
    if perc >= 30 {
        perc
    } else {
        0
    }
}
// "recovery" with an unnecessary `return`:
fn min(perc: u8) -> u8 {
    return if perc >= 30 {
        perc
    } else {
        0
    }
}
// *fmt*
fn min(perc: u8) -> u8 {
    return if perc >= 30 { perc } else { 0 };
}
// collapsed nicely

This kills the crab.

The expansion damage is also compounded by defaults of match_arm_blocks: true and overflow_delimited_expr: false.

@calebcartwright
Copy link
Member

My reading of this is that it's in alignment with the section Rust Style Guide that governs whether single lining of an if/else is possible, though I do think it would be beneficial to add some additional examples to that section in the Guide to make that explicitly clear.

https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/expressions.md#single-line-if-else

@qm3ster
Copy link
Author

qm3ster commented Jul 24, 2020

I see the

if x {
    0
} else {
    1
}

in there anywhere I can read about the rationale?

However, perhaps "non-()-returning if else in implicit return position is not a standalone statement" is a valid view?

Perhaps even "non-implicit-()-returning" (only empty block {} or last statement in the block ending with semicolon ; makes the if standalone)?

if x {
    ();
} else {
    ()
}
// and
if x {
} else {
    ()
}
// but
if x { () } else { () };

@calebcartwright
Copy link
Member

Ahh I think I remember this one. Try adding version = "Two" to your rustfmt config file

@calebcartwright
Copy link
Member

What happens is that those get parsed as expression statements. It was discovered after the rustfmt 1.0 release that expression statements in the last position of their respective blocks could be formatted as expressions though, and this could use the same-line logic as well. However, because that's a breaking change it had to be version gated (this is why you have to set version = "Two").

As far as the part of your snippet with the same in a match arm body:

let y = match () {
    _ => {
        if true {
            1
        } else {
            2
        }
    }
};

This is also governed by the style rules for matches, and because from an AST perspective it's a control flow expression within the Stmt's Expr, it has to go in the block formatting (my emphasis added to pertinent section)

If the body is a single expression with no line comments and not a control flow expression, then it may be started on the same line as the right-hand side. If not, then it must be in a block

So your expected output shared in the OP would contradict with the style guide

let y = match () {
    _ => if true { 1 } else { 2 },
};

Though there may be a case to be made for supporting similar single-line within the block of the arm body

@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

@calebcartwright is this expected behavior or should we extend the single line control flow rules to include match arms?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-matches match arms, patterns, blocks, etc poor-formatting
Projects
None yet
Development

No branches or pull requests

4 participants