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

Internal error when trying to format code with inline block comment #6200

Open
Sorseg opened this issue Jun 18, 2024 · 4 comments
Open

Internal error when trying to format code with inline block comment #6200

Sorseg opened this issue Jun 18, 2024 · 4 comments
Labels
a-comments e-trailing whitespace error[internal]: left behind trailing whitespace

Comments

@Sorseg
Copy link

Sorseg commented Jun 18, 2024

When formatting this file
cargo +nightly fmt does not modify the file and reports this error:

error[internal]: left behind trailing whitespace
   --> <...>/riano/src/main.rs:353:353:58
    |
353 |                                     piano.sustain = true; 
    |                                                          ^
    |

warning: rustfmt has failed to format. See previous 1 errors.
$ cargo +nightly fmt --version
rustfmt 1.7.0-nightly (59e2c01 2024-06-17)

Removing the block comment on this line allows rustfmt to format the file again.

Thank you for maintaining this! 🧡

@ytmimi
Copy link
Contributor

ytmimi commented Jun 18, 2024

@Sorseg linking to the file is definitely useful. When you get a chance could you try to create a minimal test case that someone could use to reproduce the issue.

@ytmimi ytmimi added needs-mcve needs a Minimal Complete and Verifiable Example e-trailing whitespace error[internal]: left behind trailing whitespace labels Jun 18, 2024
@Sorseg
Copy link
Author

Sorseg commented Jun 21, 2024

I was able to boil it down to this

struct S {
    f: u32,
}

fn main() {
    let s = Some(S { f: 42 });
    match s {
        Some( S{f: /* some special value */3}) => { }
        _ => {}
    }
}

cargo fmt doesn't error, just silently does nothing

Is this workable?

Thank you for taking a look

@ytmimi
Copy link
Contributor

ytmimi commented Jun 24, 2024

If you set error_on_unformatted = true you'll get some additional information about why this failed to format. The issue here is that rustfmt isn't expecting to find a comment between f: 3 in the pattern. To prevent dropping the comment rustfmt doesn't rewrite the code:

You should see an error similar to this:

error[internal]: not formatted because a comment would be lost
 --> <stdin>:7
  |
7 |     match s {
  |
  = note: set `error_on_unformatted = false` to suppress the warning against comments or string literals

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

@ytmimi
Copy link
Contributor

ytmimi commented Jun 24, 2024

I believe we'd need to make adjustments in the Rewrite for PatField impl to address this, though my current recommendation would be to not write comments within struct fields:

rustfmt/src/patterns.rs

Lines 385 to 433 in e494418

impl Rewrite for PatField {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
let hi_pos = if let Some(last) = self.attrs.last() {
last.span.hi()
} else {
self.pat.span.lo()
};
let attrs_str = if self.attrs.is_empty() {
String::from("")
} else {
self.attrs.rewrite(context, shape)?
};
let pat_str = self.pat.rewrite(context, shape)?;
if self.is_shorthand {
combine_strs_with_missing_comments(
context,
&attrs_str,
&pat_str,
mk_sp(hi_pos, self.pat.span.lo()),
shape,
false,
)
} else {
let nested_shape = shape.block_indent(context.config.tab_spaces());
let id_str = rewrite_ident(context, self.ident);
let one_line_width = id_str.len() + 2 + pat_str.len();
let pat_and_id_str = if one_line_width <= shape.width {
format!("{id_str}: {pat_str}")
} else {
format!(
"{}:\n{}{}",
id_str,
nested_shape.indent.to_string(context.config),
self.pat.rewrite(context, nested_shape)?
)
};
combine_strs_with_missing_comments(
context,
&attrs_str,
&pat_and_id_str,
mk_sp(hi_pos, self.pat.span.lo()),
nested_shape,
false,
)
}
}
}

@ytmimi ytmimi added a-comments and removed needs-mcve needs a Minimal Complete and Verifiable Example labels Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-comments e-trailing whitespace error[internal]: left behind trailing whitespace
Projects
None yet
Development

No branches or pull requests

2 participants