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

"let else" causing internal error ("left behind trailing whitespace") #5714

Closed
jmaargh opened this issue Mar 14, 2023 · 7 comments
Closed

"let else" causing internal error ("left behind trailing whitespace") #5714

jmaargh opened this issue Mar 14, 2023 · 7 comments
Labels
duplicate e-trailing whitespace error[internal]: left behind trailing whitespace

Comments

@jmaargh
Copy link

jmaargh commented Mar 14, 2023

$ cargo fmt --version
rustfmt 1.5.1-stable (d5a82bb 2023-02-07)

Minimal (perhaps not exactly) reproducible example:

fn main() {
    pub fn example(arg: Option<f32>) -> f32 {
        let Some(a) = arg else { 
    let b = 0.0;
            return b;
        };
        a
    }
}

Leave formatting errors (e.g. spurious whitespace) anywhere except the let-else expression (which isn't formatted, I understand to be intended behaviour), and rustfmt crashes rather than formatting it away. It outputs this with exit code 1:

$ cargo fmt
error[internal]: left behind trailing whitespace
 --> /home/jmaargh/code/eg/src/main.rs:3:3:33
  |
3 |         let Some(a) = arg else { 
  |                                 ^
  |

warning: rustfmt has failed to format. See previous 1 errors.

This appears to only happen when there is trailing whitespace left within the else block. That is, the trailing whitespace within the else block causes all formatting elsewhere in the file to break. If you manually "properly" format this else block by removing trailing whitespace, the error appears to go away.

@ytmimi
Copy link
Contributor

ytmimi commented Mar 14, 2023

Thanks for reaching out. This is a duplicate of #5688. let-else formatting is currently being worked on in #5690, but until those changes are merged and released rustfmt will continue to leave let-else statements unformatted. As a result it's up to the developer to remove trailing whitespace.

@ytmimi ytmimi closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2023
@ytmimi ytmimi added duplicate e-trailing whitespace error[internal]: left behind trailing whitespace labels Mar 14, 2023
@jmaargh
Copy link
Author

jmaargh commented Mar 14, 2023

As I said, I do not expect the whitespace within the let-else to be removed. This is a bug where valid input causes the application to crash.

@ytmimi
Copy link
Contributor

ytmimi commented Mar 14, 2023

@jmaargh I don't think that's correct. Are you invoking rustfmt directly on your code? If not it's likely that whatever program is calling rustfmt is preventing the formatting due to the 1 exit code.

The error does not prevent rustfmt from formatting the file. Take the following modified example:

fn main() {
    pub fn example(arg: Option<f32>) -> f32 {
        let Some(a) = arg else { 
    let b = 0.0;
            return b;
        };
        a
    }
}

struct MyPoorlyFormattedStruct {



a:     usize,
b:            String,
}

After running rustfmt directly on the input you get (note the let-else code is left unchanged while the other code is formatted):

fn main() {
    pub fn example(arg: Option<f32>) -> f32 {
        let Some(a) = arg else { 
    let b = 0.0;
            return b;
        };
        a
    }
}

struct MyPoorlyFormattedStruct {
    a: usize,
    b: String,
}

And you also get the error:

error[internal]: left behind trailing whitespace
 --> <stdin>:3:3:33
  |
3 |         let Some(a) = arg else { 
  |                                 ^
  |

warning: rustfmt has failed to format. See previous 1 errors.

The 1 exit code was recently discussed in #5711, and in the future we'll add an option to allow users to ignore trailing whitespace issue.

I also want to emphasize again that the underlying issue here is the lack of let-else support. I ran your input snippet against my feature branch for PR #5690 and got the following output with no errors.

fn main() {
    pub fn example(arg: Option<f32>) -> f32 {
        let Some(a) = arg else {
            let b = 0.0;
            return b;
        };
        a
    }
}

@ytmimi
Copy link
Contributor

ytmimi commented Mar 14, 2023

However calling cargo fmt might be where the issue is coming from in your case. It's likely that what your experiencing is related to / a duplicate of #3008

@jmaargh
Copy link
Author

jmaargh commented Mar 14, 2023

I see, thanks for explaining.

Personally, I'd still consider this to be two bugs (ignoring the fact that let-else is not yet supported, which is expected right now):

  1. Something odd is happening with trailing whitespace that's causing rustfmt to exit with 1, this doesn't appear to be quite the same as Trailing whitespace followed by long unbreakable line causes error[internal]: left behind trailing whitespace #5711 as it doesn't seem to have anything to do with over-long lines.
  2. rustfmts response to some valid input (above example) causes all formatting to cease, at least when run through another tool. My guess is that this is because cargo fmt sees a non-zero exit code and (understandably) errors out. This seems related to the couple of examples raised in cargo fmt --all completely stops formatting when failing on a single file #3008.

I'd be happy to help diagnose further/fix either of these issues. Personally, I'd consider exiting with non-zero code on valid input (that is, well-formed Rust) to be a bug, but I don't want to get my hands dirty with a new repo until I'm on the same page as the maintainers on the issue.

@ytmimi
Copy link
Contributor

ytmimi commented Mar 14, 2023

Something odd is happening with trailing whitespace that's causing rustfmt to exit with 1

There's nothing odd happening here, and exiting with 1 is intentional. rustfmt considers trailing whitespace to be a formatting error. From rustfmt's perspective the presence of trailing whitespace after formatting is an indication that something went wrong. Regardless of the reason (max width issue, unsupported language construct, some other underlying bug) rustfmt wasn't able to remove the trailing whitespace in the input. In the best case It's some weird edge case where there was whitespace in the input that the user can manually remove, and in the worst case (like the recently filed #5703) rustfmt was responsible for adding that trailing whitespace. Can you see why the latter would be considered an issue?

Personally, I'd consider exiting with non-zero code on valid input (that is, well-formed Rust) to be a bug

rustfmt's main concern is the output it produces, not the input it receives. In general the input doesn't even need to compile. The only criteria is that the input can be parsed as valid rust code. so I think shifting your perspective from "well-formed input" to "well-formed and well-formatted" output might help.

I'd be happy to help diagnose further/fix either of these issues.

I appreciate your willingness to help here! First and foremost we'd need a minimal example that reproduces the issue. As I demonstrated in #5714 (comment) code within the same file is still formatted, so maybe we need more than one file to reproduce the issue that you're describing.

If you're able to nail it down I'd suggest opening a new issue.

@calebcartwright
Copy link
Member

calebcartwright commented Mar 14, 2023

Want to add a bit of (hopefully helpful) context.

rustfmt is a pretty printer that works with the AST and doesn't directly process files/tokens. A key tenant of that model of formatter is ensuring that it can deal with any issue/failure/etc. that occurs when trying to format an AST node so that the formatter doesn't completely obliterate code the user wrote just because it can't format it.

The inability/failure to format can happen for a number of a reasons, and unsurprisingly as is often the case with software, rustfmt doesn't necessarily at compile time explicitly know the root cause of a runtime issue.

The error behavior reported and discussed here is part of a generalized recovery and alerting process to handle such formatting issues. That process exists both to ensure that the code is not lost and also to inform the user so that they know part of their code actually wasn't formatted (and thus likely needs human inspection), and so that they can hopefully share with the rustfmt team so that the underlying issue can be identified and addressed.

It's somewhat analogous in form and function to rustc ICEs, and is definitely not a bug/unintentional; the occurrence that triggers an ICE in rustc, or the trailing whitespace internal error in rustfmt, is a bug, but the reporting mechanism and treatment of the internal bug as an error condition, is not.

There could potentially be room for enhancements to allow users to opt into making this a warning vs. a hard error wrt the program exit behavior, but I think such a discussion around the "if" and "how" would be best carried out in a new, dedicated issue (same holds for any consideration around cargo fmt behavior when it encounters an error exit from its rustfmt ... invocations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate e-trailing whitespace error[internal]: left behind trailing whitespace
Projects
None yet
Development

No branches or pull requests

3 participants