Skip to content

Commit 966f947

Browse files
authored
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 > ```
2 parents bc75590 + b674a60 commit 966f947

18 files changed

+151
-80
lines changed

src/cargo/util/frontmatter.rs

Lines changed: 60 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,18 @@ impl<'s> ScriptSource<'s> {
7272
format!(
7373
"found {fence_length} `{FENCE_CHAR}` in rust frontmatter, expected at least 3"
7474
),
75-
open_start..open_end,
76-
));
75+
raw.len()..raw.len(),
76+
).push_visible_span(open_start..open_end));
7777
}
7878
_ => {}
7979
}
8080
source.open = Some(open_start..open_end);
8181
let Some(info_nl) = input.find_slice("\n") else {
8282
return Err(FrontmatterError::new(
83-
format!("no closing `{fence_pattern}` found for frontmatter"),
84-
open_start..open_end,
85-
));
83+
format!("unclosed frontmatter; expected `{fence_pattern}`"),
84+
raw.len()..raw.len(),
85+
)
86+
.push_visible_span(open_start..open_end));
8687
};
8788
let info = input.next_slice(info_nl.start);
8889
let info = info.trim_matches(is_whitespace);
@@ -103,15 +104,20 @@ impl<'s> ScriptSource<'s> {
103104
let close_start = input.current_token_start();
104105
let _ = input.next_slice(len);
105106
let close_end = input.current_token_start();
107+
let fewer_dashes = fence_length - len;
106108
return Err(FrontmatterError::new(
107-
format!("closing code fence has too few `-`"),
109+
format!(
110+
"closing code fence has {fewer_dashes} less `-` than the opening fence"
111+
),
108112
close_start..close_end,
109-
));
113+
)
114+
.push_visible_span(open_start..open_end));
110115
}
111116
return Err(FrontmatterError::new(
112-
format!("no closing `{fence_pattern}` found for frontmatter"),
113-
open_start..open_end,
114-
));
117+
format!("unclosed frontmatter; expected `{fence_pattern}`"),
118+
raw.len()..raw.len(),
119+
)
120+
.push_visible_span(open_start..open_end));
115121
};
116122
let frontmatter_start = input.current_token_start() + 1; // skip nl from infostring
117123
let _ = input.next_slice(frontmatter_nl.start + 1);
@@ -128,13 +134,30 @@ impl<'s> ScriptSource<'s> {
128134
.unwrap_or_else(|| input.eof_offset()),
129135
);
130136
let content_start = input.current_token_start();
131-
let after_closing_fence = after_closing_fence.trim_matches(is_whitespace);
132-
if !after_closing_fence.is_empty() {
133-
// extra characters beyond the original fence pattern, even if they are extra `-`
137+
let extra_dashes = after_closing_fence
138+
.chars()
139+
.take_while(|b| *b == FENCE_CHAR)
140+
.count();
141+
if 0 < extra_dashes {
142+
let extra_start = close_end;
143+
let extra_end = extra_start + extra_dashes;
134144
return Err(FrontmatterError::new(
135-
format!("trailing characters found after frontmatter close"),
136-
close_end..content_start,
137-
));
145+
format!("closing code fence has {extra_dashes} more `-` than the opening fence"),
146+
extra_start..extra_end,
147+
)
148+
.push_visible_span(open_start..open_end));
149+
} else {
150+
let after_closing_fence = after_closing_fence.trim_matches(is_whitespace);
151+
if !after_closing_fence.is_empty() {
152+
// extra characters beyond the original fence pattern
153+
let after_start = after_closing_fence.offset_from(&raw);
154+
let after_end = after_start + after_closing_fence.len();
155+
return Err(FrontmatterError::new(
156+
format!("unexpected characters after frontmatter close"),
157+
after_start..after_end,
158+
)
159+
.push_visible_span(open_start..open_end));
160+
}
138161
}
139162

140163
source.content = content_start..content_end;
@@ -153,7 +176,9 @@ impl<'s> ScriptSource<'s> {
153176
return Err(FrontmatterError::new(
154177
format!("only one frontmatter is supported"),
155178
fence_start..fence_end,
156-
));
179+
)
180+
.push_visible_span(open_start..open_end)
181+
.push_visible_span(close_start..close_end));
157182
}
158183

159184
Ok(source)
@@ -270,23 +295,34 @@ fn is_whitespace(c: char) -> bool {
270295
#[derive(Debug)]
271296
pub struct FrontmatterError {
272297
message: String,
273-
span: Span,
298+
primary_span: Span,
299+
visible_spans: Vec<Span>,
274300
}
275301

276302
impl FrontmatterError {
277303
pub fn new(message: impl Into<String>, span: Span) -> Self {
278304
Self {
279305
message: message.into(),
280-
span,
306+
primary_span: span,
307+
visible_spans: Vec::new(),
281308
}
282309
}
283310

311+
pub fn push_visible_span(mut self, span: Span) -> Self {
312+
self.visible_spans.push(span);
313+
self
314+
}
315+
284316
pub fn message(&self) -> &str {
285317
self.message.as_str()
286318
}
287319

288-
pub fn span(&self) -> Span {
289-
self.span.clone()
320+
pub fn primary_span(&self) -> Span {
321+
self.primary_span.clone()
322+
}
323+
324+
pub fn visible_spans(&self) -> &[Span] {
325+
&self.visible_spans
290326
}
291327
}
292328

@@ -584,7 +620,7 @@ content: "\nfn main() {}\n"
584620
fn main() {}
585621
"#,
586622
),
587-
str!["trailing characters found after frontmatter close"],
623+
str!["closing code fence has 2 more `-` than the opening fence"],
588624
);
589625
}
590626

@@ -621,7 +657,7 @@ time="0.1.25"
621657
fn main() {}
622658
"#,
623659
),
624-
str!["trailing characters found after frontmatter close"],
660+
str!["closing code fence has 1 more `-` than the opening fence"],
625661
);
626662
}
627663

@@ -636,7 +672,7 @@ time="0.1.25"
636672
fn main() {}
637673
"#,
638674
),
639-
str!["no closing `---` found for frontmatter"],
675+
str!["unclosed frontmatter; expected `---`"],
640676
);
641677
}
642678
}

src/cargo/util/toml/embedded.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,21 @@ pub(super) fn expand_manifest(content: &str) -> Result<String, FrontmatterError>
1010
match source.info() {
1111
Some("cargo") | None => {}
1212
Some(other) => {
13+
let info_span = source.info_span().unwrap();
14+
let close_span = source.close_span().unwrap();
1315
if let Some(remainder) = other.strip_prefix("cargo,") {
1416
return Err(FrontmatterError::new(
15-
format!(
16-
"cargo does not support frontmatter infostring attributes like `{remainder}` at this time"
17-
),
18-
source.info_span().unwrap(),
19-
));
17+
format!("unsupported frontmatter infostring attributes: `{remainder}`"),
18+
info_span,
19+
)
20+
.push_visible_span(close_span));
2021
} else {
2122
return Err(FrontmatterError::new(
2223
format!(
23-
"frontmatter infostring `{other}` is unsupported by cargo; specify `cargo` for embedding a manifest"
24+
"unsupported frontmatter infostring `{other}`; specify `cargo` for embedding a manifest"
2425
),
25-
source.info_span().unwrap(),
26-
));
26+
info_span,
27+
).push_visible_span(close_span));
2728
}
2829
}
2930
}

src/cargo/util/toml/mod.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2783,7 +2783,7 @@ fn emit_frontmatter_diagnostic(
27832783
manifest_file: &Path,
27842784
gctx: &GlobalContext,
27852785
) -> anyhow::Error {
2786-
let span = e.span();
2786+
let primary_span = e.primary_span();
27872787

27882788
// Get the path to the manifest, relative to the cwd
27892789
let manifest_path = diff_paths(manifest_file, gctx.cwd())
@@ -2793,7 +2793,12 @@ fn emit_frontmatter_diagnostic(
27932793
let group = Group::with_title(Level::ERROR.primary_title(e.message())).element(
27942794
Snippet::source(contents)
27952795
.path(manifest_path)
2796-
.annotation(AnnotationKind::Primary.span(span)),
2796+
.annotation(AnnotationKind::Primary.span(primary_span))
2797+
.annotations(
2798+
e.visible_spans()
2799+
.iter()
2800+
.map(|s| AnnotationKind::Visible.span(s.clone())),
2801+
),
27972802
);
27982803

27992804
if let Err(err) = gctx.shell().print_report(&[group], true) {
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
[ERROR] frontmatter infostring `.toml` is unsupported by cargo; specify `cargo` for embedding a manifest
1+
[ERROR] unsupported frontmatter infostring `.toml`; specify `cargo` for embedding a manifest
22
--> script:1:4
33
|
44
1 | ---.toml
55
| ^^^^^
6+
2 | //~^ ERROR: invalid infostring for frontmatter
7+
3 | ---
8+
|
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
[ERROR] frontmatter infostring `Cargo.toml` is unsupported by cargo; specify `cargo` for embedding a manifest
1+
[ERROR] unsupported frontmatter infostring `Cargo.toml`; specify `cargo` for embedding a manifest
22
--> script:1:4
33
|
44
1 | ---Cargo.toml
55
| ^^^^^^^^^^
6+
2 | ---
7+
|
Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
[ERROR] trailing characters found after frontmatter close
1+
[ERROR] unexpected characters after frontmatter close
22
--> script:2:4
33
|
4-
2 | ---cargo
5-
| ____^
6-
3 | | //~^ ERROR: extra characters after frontmatter close are not allowed
7-
| |_^
4+
1 | ---
5+
2 | ---cargo
6+
| ^^^^^
Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
[ERROR] no closing `---` found for frontmatter
2-
--> script:1:1
3-
|
4-
1 | ---cargo
5-
| ^^^
1+
[ERROR] unclosed frontmatter; expected `---`
2+
--> script:14:56
3+
|
4+
1 | ---cargo
5+
...
6+
14 | // within them and get treated as a frontmatter close.
7+
| ^
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
[ERROR] frontmatter infostring `-toml` is unsupported by cargo; specify `cargo` for embedding a manifest
1+
[ERROR] unsupported frontmatter infostring `-toml`; specify `cargo` for embedding a manifest
22
--> script:1:5
33
|
44
1 | --- -toml
55
| ^^^^^
6+
2 | //~^ ERROR: invalid infostring for frontmatter
7+
3 | ---
8+
|
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
[ERROR] frontmatter infostring `Cargo-toml` is unsupported by cargo; specify `cargo` for embedding a manifest
1+
[ERROR] unsupported frontmatter infostring `Cargo-toml`; specify `cargo` for embedding a manifest
22
--> script:1:5
33
|
44
1 | --- Cargo-toml
55
| ^^^^^^^^^^
6+
2 | ---
7+
|
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
[ERROR] cargo does not support frontmatter infostring attributes like `clippy` at this time
1+
[ERROR] unsupported frontmatter infostring attributes: `clippy`
22
--> script:1:4
33
|
44
1 | ---cargo,clippy
55
| ^^^^^^^^^^^^
6+
2 | //~^ ERROR: invalid infostring for frontmatter
7+
3 | ---
8+
|

0 commit comments

Comments
 (0)