From 06c31de9724b91e2c4ea671c7988c362dcec15bf Mon Sep 17 00:00:00 2001 From: Robert DiMartino Date: Mon, 19 Sep 2022 23:41:20 -0400 Subject: [PATCH 1/3] Improve errors for TOML fields that support workspace inheritance --- Cargo.toml | 1 + src/cargo/util/toml/mod.rs | 19 ++++++++++++++++++- tests/testsuite/metadata.rs | 24 ++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 21fb83dd4..5fcac42c7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ semver = { version = "1.0.3", features = ["serde"] } serde = { version = "1.0.123", features = ["derive"] } serde_ignored = "0.1.0" serde_json = { version = "1.0.30", features = ["raw_value"] } +serde-value = "0.7.0" shell-escape = "0.1.4" strip-ansi-escapes = "0.1.0" tar = { version = "0.4.38", default-features = false } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index ee9bbd383..3253ed2b1 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1007,13 +1007,30 @@ where /// Enum that allows for the parsing of `field.workspace = true` in a Cargo.toml /// /// It allows for things to be inherited from a workspace or defined as needed -#[derive(Deserialize, Serialize, Clone, Debug)] +#[derive(Serialize, Clone, Debug)] #[serde(untagged)] pub enum MaybeWorkspace { Workspace(TomlWorkspaceField), Defined(T), } +impl<'de, T: Deserialize<'de>> de::Deserialize<'de> for MaybeWorkspace { + fn deserialize(deserializer: D) -> Result, D::Error> + where + D: de::Deserializer<'de>, + { + let value = serde_value::Value::deserialize(deserializer)?; + if let Ok(workspace) = TomlWorkspaceField::deserialize(serde_value::ValueDeserializer::< + D::Error, + >::new(value.clone())) + { + return Ok(MaybeWorkspace::Workspace(workspace)); + } + T::deserialize(serde_value::ValueDeserializer::::new(value)) + .map(MaybeWorkspace::Defined) + } +} + impl MaybeWorkspace { fn resolve<'a>( self, diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index b0365f4e7..1d01127d9 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -1730,6 +1730,30 @@ Caused by: .run(); } +#[cargo_test] +fn cargo_metadata_with_invalid_version_field() { + let p = project() + .file("src/foo.rs", "") + .file( + "Cargo.toml", + r#" + [package] + version = 1 + "#, + ) + .build(); + + p.cargo("metadata") + .with_status(101) + .with_stderr( + r#"[ERROR] failed to parse manifest at `[..]` + +Caused by: + invalid type: integer `1`, expected SemVer version for key `package.version`"#, + ) + .run(); +} + const MANIFEST_OUTPUT: &str = r#" { "packages": [{ From 7a50c0c718f3547e3d76498770fc3ed256cb3994 Mon Sep 17 00:00:00 2001 From: Robert DiMartino Date: Mon, 19 Sep 2022 23:45:51 -0400 Subject: [PATCH 2/3] Remove maybe_workspace_vec_string specific deserializer --- src/cargo/util/toml/mod.rs | 47 ------------------------------------- tests/testsuite/metadata.rs | 2 +- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 3253ed2b1..fe49fb34d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1058,43 +1058,6 @@ impl MaybeWorkspace { } } -fn maybe_workspace_vec_string<'de, D>( - deserializer: D, -) -> Result>>, D::Error> -where - D: de::Deserializer<'de>, -{ - struct Visitor; - - impl<'de> de::Visitor<'de> for Visitor { - type Value = Option>>; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("vector of strings") - } - - fn visit_seq(self, v: V) -> Result - where - V: de::SeqAccess<'de>, - { - let seq = de::value::SeqAccessDeserializer::new(v); - let defined = Vec::::deserialize(seq).map(MaybeWorkspace::Defined)?; - Ok(Some(defined)) - } - - fn visit_map(self, map: V) -> Result - where - V: de::MapAccess<'de>, - { - let mvd = de::value::MapAccessDeserializer::new(map); - let workspace = TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace)?; - Ok(Some(workspace)) - } - } - - deserializer.deserialize_any(Visitor) -} - #[derive(Deserialize, Serialize, Clone, Debug)] pub struct TomlWorkspaceField { workspace: bool, @@ -1114,8 +1077,6 @@ pub struct TomlProject { name: InternedString, #[serde(deserialize_with = "version_trim_whitespace")] version: MaybeWorkspace, - #[serde(default)] - #[serde(deserialize_with = "maybe_workspace_vec_string")] authors: Option>>, build: Option, metabuild: Option, @@ -1124,11 +1085,7 @@ pub struct TomlProject { #[serde(rename = "forced-target")] forced_target: Option, links: Option, - #[serde(default)] - #[serde(deserialize_with = "maybe_workspace_vec_string")] exclude: Option>>, - #[serde(default)] - #[serde(deserialize_with = "maybe_workspace_vec_string")] include: Option>>, publish: Option>, workspace: Option, @@ -1144,11 +1101,7 @@ pub struct TomlProject { homepage: Option>, documentation: Option>, readme: Option>, - #[serde(default)] - #[serde(deserialize_with = "maybe_workspace_vec_string")] keywords: Option>>, - #[serde(default)] - #[serde(deserialize_with = "maybe_workspace_vec_string")] categories: Option>>, license: Option>, license_file: Option>, diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 1d01127d9..922544a6e 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -1725,7 +1725,7 @@ fn cargo_metadata_with_invalid_authors_field() { r#"[ERROR] failed to parse manifest at `[..]` Caused by: - invalid type: string "", expected vector of strings for key `package.authors`"#, + invalid type: string "", expected a sequence for key `package.authors`"#, ) .run(); } From 7b16e59896fc44ce23d24ff8dde2e83382e1be5b Mon Sep 17 00:00:00 2001 From: Robert DiMartino Date: Tue, 20 Sep 2022 09:34:58 -0400 Subject: [PATCH 3/3] Add metadata test for invalid publish field --- tests/testsuite/metadata.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 922544a6e..fba40d118 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -1754,6 +1754,30 @@ Caused by: .run(); } +#[cargo_test] +fn cargo_metadata_with_invalid_publish_field() { + let p = project() + .file("src/foo.rs", "") + .file( + "Cargo.toml", + r#" + [package] + publish = "foo" + "#, + ) + .build(); + + p.cargo("metadata") + .with_status(101) + .with_stderr( + r#"[ERROR] failed to parse manifest at `[..]` + +Caused by: + invalid type: string "foo", expected a boolean or vector of strings for key `package.publish`"#, + ) + .run(); +} + const MANIFEST_OUTPUT: &str = r#" { "packages": [{