Skip to content

Commit 31b833d

Browse files
committed
feat: Add rustc style errors for manifest parsing
1 parent 6982b44 commit 31b833d

15 files changed

+306
-223
lines changed

Cargo.lock

+11
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ edition = "2021"
1616
license = "MIT OR Apache-2.0"
1717

1818
[workspace.dependencies]
19+
annotate-snippets = "0.10.0"
1920
anstream = "0.6.5"
2021
anstyle = "1.0.4"
2122
anyhow = "1.0.75"
@@ -137,6 +138,7 @@ name = "cargo"
137138
path = "src/cargo/lib.rs"
138139

139140
[dependencies]
141+
annotate-snippets.workspace = true
140142
anstream.workspace = true
141143
anstyle.workspace = true
142144
anyhow.workspace = true

src/cargo/util/toml/mod.rs

+107-7
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1+
use annotate_snippets::{Annotation, AnnotationType, Renderer, Slice, Snippet, SourceAnnotation};
12
use std::collections::{BTreeMap, BTreeSet, HashMap};
23
use std::ffi::OsStr;
34
use std::path::{Path, PathBuf};
45
use std::rc::Rc;
56
use std::str::{self, FromStr};
67

8+
use crate::AlreadyPrintedError;
79
use anyhow::{anyhow, bail, Context as _};
810
use cargo_platform::Platform;
911
use cargo_util::paths;
1012
use itertools::Itertools;
1113
use lazycell::LazyCell;
14+
use pathdiff::diff_paths;
1215
use tracing::{debug, trace};
1316
use url::Url;
1417

@@ -24,6 +27,7 @@ use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
2427
use crate::util::errors::{CargoResult, ManifestError};
2528
use crate::util::interning::InternedString;
2629
use crate::util::restricted_names;
30+
use crate::util::style::{ERROR, NOTE, WARN};
2731
use crate::util::{
2832
self, config::ConfigRelativePath, validate_package_name, Config, IntoUrl, OptVersionReq,
2933
};
@@ -46,7 +50,7 @@ pub fn read_manifest(
4650
path: &Path,
4751
source_id: SourceId,
4852
config: &Config,
49-
) -> Result<(EitherManifest, Vec<PathBuf>), ManifestError> {
53+
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
5054
trace!(
5155
"read_manifest; path={}; source-id={}",
5256
path.display(),
@@ -59,15 +63,23 @@ pub fn read_manifest(
5963
return Err(ManifestError::new(
6064
anyhow::anyhow!("parsing `{}` requires `-Zscript`", path.display()),
6165
path.into(),
62-
));
66+
))?;
6367
}
6468
contents = embedded::expand_manifest(&contents, path, config)
6569
.map_err(|err| ManifestError::new(err, path.into()))?;
6670
}
6771

68-
read_manifest_from_str(&contents, path, embedded, source_id, config)
69-
.with_context(|| format!("failed to parse manifest at `{}`", path.display()))
70-
.map_err(|err| ManifestError::new(err, path.into()))
72+
read_manifest_from_str(&contents, path, embedded, source_id, config).map_err(|err| {
73+
if err.is::<AlreadyPrintedError>() {
74+
err
75+
} else {
76+
ManifestError::new(
77+
err.context(format!("failed to parse manifest at `{}`", path.display())),
78+
path.into(),
79+
)
80+
.into()
81+
}
82+
})
7183
}
7284

