Auto merge of #11113 - rdimartino:maybeworkspace-deserialize, r=epage

Improve errors for TOML fields that support workspace inheritance

Fixes #10997

This also addresses the issue with `MaybeWorkspace<VecStringOrBool>` mentioned in #10942:

```
Caused by:
  invalid type: string "foo", expected a boolean or vector of strings for key `package.publish`
```

I removed the `maybe_workspace_vec_string` deserializer in 7a50c0c718 because the error message from the inner `Vec<String>` is now surfaced, but I can revert that if it's preferable to keep those changes.

I tried to base the `deserialize` implementation off of the derived impl for an untagged enum from `cargo expand`. This approach [ultimately led me](https://github.com/serde-rs/serde/blob/v1.0.144/serde/src/private/de.rs#L218) to adding the `serde-value` dependency.

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 }

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<T> {
     Workspace(TomlWorkspaceField),
     Defined(T),
 }
 
+impl<'de, T: Deserialize<'de>> de::Deserialize<'de> for MaybeWorkspace<T> {
+    fn deserialize<D>(deserializer: D) -> Result<MaybeWorkspace<T>, 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::<D::Error>::new(value))
+            .map(MaybeWorkspace::Defined)
+    }
+}
+
 impl<T> MaybeWorkspace<T> {
     fn resolve<'a>(
         self,
@@ -1041,43 +1058,6 @@ impl<T> MaybeWorkspace<T> {
     }
 }
 
-fn maybe_workspace_vec_string<'de, D>(
-    deserializer: D,
-) -> Result<Option<MaybeWorkspace<Vec<String>>>, D::Error>
-where
-    D: de::Deserializer<'de>,
-{
-    struct Visitor;
-
-    impl<'de> de::Visitor<'de> for Visitor {
-        type Value = Option<MaybeWorkspace<Vec<String>>>;
-
-        fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
-            formatter.write_str("vector of strings")
-        }
-
-        fn visit_seq<V>(self, v: V) -> Result<Self::Value, V::Error>
-        where
-            V: de::SeqAccess<'de>,
-        {
-            let seq = de::value::SeqAccessDeserializer::new(v);
-            let defined = Vec::<String>::deserialize(seq).map(MaybeWorkspace::Defined)?;
-            Ok(Some(defined))
-        }
-
-        fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
-        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,
@@ -1097,8 +1077,6 @@ pub struct TomlProject {
     name: InternedString,
     #[serde(deserialize_with = "version_trim_whitespace")]
     version: MaybeWorkspace<semver::Version>,
-    #[serde(default)]
-    #[serde(deserialize_with = "maybe_workspace_vec_string")]
     authors: Option<MaybeWorkspace<Vec<String>>>,
     build: Option<StringOrBool>,
     metabuild: Option<StringOrVec>,
@@ -1107,11 +1085,7 @@ pub struct TomlProject {
     #[serde(rename = "forced-target")]
     forced_target: Option<String>,
     links: Option<String>,
-    #[serde(default)]
-    #[serde(deserialize_with = "maybe_workspace_vec_string")]
     exclude: Option<MaybeWorkspace<Vec<String>>>,
-    #[serde(default)]
-    #[serde(deserialize_with = "maybe_workspace_vec_string")]
     include: Option<MaybeWorkspace<Vec<String>>>,
     publish: Option<MaybeWorkspace<VecStringOrBool>>,
     workspace: Option<String>,
@@ -1127,11 +1101,7 @@ pub struct TomlProject {
     homepage: Option<MaybeWorkspace<String>>,
     documentation: Option<MaybeWorkspace<String>>,
     readme: Option<MaybeWorkspace<StringOrBool>>,
-    #[serde(default)]
-    #[serde(deserialize_with = "maybe_workspace_vec_string")]
     keywords: Option<MaybeWorkspace<Vec<String>>>,
-    #[serde(default)]
-    #[serde(deserialize_with = "maybe_workspace_vec_string")]
     categories: Option<MaybeWorkspace<Vec<String>>>,
     license: Option<MaybeWorkspace<String>>,
     license_file: Option<MaybeWorkspace<String>>,

tests/testsuite/metadata.rs

@@ -1725,7 +1725,55 @@ 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();
+}
+
+#[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();
+}
+
+#[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();
 }