Confusing error message on syntax error in code in doc comments

[feds01]

When enabling format_code_in_doc_comments, the formatter reports that there is a syntax issue in a code block. However the syntax issue source location seems to be confusing/ambigious:

Running rustfmt on the following code snippet produces a confusing error message:

/// Some doc comment with code snippet:
///```
/// '\u{1F}
/// ```
pub struct Code {}

fn main() {}

The error that is produced:

error[E0762]: unterminated character literal
 --> <stdin>:2:5
  |
2 |     '\u{1F}
  |     ^^^^^^^

This is not explicitly a bug, but nonetheless is confusing as it does not indicate the true origin of the issue.

[ytmimi]

Thanks for the report. Just want to understand how you're calling rustfmt. Are you running rustfmt directly on a file or is it your editor that's invoking rustfmt?

[ytmimi]

Linking the tracking issue format_code_in_doc_comments #3348

[feds01]

Calling through cargo fmt in the project workspace - hence why the error pops up and its unclear where the source of the error is.

[ytmimi]

@feds01 by the way, what version of rustfmt are you using?

[ytmimi]

@davidtwco I'm assuming this might have been caused by sharing the SilentEmitter between rustc and rustfmt in rust-lang/rust#121301. I believe this is coming up because rustc's SilentEmitter is defined to still emit Fatal diagnostics, while rustfmt's SilentEmitter emits nothing. I'm able to reproduce the issue using nightly rustfmt, but not when building rustfmt from source, presumably because we haven't pulled these changes back into rustfmt yet.

Do you think it's possible to modify rustc's SilentEmitter to conditionally ignore Fatal diagnostics as well? For rustfmt I think that's the behavior that we'd want.

[feds01]

@feds01 by the way, what version of rustfmt are you using?

$ cargo --version --verbose

cargo 1.78.0-nightly (a4c63fe53 2024-03-06)
release: 1.78.0-nightly
commit-hash: a4c63fe5388beaa09e5f91196c86addab0a03580
commit-date: 2024-03-06
host: aarch64-apple-darwin
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.1.2 (sys:0.4.72+curl-8.6.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 14.1.1 [64-bit]

cargo fmt --version
rustfmt 1.7.0-nightly (2d24fe59 2024-03-09)

[ytmimi]

I just tested with the nightly-2024-03-25 toolchain (rustfmt 1.7.0-nightly (0824b300 2024-03-24)) and this issue should be resolved.

@feds01 when you have a moment could you also confirm this:

Steps to confirm:

• rustup install nightly-2024-03-25
• rustfmt +nightly-2024-03-25 --config=format_code_in_doc_comments=true /path/to/test/file.rs

[ytmimi]

Before closing this issue I think it would be great if we could add a test case to prevent a regression

[feds01]

It is now not reporting any issues with the file, is the desired behaviour?

[ytmimi]

@feds01 In this case yes. The previous behavior was for rustfmt to ignore the diagnostic message produced by the rustc parser, however there were some internal changes made in rust-lang/rust that changed that behavior. rust-lang/rust#122737 restored rustfmts previous behavior of ignoring parser errors when formatting code in doc comments.

[giordan12]

Hi, I'm planning to help with this as my first contribution. From looking at the contributing.md doc I understand that I would need to create single target file with the code mentioned at the top of this issue?

[ytmimi]

@giordan12 that's great. I appreciate you wanting to take this one one. Yes, you can add a test case in the target directory, and in addition to that I think a new standalone test case should be added in https://github.com/rust-lang/rustfmt/blob/master/tests/rustfmt/main.rs to validate that no errors are emitted to stdout or stderr.

[giordan12]

@ytmimi appreciate the guidance, just created the PR. Let me know if there is something to add (I'm expecting there will be :) )

[ytmimi]

closing now that #6232 is merged

[feds01]

Thanks for this guys ♥️ Let me know if I can also help with anything, I'd love to contribute!

[giordan12]

@ytmimi I was wondering if it's worth making it clear in Contributing.md that besides creating source and target files an actual test is needed? The current text makes it look like it's enough to make the test files IMO

[ytmimi]

I don't think it's necessary. It's mostly this specific case since I wanted to add a test to ensure nothing was being output to stdout and stderr. Typically a source and target file are all that's required