rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.02k stars 889 forks source link

`match_arm_blocks = false` interacts poorly on some lines that are exactly `max_width + 2` long. #6136

Open pyrrho opened 7 months ago

pyrrho commented 7 months ago

I believe this to be a somewhat pathological off-by-one error.

I set up a reproduction repository, which I expect will also be the clearest description of this issue that I can give.

Attempting to summarize; given a max_width of 60, match_arm_blocks = false, and something like line 4 below, running cargo +nightly fmt will result in a line that is 61 characters wide. (NB. The specific max_width is unimportant; I'm using 60 here only to make the example slightly more concise.)

// Input
fn foo(i: i32) {
    match i {
        // This line is 62 characters wide
        _ => println!("1st argument: {}", "2nd argument ___"),
    }
}

// Output
fn foo(i: i32) {
    match i {
        _ =>
            // This line is 61 characters wide
            println!("1st argument: {}", "2nd argument ___"),
    }
}

This could be avoided if the arguments in the println! invocation were moved into the vertical style. Interestingly, if you add one more _ to line 4 (making that line 63 characters wide), those arguments will be moved into a vertical form, and max_width won't be exceeded. Also, if you set match_arm_blocks = true, there will be no trailing comma after the println!(..) (the comma will be moved to a }, on the line below), and max_width won't be exceeded.

I only noticed this edge case because the repository I'm working in also has error_on_line_overflow = true and error_on_unformatted = true set, which causes rustfmt to yield an error when run against a line like the one above. Which we somehow managed to already have in our repo when I started experimenting with match_arm_blocks = false.

ytmimi commented 7 months ago

Thanks for the detailed report!

I haven't had a chance to look into what's going on in this case, but it might be that we're failing to take the trailing comma into consideration when match_arm_blocks = false is set.