Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(toml): Improve parse errors #12556

Merged
merged 4 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::io::prelude::*;

use crate::core::{resolver, Resolve, ResolveVersion, Workspace};
use crate::util::errors::CargoResult;
use crate::util::toml as cargo_toml;
use crate::util::Filesystem;

use anyhow::Context as _;
Expand All @@ -20,8 +19,7 @@ pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {
.with_context(|| format!("failed to read file: {}", f.path().display()))?;

let resolve = (|| -> CargoResult<Option<Resolve>> {
let resolve: toml::Table = cargo_toml::parse_document(&s, f.path(), ws.config())?;
let v: resolver::EncodableResolve = resolve.try_into()?;
let v: resolver::EncodableResolve = toml::from_str(&s)?;
Ok(Some(v.into_resolve(&s, ws)?))
})()
.with_context(|| format!("failed to parse lock file at: {}", f.path().display()))?;
Expand Down
46 changes: 24 additions & 22 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use cargo_util::paths;
use itertools::Itertools;
use lazycell::LazyCell;
use semver::{self, VersionReq};
use serde::de::IntoDeserializer as _;
use serde::de::{self, Unexpected};
use serde::ser;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -99,26 +98,9 @@ fn read_manifest_from_str(
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
let package_root = manifest_file.parent().unwrap();

let toml = {
let pretty_filename = manifest_file
.strip_prefix(config.cwd())
.unwrap_or(manifest_file);
parse_document(contents, pretty_filename, config)?
};

// Provide a helpful error message for a common user error.
if let Some(package) = toml.get("package").or_else(|| toml.get("project")) {
if let Some(feats) = package.get("cargo-features") {
bail!(
"cargo-features = {} was found in the wrong location: it \
should be set at the top of Cargo.toml before any tables",
feats
);
}
}

let mut unused = BTreeSet::new();
let manifest: TomlManifest = serde_ignored::deserialize(toml.into_deserializer(), |path| {
let deserializer = toml::de::Deserializer::new(contents);
let manifest: TomlManifest = serde_ignored::deserialize(deserializer, |path| {
let mut key = String::new();
stringify(&mut key, &path);
unused.insert(key);
Expand Down Expand Up @@ -194,8 +176,7 @@ fn read_manifest_from_str(

pub fn parse_document(toml: &str, _file: &Path, _config: &Config) -> CargoResult<toml::Table> {
// At the moment, no compatibility checks are needed.
toml.parse()
.map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))
toml.parse().map_err(Into::into)
}

/// Warn about paths that have been deprecated and may conflict.
Expand Down Expand Up @@ -1537,6 +1518,10 @@ pub struct TomlPackage {
repository: Option<MaybeWorkspaceString>,
resolver: Option<String>,

// Provide a helpful error message for a common user error.
#[serde(rename = "cargo-features", skip_serializing)]
_invalid_cargo_features: Option<InvalidCargoFeatures>,

// Note that this field must come last due to the way toml serialization
// works which requires tables to be emitted after all values.
metadata: Option<toml::Value>,
Expand Down Expand Up @@ -3547,3 +3532,20 @@ impl TomlLintLevel {
}
}
}

#[derive(Copy, Clone, Debug)]
#[non_exhaustive]
struct InvalidCargoFeatures {}

impl<'de> de::Deserialize<'de> for InvalidCargoFeatures {
fn deserialize<D>(_d: D) -> Result<Self, D::Error>
where
D: de::Deserializer<'de>,
{
use serde::de::Error as _;

Err(D::Error::custom(
"the field `cargo-features` should be set at the top of Cargo.toml before any tables",
))
}
}
39 changes: 24 additions & 15 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,6 @@ fn invalid_global_config() {
Caused by:
could not parse TOML configuration in `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 1, column 2
|
Expand All @@ -199,8 +196,11 @@ fn bad_cargo_lock() {
[ERROR] failed to parse lock file at: [..]Cargo.lock

Caused by:
TOML parse error at line 1, column 1
|
1 | [[package]]
| ^^^^^^^^^^^
missing field `name`
in `package`
",
)
.run();
Expand Down Expand Up @@ -303,8 +303,11 @@ fn bad_source_in_cargo_lock() {
[ERROR] failed to parse lock file at: [..]

Caused by:
TOML parse error at line 12, column 26
|
12 | source = \"You shall not parse\"
| ^^^^^^^^^^^^^^^^^^^^^
invalid source `You shall not parse`
in `package.source`
",
)
.run();
Expand Down Expand Up @@ -448,9 +451,6 @@ fn malformed_override() {
"\
[ERROR] failed to parse manifest at `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 8, column 27
|
Expand Down Expand Up @@ -803,9 +803,6 @@ error: could not load Cargo configuration
Caused by:
could not parse TOML configuration in `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 1, column 7
|
Expand Down Expand Up @@ -1288,8 +1285,11 @@ fn bad_dependency() {
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 [..]
in `dependencies.bar`
",
)
.run();
Expand Down Expand Up @@ -1320,8 +1320,11 @@ fn bad_debuginfo() {
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\"
in `profile.dev.debug`
",
)
.run();
Expand Down Expand Up @@ -1352,8 +1355,11 @@ fn bad_debuginfo2() {
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\"
in `profile.dev.debug`
",
)
.run();
Expand Down Expand Up @@ -1382,8 +1388,11 @@ fn bad_opt_level() {
error: failed to parse manifest at `[..]`

Caused by:
TOML parse error at line 6, column 25
|
6 | build = 3
| ^
expected a boolean or a string
in `package.build`
",
)
.run();
Expand Down
14 changes: 4 additions & 10 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,6 @@ fn cargo_compile_with_invalid_manifest2() {
"\
[ERROR] failed to parse manifest at `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 3, column 23
|
Expand All @@ -283,9 +280,6 @@ fn cargo_compile_with_invalid_manifest3() {
"\
[ERROR] failed to parse manifest at `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 1, column 5
|
Expand Down Expand Up @@ -346,8 +340,11 @@ fn cargo_compile_with_invalid_version() {
[ERROR] failed to parse manifest at `[..]`

Caused by:
TOML parse error at line 4, column 19
|
4 | version = \"1.0\"
| ^^^^^
unexpected end of input while parsing minor version number
in `package.version`
",
)
.run();
Expand Down Expand Up @@ -3035,9 +3032,6 @@ fn bad_cargo_config() {
Caused by:
could not parse TOML configuration in `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 1, column 6
|
Expand Down
3 changes: 0 additions & 3 deletions tests/testsuite/cargo_add/invalid_manifest/stderr.log
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
error: failed to parse manifest at `[ROOT]/case/Cargo.toml`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 8, column 7
|
Expand Down
3 changes: 0 additions & 3 deletions tests/testsuite/cargo_alias_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ fn alias_malformed_config_string() {
Caused by:
could not parse TOML configuration in `[..]/config`

Caused by:
[..]

Caused by:
TOML parse error at line [..]
|
Expand Down
7 changes: 5 additions & 2 deletions tests/testsuite/cargo_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,8 +676,11 @@ fn wrong_position() {
error: failed to parse manifest at [..]

Caused by:
cargo-features = [\"test-dummy-unstable\"] was found in the wrong location: it \
should be set at the top of Cargo.toml before any tables
TOML parse error at line 5, column 34
|
5 | cargo-features = [\"test-dummy-unstable\"]
| ^^^^^^^^^^^^^^^^^^^^^^^
the field `cargo-features` should be set at the top of Cargo.toml before any tables
",
)
.run();
Expand Down
6 changes: 0 additions & 6 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,9 +721,6 @@ could not load Cargo configuration
Caused by:
could not parse TOML configuration in `[..]/.cargo/config`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 1, column 5
|
Expand Down Expand Up @@ -1090,9 +1087,6 @@ could not load Cargo configuration
Caused by:
could not parse TOML configuration in `[..]/.cargo/config`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 3, column 1
|
Expand Down
3 changes: 0 additions & 3 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2577,9 +2577,6 @@ Caused by:
Caused by:
failed to parse manifest at `[..]`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 8, column 21
|
Expand Down
8 changes: 4 additions & 4 deletions tests/testsuite/inheritable_workspace_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1234,8 +1234,11 @@ fn error_workspace_false() {
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`

Caused by:
TOML parse error at line 7, column 41
|
7 | description = { workspace = false }
| ^^^^^
`workspace` cannot be false
in `package.description.workspace`
",
)
.run();
Expand Down Expand Up @@ -1321,9 +1324,6 @@ fn error_malformed_workspace_root() {
"\
[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml`

Caused by:
[..]

Caused by:
[..]
|
Expand Down
21 changes: 15 additions & 6 deletions tests/testsuite/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1821,8 +1821,11 @@ fn cargo_metadata_with_invalid_authors_field() {
r#"[ERROR] failed to parse manifest at `[..]`

Caused by:
invalid type: string "", expected a vector of strings or workspace
in `package.authors`"#,
TOML parse error at line 3, column 27
|
3 | authors = ""
| ^^
invalid type: string "", expected a vector of strings or workspace"#,
)
.run();
}
Expand All @@ -1846,8 +1849,11 @@ fn cargo_metadata_with_invalid_version_field() {
r#"[ERROR] failed to parse manifest at `[..]`

Caused by:
invalid type: integer `1`, expected SemVer version
in `package.version`"#,
TOML parse error at line 3, column 27
|
3 | version = 1
| ^
invalid type: integer `1`, expected SemVer version"#,
)
.run();
}
Expand All @@ -1871,8 +1877,11 @@ fn cargo_metadata_with_invalid_publish_field() {
r#"[ERROR] failed to parse manifest at `[..]`

Caused by:
invalid type: string "foo", expected a boolean, a vector of strings, or workspace
in `package.publish`"#,
TOML parse error at line 3, column 27
|
3 | publish = "foo"
| ^^^^^
invalid type: string "foo", expected a boolean, a vector of strings, or workspace"#,
)
.run();
}
Expand Down
3 changes: 0 additions & 3 deletions tests/testsuite/workspaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1090,9 +1090,6 @@ fn new_warning_with_corrupt_ws() {

failed to parse manifest at `[..]foo/Cargo.toml`

Caused by:
could not parse input as TOML

Caused by:
TOML parse error at line 1, column 5
|
Expand Down