Skip to content
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

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

Open
bddap opened this issue Dec 14, 2021 · 12 comments · May be fixed by #5483
Open

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

bddap opened this issue Dec 14, 2021 · 12 comments · May be fixed by #5483
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release a-strings String literals bug Panic, non-idempotency, invalid code, etc. help wanted only-with-option requires a non-default option value to reproduce

Comments

@bddap
Copy link

bddap commented Dec 14, 2021

#[rustfmt::skip]
fn raw() {
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\a";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\";
    "\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\";
}

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

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

@bddap
Copy link
Author

bddap commented Dec 14, 2021

format_strings = true

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

@ytmimi ytmimi added only-with-option requires a non-default option value to reproduce bug Panic, non-idempotency, invalid code, etc. a-strings String literals labels Dec 15, 2021
@calebcartwright
Copy link
Member

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
Copy link
Contributor

ytmimi commented Dec 17, 2021

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?

@calebcartwright
Copy link
Member

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
Copy link
Contributor

ytmimi commented Dec 23, 2021

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
Copy link
Member

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
Copy link
Contributor

ytmimi commented Dec 23, 2021

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
Copy link

scovl commented Jan 26, 2022

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 (see max_width option), 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
Copy link
Author

bddap commented Jan 26, 2022

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

@scovl
Copy link

scovl commented Jan 27, 2022

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
Copy link
Author

bddap commented Jan 31, 2022

I have not.

davidBar-On added a commit to davidBar-On/rustfmt that referenced this issue Aug 4, 2022
davidBar-On added a commit to davidBar-On/rustfmt that referenced this issue Aug 4, 2022
@ldm0
Copy link

ldm0 commented Oct 8, 2022

fn main() {
    let body = format!("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"");
    
    /* After cargo fmt:
    
    let body = format!(
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\
         ""
    );
    
     */
}

Another badcase with format_strings = true enabled.

@ytmimi ytmimi added the 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release label Oct 12, 2022
ahl added a commit to ahl/rustfmt that referenced this issue Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release a-strings String literals bug Panic, non-idempotency, invalid code, etc. help wanted only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants