feat: Add rustc style errors for manifest parsing

Author: Muscraft

Cargo.lock

@@ -18,6 +18,7 @@ homepage = "https://github.com/rust-lang/cargo"
 repository = "https://github.com/rust-lang/cargo"
 
 [workspace.dependencies]
+annotate-snippets = "0.10.1"
 anstream = "0.6.5"
 anstyle = "1.0.4"
 anyhow = "1.0.79"
@@ -139,6 +140,7 @@ name = "cargo"
 path = "src/cargo/lib.rs"
 
 [dependencies]
+annotate-snippets.workspace = true
 anstream.workspace = true
 anstyle.workspace = true
 anyhow.workspace = true

Cargo.toml

@@ -1,16 +1,19 @@
+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 cargo_util_schemas::manifest;
 use cargo_util_schemas::manifest::RustVersion;
 use itertools::Itertools;
 use lazycell::LazyCell;
+use pathdiff::diff_paths;
 use tracing::{debug, trace};
 use url::Url;
 
@@ -43,7 +46,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(),
@@ -56,15 +59,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`
@@ -94,11 +105,61 @@ 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) => {
+            let Some(span) = e.span() else {
+                return Err(e.into());
+            };
+
+            let (line_num, column) = translate_position(&contents, span.start);
+            let source_start = contents[0..span.start]
+                .rfind('\n')
+                .map(|s| s + 1)
+                .unwrap_or(0);
+            let source_end = contents[span.end - 1..]
+                .find('\n')
+                .map(|s| s + span.end)
+                .unwrap_or(contents.len());
+            let source = &contents[source_start..source_end];
+            // 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 + contents[span].chars().count())
+                .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_else(|| 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_num + 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();
+            writeln!(config.shell().err(), "{}", renderer.render(snippet))?;
+            return Err(AlreadyPrintedError::new(e.into()).into());
+        }
+    };
     let add_unused = |warnings: &mut Warnings| {
         for key in unused {
             warnings.add_warning(format!("unused manifest key: {}", key));
@@ -2113,3 +2174,33 @@ impl ResolveToPath for ConfigRelativePath {
         self.resolve_path(c)
     }
 }
+
+fn translate_position(input: &str, 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 nl = input[0..safe_index]
+        .as_bytes()
+        .iter()
+        .rev()
+        .enumerate()
+        .find(|(_, b)| **b == b'\n')
+        .map(|(nl, _)| safe_index - nl - 1);
+    let line_start = match nl {
+        Some(nl) => nl + 1,
+        None => 0,
+    };
+    let line = input[0..line_start]
+        .as_bytes()
+        .iter()
+        .filter(|c| **c == b'\n')
+        .count();
+    let column = input[line_start..=safe_index].chars().count() - 1;
+    let column = column + column_offset;
+
+    (line, column)
+}

src/cargo/util/toml/mod.rs

@@ -712,16 +712,17 @@ fn bad_registry_name() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
-
-Caused by:
-  TOML parse error at line 7, column 17
-    |
-  7 |                 [dependencies.bar]
-    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-  invalid character ` ` in registry name: `bad name`, [..]
+[ERROR] invalid character ` ` in registry name: `bad name`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)
 
 
+ --> Cargo.toml:7:17
+  |
+7 |                   [dependencies.bar]
+  |  _________________^
+8 | |                 version = \"0.0.1\"
+9 | |                 registry = \"bad name\"
+  | |_____________________________________^
+  |
 ",
         )
         .run();
@@ -1622,16 +1623,14 @@ fn empty_dependency_registry() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
-
-Caused by:
-  TOML parse error at line 7, column 23
-    |
-  7 |                 bar = { version = \"0.1.0\", registry = \"\" }
-    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-  registry name cannot be empty
+[ERROR] registry name cannot be empty
 
 
+ --> Cargo.toml:7:23
+  |
+7 |                 bar = { version = \"0.1.0\", registry = \"\" }
+  |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |
 ",
         )
         .run();

tests/testsuite/alt_registry.rs

@@ -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();
@@ -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();
@@ -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();
@@ -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();
@@ -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();
@@ -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();
 }