7385
/// See also `bin/cargo/commands/run.rs`s `is_manifest_command`
@@ -97,11 +109,69 @@ fn read_manifest_from_str(
97109

98110
let mut unused = BTreeSet::new();
99111
let deserializer = toml::de::Deserializer::new(contents);
100-
let manifest: manifest::TomlManifest = serde_ignored::deserialize(deserializer, |path| {
112+
let manifest: manifest::TomlManifest = match serde_ignored::deserialize(deserializer, |path| {
101113
let mut key = String::new();
102114
stringify(&mut key, &path);
103115
unused.insert(key);
104-
})?;
116+
}) {
117+
Ok(manifest) => manifest,
118+
Err(e) => {
119+
return if let Some(span) = e.span() {
120+
let (line, column) = translate_position(contents.as_bytes(), span.start);
121+
let lines = contents.lines();
122+
let line_count = lines.clone().count();
123+
let mut source = lines.skip(line).next().unwrap_or_default().to_string();
124+
// Add back the new line that was stripped by `.lines()`
125+
if line_count > line + 1 {
126+
source.push('\n');
127+
// If we are trying to highlight past the end of the file, add a
128+
// placeholder character to the end of the line.
129+
} else if span.end >= source.len() - 1 {
130+
source.push('∅');
131+
}
132+
// Make sure we don't try to highlight past the end of the line,
133+
// but also make sure we are highlighting at least one character
134+
let highlight_end = (column + (span.end - span.start))
135+
.min(source.len())
136+
.max(column + 1);
137+
// Get the path to the manifest, relative to the cwd
138+
let manifest_path = diff_paths(manifest_file, config.cwd())
139+
.unwrap_or(manifest_file.to_path_buf())
140+
.display()
141+
.to_string();
142+
let snippet = Snippet {
143+
title: Some(Annotation {
144+
id: None,
145+
label: Some(e.message()),
146+
annotation_type: AnnotationType::Error,
147+
}),
148+
footer: vec![],
149+
slices: vec![Slice {
150+
source: &source,
151+
line_start: line + 1,
152+
origin: Some(manifest_path.as_str()),
153+
annotations: vec![SourceAnnotation {
154+
range: (column, highlight_end),
155+
label: "",
156+
annotation_type: AnnotationType::Error,
157+
}],
158+
fold: false,
159+
}],
160+
};
161+
let renderer = Renderer::styled()
162+
.error(ERROR)
163+
.warning(WARN)
164+
.info(NOTE)
165+
.note(NOTE)
166+
.help(NOTE)
167+
.line_no(NOTE);
168+
writeln!(config.shell().err(), "{}", renderer.render(snippet))?;
169+
Err(AlreadyPrintedError::new(e.into()).into())
170+
} else {
171+
Err(e.into())
172+
};
173+
}
174+
};
105175
let add_unused = |warnings: &mut Warnings| {
106176
for key in unused {
107177
warnings.add_warning(format!("unused manifest key: {}", key));
@@ -2161,3 +2231,33 @@ impl ResolveToPath for ConfigRelativePath {
21612231
self.resolve_path(c)
21622232
}
21632233
}
2234+
2235+
fn translate_position(input: &[u8], index: usize) -> (usize, usize) {
2236+
if input.is_empty() {
2237+
return (0, index);
2238+
}
2239+
2240+
let safe_index = index.min(input.len() - 1);
2241+
let column_offset = index - safe_index;
2242+
let index = safe_index;
2243+
2244+
let nl = input[0..index]
2245+
.iter()
2246+
.rev()
2247+
.enumerate()
2248+
.find(|(_, b)| **b == b'\n')
2249+
.map(|(nl, _)| index - nl - 1);
2250+
let line_start = match nl {
2251+
Some(nl) => nl + 1,
2252+
None => 0,
2253+
};
2254+
let line = input[0..line_start].iter().filter(|b| **b == b'\n').count();
2255+
let line = line;
2256+
2257+
let column = std::str::from_utf8(&input[line_start..=index])
2258+
.map(|s| s.chars().count() - 1)
2259+
.unwrap_or_else(|_| index - line_start);
2260+
let column = column + column_offset;
2261+
2262+
(line, column)
2263+
}

tests/testsuite/bad_config.rs

+39-51
Original file line numberDiff line numberDiff line change
@@ -449,15 +449,13 @@ fn malformed_override() {
449449
.with_status(101)
450450
.with_stderr(
451451
"\
452-
[ERROR] failed to parse manifest at `[..]`
453-
454-
Caused by:
455-
TOML parse error at line 8, column 27
456-
|
457-
8 | native = {
458-
| ^
459-
invalid inline table
460-
expected `}`
452+
[ERROR] invalid inline table
453+
expected `}`
454+
--> Cargo.toml:8:27
455+
|
456+
8 | native = {
457+
| ^
458+
|
461459
",
462460
)
463461
.run();
@@ -1282,14 +1280,12 @@ fn bad_dependency() {
12821280
.with_status(101)
12831281
.with_stderr(
12841282
"\
1285-
error: failed to parse manifest at `[..]`
1286-
1287-
Caused by:
1288-
TOML parse error at line 8, column 23
1289-
|
1290-
8 | bar = 3
1291-
| ^
1292-
invalid type: integer `3`, expected a version string like [..]
1283+
[ERROR] invalid type: integer `3`, expected a version string like [..]
1284+
--> Cargo.toml:8:23
1285+
|
1286+
8 | bar = 3
1287+
| ^
1288+
|
12931289
",
12941290
)
12951291
.run();
@@ -1317,14 +1313,12 @@ fn bad_debuginfo() {
13171313
.with_status(101)
13181314
.with_stderr(
13191315
"\
1320-
error: failed to parse manifest [..]
1321-
1322-
Caused by:
1323-
TOML parse error at line 8, column 25
1324-
|
1325-
8 | debug = 'a'
1326-
| ^^^
1327-
invalid value: string \"a\", expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
1316+
[ERROR] invalid value: string \"a\", expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
1317+
--> Cargo.toml:8:25
1318+
|
1319+
8 | debug = 'a'
1320+
| ^^^
1321+
|
13281322
",
13291323
)
13301324
.run();
@@ -1352,14 +1346,12 @@ fn bad_debuginfo2() {
13521346
.with_status(101)
13531347
.with_stderr(
13541348
"\
1355-
error: failed to parse manifest at `[..]`
1356-
1357-
Caused by:
1358-
TOML parse error at line 8, column 25
1359-
|
1360-
8 | debug = 3.6
1361-
| ^^^
1362-
invalid type: floating point `3.6`, expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
1349+
[ERROR] invalid type: floating point `3.6`, expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
1350+
--> Cargo.toml:8:25
1351+
|
1352+
8 | debug = 3.6
1353+
| ^^^
1354+
|
13631355
",
13641356
)
13651357
.run();
@@ -1385,14 +1377,12 @@ fn bad_opt_level() {
13851377
.with_status(101)
13861378
.with_stderr(
13871379
"\
1388-
error: failed to parse manifest at `[..]`
1389-
1390-
Caused by:
1391-
TOML parse error at line 6, column 25
1392-
|
1393-
6 | build = 3
1394-
| ^
1395-
invalid type: integer `3`, expected a boolean or string
1380+
[ERROR] invalid type: integer `3`, expected a boolean or string
1381+
--> Cargo.toml:6:25
1382+
|
1383+
6 | build = 3
1384+
| ^
1385+
|
13961386
",
13971387
)
13981388
.run();
@@ -1685,16 +1675,14 @@ fn bad_trim_paths() {
16851675
p.cargo("check -Ztrim-paths")
16861676
.masquerade_as_nightly_cargo(&["trim-paths"])
16871677
.with_status(101)
1688-
.with_stderr(
1689-
r#"error: failed to parse manifest at `[..]`
1690-
1691-
Caused by:
1692-
TOML parse error at line 7, column 30
1693-
|
1694-
7 | trim-paths = "split-debuginfo"
1695-
| ^^^^^^^^^^^^^^^^^
1696-
expected a boolean, "none", "diagnostics", "macro", "object", "all", or an array with these options
1697-
"#,
1678+
.with_stderr("\
1679+
[ERROR] expected a boolean, \"none\", \"diagnostics\", \"macro\", \"object\", \"all\", or an array with these options
1680+
--> Cargo.toml:7:30
1681+
|
1682+
7 | trim-paths = \"split-debuginfo\"
1683+
| ^^^^^^^^^^^^^^^^^
1684+
|
1685+
",
16981686
)
16991687
.run();
17001688
}

0 commit comments

Comments
 (0)