rust-lang / rustfmt

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

format_strings breaks after "\\" resulting in invalid output #5138

Open bddap opened 2 years ago

bddap commented 2 years ago
#[rustfmt::skip]
fn raw() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\";
}

fn formatted() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\
     \a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
     \";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
     \\\";
}

rustfmt 1.4.38-nightly (efec545 2021-12-04) installed via rustup

bddap commented 2 years ago

format_strings = true

Note this happens when the "\\" is at a position close to max width.

calebcartwright commented 2 years ago

This is one I'm fairly confident has been fixed on the other branch and just needs to have the commits cherry-picked over

ytmimi commented 2 years ago

This is one I'm fairly confident has been fixed on the other branch and just needs to have the commits cherry-picked over

Was it maybe 472a2ed0f6f75cc33f42a72beaf1827c13b713d1?

calebcartwright commented 2 years ago

This is one I'm fairly confident has been fixed on the other branch and just needs to have the commits cherry-picked over

Was it maybe 472a2ed?

I actually was thinking about #4524. Whenever you get a chance would you mind seeing if this behavior is reproducible on the 2.0 branch? If not that means the it's covered by that fix too, in which case we should give that another quick pass and consider backporting the commit(s)

ytmimi commented 2 years ago

on the 2.x branch here's how rustfmt formats the input:

fn raw() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\
    \\a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    \";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    \\\";
}

It's a slightly different formatting that the 1.x branch, but it still doesn't compile. The silver lining here is that the 2.x branch formats the first line in a way that would compile if it was the only line that we needed to be reformatted.

// This compiles
fn main() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\
    \\a";
}

The bad news is that the last two lines aren't formatted any differently so there's still some work to do, but I think backporting the existing changes is a good starting point.

calebcartwright commented 2 years ago

Thanks for digging into this!

The bad news is that the last two lines aren't formatted any differently so there's still some work to do, but I think backporting the existing changes is a good starting point.

Perhaps, though if that doesn't fully address the overall issue it may be better to try to find a holistic solution instead

ytmimi commented 2 years ago

Perhaps, though if that doesn't fully address the overall issue it may be better to try to find a holistic solution instead

Yeah you might be right

scovl commented 2 years ago

This incorrect output has already been fixed in version 1.60.0-nightly. In fact, the following output appears:

error[internal]: line formatted, but exceeded maximum width (maximum: 100 (seemax_widthoption), found: 103)

In this case, I don't know if the correct thing would be to increase the size or simply show the same message. Also because the case above, from a practical and functional point of view, does not seem to make sense to me. Any objection to this question @bddap ?

// This compiles
fn main() {
    println!(
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\
    \\a"
    );
}
bddap commented 2 years ago

Sorry, @lobocode . Not sure I understand what you are asking.

scovl commented 2 years ago

Sorry, @lobocode . Not sure I understand what you are asking.

I'm asking if you've checked, if this bug has been fixed in later versions of rustfmt.

bddap commented 2 years ago

I have not.

ldm0 commented 1 year ago
fn main() {
    let body = format!("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"");

    /* After cargo fmt:

    let body = format!(
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\
         ""
    );

     */
}

Another badcase with format_strings = true enabled.