Skip to content

Commit

Permalink
feat: Add rustc style errors for manifest parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
Muscraft committed Dec 14, 2023
1 parent 6982b44 commit 31b833d
Show file tree
Hide file tree
Showing 15 changed files with 306 additions and 223 deletions.
11 changes: 11 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ edition = "2021"
license = "MIT OR Apache-2.0"

[workspace.dependencies]
annotate-snippets = "0.10.0"
anstream = "0.6.5"
anstyle = "1.0.4"
anyhow = "1.0.75"
Expand Down Expand Up @@ -137,6 +138,7 @@ name = "cargo"
path = "src/cargo/lib.rs"

[dependencies]
annotate-snippets.workspace = true
anstream.workspace = true
anstyle.workspace = true
anyhow.workspace = true
Expand Down
114 changes: 107 additions & 7 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
use annotate_snippets::{Annotation, AnnotationType, Renderer, Slice, Snippet, SourceAnnotation};
use std::collections::{BTreeMap, BTreeSet, HashMap};
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::str::{self, FromStr};

use crate::AlreadyPrintedError;
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
use cargo_util::paths;
use itertools::Itertools;
use lazycell::LazyCell;
use pathdiff::diff_paths;
use tracing::{debug, trace};
use url::Url;

Expand All @@ -24,6 +27,7 @@ use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::restricted_names;
use crate::util::style::{ERROR, NOTE, WARN};
use crate::util::{
self, config::ConfigRelativePath, validate_package_name, Config, IntoUrl, OptVersionReq,
};
Expand All @@ -46,7 +50,7 @@ pub fn read_manifest(
path: &Path,
source_id: SourceId,
config: &Config,
) -> Result<(EitherManifest, Vec<PathBuf>), ManifestError> {
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
trace!(
"read_manifest; path={}; source-id={}",
path.display(),
Expand All @@ -59,15 +63,23 @@ pub fn read_manifest(
return Err(ManifestError::new(
anyhow::anyhow!("parsing `{}` requires `-Zscript`", path.display()),
path.into(),
));
))?;
}
contents = embedded::expand_manifest(&contents, path, config)
.map_err(|err| ManifestError::new(err, path.into()))?;
}

read_manifest_from_str(&contents, path, embedded, source_id, config)
.with_context(|| format!("failed to parse manifest at `{}`", path.display()))
.map_err(|err| ManifestError::new(err, path.into()))
read_manifest_from_str(&contents, path, embedded, source_id, config).map_err(|err| {
if err.is::<AlreadyPrintedError>() {
err
} else {
ManifestError::new(
err.context(format!("failed to parse manifest at `{}`", path.display())),
path.into(),
)
.into()
}
})
}

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

