mirror of
https://github.com/rust-lang/cargo
synced 2024-08-28 03:39:31 +00:00
Auto merge of #12556 - epage:serde, r=arlosi
fix(toml): Improve parse errors ### What does this PR try to resolve? When we adopted `toml_edit`, we got TOML syntax errors that showed the context for where the error occurred. However, the work was not done to extend this to semantic errors reported by serde. This updates `Cargo.toml` and `Cargo.lock` code to provide that context on semantic errors. `config.toml` is not done because the schema is decentralized. In theory, this will also improve performance because we aren't having to allocate a lot of intermediate data to then throw away for every `Cargo.toml` we read. ### How should we test and review this PR? Check by commit to see this change gradually. - The `package.cargo-features` change was made to drop out dependence on `toml::Table` so we could do the direct deserialization
This commit is contained in:
commit
b2c162c067
|
@ -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 _;
|
||||
|
@ -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()))?;
|
||||
|
|
|
@ -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};
|
||||
|
@ -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);
|
||||
|
@ -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.
|
||||
|
@ -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>,
|
||||
|
@ -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",
|
||||
))
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
@ -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();
|
||||
|
@ -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();
|
||||
|
@ -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
|
||||
|
|
||||
|
@ -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
|
||||
|
|
||||
|
@ -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();
|
||||
|
@ -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();
|
||||
|
@ -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();
|
||||
|
@ -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();
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
@ -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
|
||||
|
|
||||
|
@ -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();
|
||||
|
@ -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
|
||||
|
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
|
|
@ -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 [..]
|
||||
|
|
||||
|
|
|
@ -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();
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
@ -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
|
||||
|
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
|
|
@ -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();
|
||||
|
@ -1321,9 +1324,6 @@ fn error_malformed_workspace_root() {
|
|||
"\
|
||||
[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml`
|
||||
|
||||
Caused by:
|
||||
[..]
|
||||
|
||||
Caused by:
|
||||
[..]
|
||||
|
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
|
Loading…
Reference in a new issue