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

Confusing error message on syntax error in code in doc comments #6109

Open
feds01 opened this issue Mar 10, 2024 · 10 comments
Open

Confusing error message on syntax error in code in doc comments #6109

feds01 opened this issue Mar 10, 2024 · 10 comments
Labels
a-comments good first issue Issues up for grabs, also good candidates for new rustfmt contributors needs-test only-with-option requires a non-default option value to reproduce

Comments

@feds01
Copy link

feds01 commented Mar 10, 2024

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

ytmimi commented Mar 11, 2024

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

ytmimi commented Mar 11, 2024

Linking the tracking issue format_code_in_doc_comments #3348

@ytmimi ytmimi added the only-with-option requires a non-default option value to reproduce label Mar 11, 2024
@feds01
Copy link
Author

feds01 commented Mar 14, 2024

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

ytmimi commented Mar 19, 2024

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

@ytmimi
Copy link
Contributor

ytmimi commented Mar 19, 2024

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

feds01 commented Mar 19, 2024

@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)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 24, 2024
…diagnostic, r=davidtwco

conditionally ignore fatal diagnostic in the SilentEmitter

This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in rust-lang#121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics, which lead to rust-lang/rustfmt#6109.

These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 24, 2024
…diagnostic, r=davidtwco

conditionally ignore fatal diagnostic in the SilentEmitter

This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in rust-lang#121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics, which lead to rust-lang/rustfmt#6109.

These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 24, 2024
Rollup merge of rust-lang#122737 - ytmimi:conditionally_ignore_fatal_diagnostic, r=davidtwco

conditionally ignore fatal diagnostic in the SilentEmitter

This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in rust-lang#121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics, which lead to rust-lang/rustfmt#6109.

These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
RenjiSann pushed a commit to RenjiSann/rust that referenced this issue Mar 25, 2024
…diagnostic, r=davidtwco

conditionally ignore fatal diagnostic in the SilentEmitter

This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in rust-lang#121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics, which lead to rust-lang/rustfmt#6109.

These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
@ytmimi
Copy link
Contributor

ytmimi commented Mar 25, 2024

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

ytmimi commented Mar 25, 2024

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

@ytmimi ytmimi added good first issue Issues up for grabs, also good candidates for new rustfmt contributors needs-test labels Mar 25, 2024
@feds01
Copy link
Author

feds01 commented Mar 25, 2024

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

@ytmimi
Copy link
Contributor

ytmimi commented Mar 25, 2024

@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.

github-actions bot pushed a commit to ytmimi/rustfmt that referenced this issue Apr 8, 2024
…c, r=davidtwco

conditionally ignore fatal diagnostic in the SilentEmitter

This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in #121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics, which lead to rust-lang#6109.

These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-comments good first issue Issues up for grabs, also good candidates for new rustfmt contributors needs-test only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

No branches or pull requests

2 participants