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

error[internal]: left behind trailing whitespace #4778

Closed
nico-abram opened this issue Apr 1, 2021 · 2 comments
Closed

error[internal]: left behind trailing whitespace #4778

nico-abram opened this issue Apr 1, 2021 · 2 comments
Labels
bug Panic, non-idempotency, invalid code, etc.

Comments

@nico-abram
Copy link

Rustfmt seems to kinda give up formatting some bits of code. It keeps whitespace (Both trailing and leading) as-is.

When I ran cargo fmt I got the following output:
imagen

To Reproduce

I think it's related to the thread_local! macro. I think this playground reproduces my problem: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a9f5c6ef67347cb0cf3885f9d3cbc471

I originally encountered the problem in this repo, around this area

Expected behavior

I expected rustfmt to correctly format the code

Meta

  • rustfmt version: rustfmt 1.4.36-nightly (7de6968 2021-02-07) (I also tried 1.4.36-nightly (7de6968e 2021-02-07) and rustfmt 1.4.30-stable (8c6769d7 2021-01-18) and got the same results/output)
  • From where did you install rustfmt?: rustup
  • How do you run rustfmt: cargo fmt, and originally with the rust-analyzer vscode plugin
@nico-abram nico-abram added the bug Panic, non-idempotency, invalid code, etc. label Apr 1, 2021
@calebcartwright
Copy link
Member

hi thanks for reaching out with the report, and boiling it down to a minimal repo!

The short answer is that you're running into a combination of rustfmt's minimal support for macro formatting in conjunction with how rustfmt deals with scenarios where it's simply not possible to format the input code within the max_width constraints. I know it's not necessarily the most intuitive behavior, but it is another report that's caused by known limitations/behaviors and open issues, so I'm going to go ahead and close this one.

For a longer answer, along with some steps you can go ahead and take if you'd like to get things formatting again:

Your current thread_local! maccall isn't something rustfmt is able to format (refs #8) so I'd suggest considering using { delimiters instead of ( and adjusting the indentation/formatting of call if/as needed.

Either way, the line in that call, with the associated indentation, is what's blowing past the width limit and starting the snowball. However, if importing RefCell prior to the call is an option for you (and switching to {) then you should be able to make those changes which will enable rustfmt to format.

E.g. this will be formatted:

fn test() {
    use std::cell::RefCell;
    let v = vec![];

    v.for_each(|x| {   
    println!("1");
                        {
                    thread_local! {
                    static _RAND: RefCell<rand::RandState> = RefCell::new(rand::RandState::new())
                };
                }
            });
}

In the absence of such changes, rustfmt is left with a macro call it can't interpret/safely format, and that results in content that's well past the max_width limit of 100 columns. In these types of scenarios, rustfmt simply bails and re-emits the original formatting, including any extra/trailing whitespace the developer may have had in their input code (by design rustfmt does not re-attempt to partially do any formatting of any part of the AST it previously failed to format).

The original thinking behind this behavior was to just defer to the developer and let them refactor their code accordingly so that it could be reformatted within the width limits, but by default rustfmt does this silently today; you can turn on error_on_line_overflow and/or error_on_unformatted to allow rustfmt to loudly flag these cases.

Additionally, the too-long-to-format code you have is contained within a closure that's part of a chain (v.for_each(...), and when rustfmt is unable to format any part of a chain due to width issues, it ends up not formatting the entire chain (#3863).

That means that in your original repo, nothing within your scanline_iter.for_each(|(i, output_scanline)| { chain is able to be formatted, and your original input (trailing whitespaces and all) is left in place, resulting in the whitespace errors you are seeing
https://github.com/nico-abram/rtweekend-rs/blob/cd3fc6db33967350429f180624222a25e388a4e4/src/main.rs#L270-L305

Hope that helps!

@nico-abram
Copy link
Author

Thanks a lot for the explanation and the workaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

No branches or pull requests

2 participants