let mut unused = BTreeSet::new();
let deserializer = toml::de::Deserializer::new(contents);
let manifest: manifest::TomlManifest = serde_ignored::deserialize(deserializer, |path| {
let manifest: manifest::TomlManifest = match serde_ignored::deserialize(deserializer, |path| {
let mut key = String::new();
stringify(&mut key, &path);
unused.insert(key);
})?;
}) {
Ok(manifest) => manifest,
Err(e) => {
return if let Some(span) = e.span() {
let (line, column) = translate_position(contents.as_bytes(), span.start);
let lines = contents.lines();
let line_count = lines.clone().count();
let mut source = lines.skip(line).next().unwrap_or_default().to_string();
// Add back the new line that was stripped by `.lines()`
if line_count > line + 1 {
source.push('\n');
// If we are trying to highlight past the end of the file, add a
// placeholder character to the end of the line.
} else if span.end >= source.len() - 1 {
source.push('∅');
}
// Make sure we don't try to highlight past the end of the line,
// but also make sure we are highlighting at least one character
let highlight_end = (column + (span.end - span.start))
.min(source.len())
.max(column + 1);
// Get the path to the manifest, relative to the cwd
let manifest_path = diff_paths(manifest_file, config.cwd())
.unwrap_or(manifest_file.to_path_buf())
.display()
.to_string();
let snippet = Snippet {
title: Some(Annotation {
id: None,
label: Some(e.message()),
annotation_type: AnnotationType::Error,
}),
footer: vec![],
slices: vec![Slice {
source: &source,
line_start: line + 1,
origin: Some(manifest_path.as_str()),
annotations: vec![SourceAnnotation {
range: (column, highlight_end),
label: "",
annotation_type: AnnotationType::Error,
}],
fold: false,
}],
};
let renderer = Renderer::styled()
.error(ERROR)
.warning(WARN)
.info(NOTE)
.note(NOTE)
.help(NOTE)
.line_no(NOTE);
writeln!(config.shell().err(), "{}", renderer.render(snippet))?;
Err(AlreadyPrintedError::new(e.into()).into())
} else {
Err(e.into())
};
}
};
let add_unused = |warnings: &mut Warnings| {
for key in unused {
warnings.add_warning(format!("unused manifest key: {}", key));
Expand Down Expand Up @@ -2161,3 +2231,33 @@ impl ResolveToPath for ConfigRelativePath {
self.resolve_path(c)
}
}

fn translate_position(input: &[u8], index: usize) -> (usize, usize) {
if input.is_empty() {
return (0, index);
}

let safe_index = index.min(input.len() - 1);
let column_offset = index - safe_index;
let index = safe_index;

let nl = input[0..index]
.iter()
.rev()
.enumerate()
.find(|(_, b)| **b == b'\n')
.map(|(nl, _)| index - nl - 1);
let line_start = match nl {
Some(nl) => nl + 1,
None => 0,
};
let line = input[0..line_start].iter().filter(|b| **b == b'\n').count();
let line = line;

let column = std::str::from_utf8(&input[line_start..=index])
.map(|s| s.chars().count() - 1)
.unwrap_or_else(|_| index - line_start);
let column = column + column_offset;

(line, column)
}
90 changes: 39 additions & 51 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,15 +449,13 @@ fn malformed_override() {
.with_status(101)
.with_stderr(
"\
[ERROR] failed to parse manifest at `[..]`
Caused by:
TOML parse error at line 8, column 27
|
8 | native = {
| ^
invalid inline table
expected `}`
[ERROR] invalid inline table
expected `}`
--> Cargo.toml:8:27
|
8 | native = {
| ^
|
",
)
.run();
Expand Down Expand Up @@ -1282,14 +1280,12 @@ fn bad_dependency() {
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[..]`
Caused by:
TOML parse error at line 8, column 23
|
8 | bar = 3
| ^
invalid type: integer `3`, expected a version string like [..]
[ERROR] invalid type: integer `3`, expected a version string like [..]
--> Cargo.toml:8:23
|
8 | bar = 3
| ^
|
",
)
.run();
Expand Down Expand Up @@ -1317,14 +1313,12 @@ fn bad_debuginfo() {
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest [..]
Caused by:
TOML parse error at line 8, column 25
|
8 | debug = 'a'
| ^^^
invalid value: string \"a\", expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
[ERROR] invalid value: string \"a\", expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
--> Cargo.toml:8:25
|
8 | debug = 'a'
| ^^^
|
",
)
.run();
Expand Down Expand Up @@ -1352,14 +1346,12 @@ fn bad_debuginfo2() {
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[..]`
Caused by:
TOML parse error at line 8, column 25
|
8 | debug = 3.6
| ^^^
invalid type: floating point `3.6`, expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
[ERROR] invalid type: floating point `3.6`, expected a boolean, 0, 1, 2, \"line-tables-only\", or \"line-directives-only\"
--> Cargo.toml:8:25
|
8 | debug = 3.6
| ^^^
|
",
)
.run();
Expand All @@ -1385,14 +1377,12 @@ fn bad_opt_level() {
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[..]`
Caused by:
TOML parse error at line 6, column 25
|
6 | build = 3
| ^
invalid type: integer `3`, expected a boolean or string
[ERROR] invalid type: integer `3`, expected a boolean or string
--> Cargo.toml:6:25
|
6 | build = 3
| ^
|
",
)
.run();
Expand Down Expand Up @@ -1685,16 +1675,14 @@ fn bad_trim_paths() {
p.cargo("check -Ztrim-paths")
.masquerade_as_nightly_cargo(&["trim-paths"])
.with_status(101)
.with_stderr(
r#"error: failed to parse manifest at `[..]`
Caused by:
TOML parse error at line 7, column 30
|
7 | trim-paths = "split-debuginfo"
| ^^^^^^^^^^^^^^^^^
expected a boolean, "none", "diagnostics", "macro", "object", "all", or an array with these options
"#,
.with_stderr("\
[ERROR] expected a boolean, \"none\", \"diagnostics\", \"macro\", \"object\", \"all\", or an array with these options
--> Cargo.toml:7:30
|
7 | trim-paths = \"split-debuginfo\"
| ^^^^^^^^^^^^^^^^^
|
",
)
.run();
}
Loading

0 comments on commit 31b833d

Please sign in to comment.