fix(frontmatter): Improve error quality (#15972)

### What does this PR try to resolve?

The wording for our errors was indirect and things weren't as clear
without more context.

### How to test and review this PR?

### Notes

Example of rustc's versions of these errors:

1

> ```
> error: unclosed frontmatter
>   --> $DIR/frontmatter-whitespace-2.rs:1:1
>    |
> LL | / ---cargo
> ...  |
> LL | |
>    | |_^
>    |
> note: frontmatter opening here was not closed
>   --> $DIR/frontmatter-whitespace-2.rs:1:1
>    |
> LL | ---cargo
>    | ^^^
> ```

2

> ```
> error: extra characters after frontmatter close are not allowed
>   --> $DIR/extra-after-end.rs:2:1
>    |
> LL | ---cargo
>    | ^^^^^^^^
> ```

3

> ```
> error: frontmatter close does not match the opening
>   --> $DIR/mismatch-1.rs:1:1
>    |
> LL |   ---cargo
>    |   ^--
>    |   |
>    |  _the opening here has 3 dashes...
>    | |
> LL | |
> LL | | ----
>    | |_---^
>    |   |
>    |   ...while the close has 4 dashes
> ```
This commit is contained in:
Eric Huss 2025-09-16 17:24:45 +00:00 committed by GitHub
commit 966f94733b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
18 changed files with 151 additions and 80 deletions

View File

@ -72,17 +72,18 @@ impl<'s> ScriptSource<'s> {
format!( format!(
"found {fence_length} `{FENCE_CHAR}` in rust frontmatter, expected at least 3" "found {fence_length} `{FENCE_CHAR}` in rust frontmatter, expected at least 3"
), ),
open_start..open_end, raw.len()..raw.len(),
)); ).push_visible_span(open_start..open_end));
} }
_ => {} _ => {}
} }
source.open = Some(open_start..open_end); source.open = Some(open_start..open_end);
let Some(info_nl) = input.find_slice("\n") else { let Some(info_nl) = input.find_slice("\n") else {
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!("no closing `{fence_pattern}` found for frontmatter"), format!("unclosed frontmatter; expected `{fence_pattern}`"),
open_start..open_end, raw.len()..raw.len(),
)); )
.push_visible_span(open_start..open_end));
}; };
let info = input.next_slice(info_nl.start); let info = input.next_slice(info_nl.start);
let info = info.trim_matches(is_whitespace); let info = info.trim_matches(is_whitespace);
@ -103,15 +104,20 @@ impl<'s> ScriptSource<'s> {
let close_start = input.current_token_start(); let close_start = input.current_token_start();
let _ = input.next_slice(len); let _ = input.next_slice(len);
let close_end = input.current_token_start(); let close_end = input.current_token_start();
let fewer_dashes = fence_length - len;
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!("closing code fence has too few `-`"), format!(
"closing code fence has {fewer_dashes} less `-` than the opening fence"
),
close_start..close_end, close_start..close_end,
)); )
.push_visible_span(open_start..open_end));
} }
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!("no closing `{fence_pattern}` found for frontmatter"), format!("unclosed frontmatter; expected `{fence_pattern}`"),
open_start..open_end, raw.len()..raw.len(),
)); )
.push_visible_span(open_start..open_end));
}; };
let frontmatter_start = input.current_token_start() + 1; // skip nl from infostring let frontmatter_start = input.current_token_start() + 1; // skip nl from infostring
let _ = input.next_slice(frontmatter_nl.start + 1); let _ = input.next_slice(frontmatter_nl.start + 1);
@ -128,13 +134,30 @@ impl<'s> ScriptSource<'s> {
.unwrap_or_else(|| input.eof_offset()), .unwrap_or_else(|| input.eof_offset()),
); );
let content_start = input.current_token_start(); let content_start = input.current_token_start();
let after_closing_fence = after_closing_fence.trim_matches(is_whitespace); let extra_dashes = after_closing_fence
if !after_closing_fence.is_empty() { .chars()
// extra characters beyond the original fence pattern, even if they are extra `-` .take_while(|b| *b == FENCE_CHAR)
.count();
if 0 < extra_dashes {
let extra_start = close_end;
let extra_end = extra_start + extra_dashes;
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!("trailing characters found after frontmatter close"), format!("closing code fence has {extra_dashes} more `-` than the opening fence"),
close_end..content_start, extra_start..extra_end,
)); )
.push_visible_span(open_start..open_end));
} else {
let after_closing_fence = after_closing_fence.trim_matches(is_whitespace);
if !after_closing_fence.is_empty() {
// extra characters beyond the original fence pattern
let after_start = after_closing_fence.offset_from(&raw);
let after_end = after_start + after_closing_fence.len();
return Err(FrontmatterError::new(
format!("unexpected characters after frontmatter close"),
after_start..after_end,
)
.push_visible_span(open_start..open_end));
}
} }
source.content = content_start..content_end; source.content = content_start..content_end;
@ -153,7 +176,9 @@ impl<'s> ScriptSource<'s> {
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!("only one frontmatter is supported"), format!("only one frontmatter is supported"),
fence_start..fence_end, fence_start..fence_end,
)); )
.push_visible_span(open_start..open_end)
.push_visible_span(close_start..close_end));
} }
Ok(source) Ok(source)
@ -270,23 +295,34 @@ fn is_whitespace(c: char) -> bool {
#[derive(Debug)] #[derive(Debug)]
pub struct FrontmatterError { pub struct FrontmatterError {
message: String, message: String,
span: Span, primary_span: Span,
visible_spans: Vec<Span>,
} }
impl FrontmatterError { impl FrontmatterError {
pub fn new(message: impl Into<String>, span: Span) -> Self { pub fn new(message: impl Into<String>, span: Span) -> Self {
Self { Self {
message: message.into(), message: message.into(),
span, primary_span: span,
visible_spans: Vec::new(),
} }
} }
pub fn push_visible_span(mut self, span: Span) -> Self {
self.visible_spans.push(span);
self
}
pub fn message(&self) -> &str { pub fn message(&self) -> &str {
self.message.as_str() self.message.as_str()
} }
pub fn span(&self) -> Span { pub fn primary_span(&self) -> Span {
self.span.clone() self.primary_span.clone()
}
pub fn visible_spans(&self) -> &[Span] {
&self.visible_spans
} }
} }
@ -584,7 +620,7 @@ content: "\nfn main() {}\n"
fn main() {} fn main() {}
"#, "#,
), ),
str!["trailing characters found after frontmatter close"], str!["closing code fence has 2 more `-` than the opening fence"],
); );
} }
@ -621,7 +657,7 @@ time="0.1.25"
fn main() {} fn main() {}
"#, "#,
), ),
str!["trailing characters found after frontmatter close"], str!["closing code fence has 1 more `-` than the opening fence"],
); );
} }
@ -636,7 +672,7 @@ time="0.1.25"
fn main() {} fn main() {}
"#, "#,
), ),
str!["no closing `---` found for frontmatter"], str!["unclosed frontmatter; expected `---`"],
); );
} }
} }

View File

@ -10,20 +10,21 @@ pub(super) fn expand_manifest(content: &str) -> Result<String, FrontmatterError>
match source.info() { match source.info() {
Some("cargo") | None => {} Some("cargo") | None => {}
Some(other) => { Some(other) => {
let info_span = source.info_span().unwrap();
let close_span = source.close_span().unwrap();
if let Some(remainder) = other.strip_prefix("cargo,") { if let Some(remainder) = other.strip_prefix("cargo,") {
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!( format!("unsupported frontmatter infostring attributes: `{remainder}`"),
"cargo does not support frontmatter infostring attributes like `{remainder}` at this time" info_span,
), )
source.info_span().unwrap(), .push_visible_span(close_span));
));
} else { } else {
return Err(FrontmatterError::new( return Err(FrontmatterError::new(
format!( format!(
"frontmatter infostring `{other}` is unsupported by cargo; specify `cargo` for embedding a manifest" "unsupported frontmatter infostring `{other}`; specify `cargo` for embedding a manifest"
), ),
source.info_span().unwrap(), info_span,
)); ).push_visible_span(close_span));
} }
} }
} }

View File

@ -2783,7 +2783,7 @@ fn emit_frontmatter_diagnostic(
manifest_file: &Path, manifest_file: &Path,
gctx: &GlobalContext, gctx: &GlobalContext,
) -> anyhow::Error { ) -> anyhow::Error {
let span = e.span(); let primary_span = e.primary_span();
// Get the path to the manifest, relative to the cwd // Get the path to the manifest, relative to the cwd
let manifest_path = diff_paths(manifest_file, gctx.cwd()) let manifest_path = diff_paths(manifest_file, gctx.cwd())
@ -2793,7 +2793,12 @@ fn emit_frontmatter_diagnostic(
let group = Group::with_title(Level::ERROR.primary_title(e.message())).element( let group = Group::with_title(Level::ERROR.primary_title(e.message())).element(
Snippet::source(contents) Snippet::source(contents)
.path(manifest_path) .path(manifest_path)
.annotation(AnnotationKind::Primary.span(span)), .annotation(AnnotationKind::Primary.span(primary_span))
.annotations(
e.visible_spans()
.iter()
.map(|s| AnnotationKind::Visible.span(s.clone())),
),
); );
if let Err(err) = gctx.shell().print_report(&[group], true) { if let Err(err) = gctx.shell().print_report(&[group], true) {

View File

@ -1,5 +1,8 @@
[ERROR] frontmatter infostring `.toml` is unsupported by cargo; specify `cargo` for embedding a manifest [ERROR] unsupported frontmatter infostring `.toml`; specify `cargo` for embedding a manifest
--> script:1:4 --> script:1:4
| |
1 | ---.toml 1 | ---.toml
| ^^^^^ | ^^^^^
2 | //~^ ERROR: invalid infostring for frontmatter
3 | ---
|

View File

@ -1,5 +1,7 @@
[ERROR] frontmatter infostring `Cargo.toml` is unsupported by cargo; specify `cargo` for embedding a manifest [ERROR] unsupported frontmatter infostring `Cargo.toml`; specify `cargo` for embedding a manifest
--> script:1:4 --> script:1:4
| |
1 | ---Cargo.toml 1 | ---Cargo.toml
| ^^^^^^^^^^ | ^^^^^^^^^^
2 | ---
|

View File

@ -1,7 +1,6 @@
[ERROR] trailing characters found after frontmatter close [ERROR] unexpected characters after frontmatter close
--> script:2:4 --> script:2:4
| |
2 | ---cargo 1 | ---
| ____^ 2 | ---cargo
3 | | //~^ ERROR: extra characters after frontmatter close are not allowed | ^^^^^
| |_^

View File

@ -1,5 +1,7 @@
[ERROR] no closing `---` found for frontmatter [ERROR] unclosed frontmatter; expected `---`
--> script:1:1 --> script:14:56
| |
1 | ---cargo 1 | ---cargo
| ^^^ ...
14 | // within them and get treated as a frontmatter close.
| ^

View File

@ -1,5 +1,8 @@
[ERROR] frontmatter infostring `-toml` is unsupported by cargo; specify `cargo` for embedding a manifest [ERROR] unsupported frontmatter infostring `-toml`; specify `cargo` for embedding a manifest
--> script:1:5 --> script:1:5
| |
1 | --- -toml 1 | --- -toml
| ^^^^^ | ^^^^^
2 | //~^ ERROR: invalid infostring for frontmatter
3 | ---
|

View File

@ -1,5 +1,7 @@
[ERROR] frontmatter infostring `Cargo-toml` is unsupported by cargo; specify `cargo` for embedding a manifest [ERROR] unsupported frontmatter infostring `Cargo-toml`; specify `cargo` for embedding a manifest
--> script:1:5 --> script:1:5
| |
1 | --- Cargo-toml 1 | --- Cargo-toml
| ^^^^^^^^^^ | ^^^^^^^^^^
2 | ---
|

View File

@ -1,5 +1,8 @@
[ERROR] cargo does not support frontmatter infostring attributes like `clippy` at this time [ERROR] unsupported frontmatter infostring attributes: `clippy`
--> script:1:4 --> script:1:4
| |
1 | ---cargo,clippy 1 | ---cargo,clippy
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
2 | //~^ ERROR: invalid infostring for frontmatter
3 | ---
|

View File

@ -1,7 +1,7 @@
[ERROR] trailing characters found after frontmatter close [ERROR] closing code fence has 1 more `-` than the opening fence
--> script:3:4 --> script:3:4
| |
3 | ---- 1 | ---cargo
| ____^ 2 | //~^ ERROR: frontmatter close does not match the opening
4 | | 3 | ----
| |_^ | ^

View File

@ -1,5 +1,7 @@
[ERROR] closing code fence has too few `-` [ERROR] closing code fence has 1 less `-` than the opening fence
--> script:3:1 --> script:3:1
| |
1 | ----cargo
2 | //~^ ERROR: frontmatter close does not match the opening
3 | ---cargo 3 | ---cargo
| ^^^ | ^^^

View File

@ -1,5 +1,8 @@
[ERROR] only one frontmatter is supported [ERROR] only one frontmatter is supported
--> script:4:1 --> script:4:1
| |
1 | ---
2 | ---
3 |
4 | --- 4 | ---
| ^^^ | ^^^

View File

@ -1,5 +1,7 @@
[ERROR] no closing `----` found for frontmatter [ERROR] unclosed frontmatter; expected `----`
--> script:1:1 --> script:10:14
| |
1 | ----cargo 1 | ----cargo
| ^^^^ ...
10 | fn main() {}
| ^

View File

@ -1,5 +1,7 @@
[ERROR] no closing `----` found for frontmatter [ERROR] unclosed frontmatter; expected `----`
--> script:1:1 --> script:15:3
| |
1 | ----cargo 1 | ----cargo
| ^^^^ ...
15 | }
| ^

View File

@ -1,5 +1,7 @@
[ERROR] no closing `----` found for frontmatter [ERROR] unclosed frontmatter; expected `----`
--> script:1:1 --> script:16:47
| |
1 | ----cargo 1 | ----cargo
| ^^^^ ...
16 | //~^ ERROR: unexpected closing delimiter: `}`
| ^

View File

@ -1,5 +1,7 @@
[ERROR] no closing `----` found for frontmatter [ERROR] unclosed frontmatter; expected `----`
--> script:1:1 --> script:9:14
| |
1 | ----cargo 1 | ----cargo
| ^^^^ ...
9 | fn main() {}
| ^

View File

@ -1,5 +1,7 @@
[ERROR] no closing `----` found for frontmatter [ERROR] unclosed frontmatter; expected `----`
--> script:1:1 --> script:10:14
| |
1 | ----cargo 1 | ----cargo
| ^^^^ ...
10 | fn main() {}
| ^