refactor: embed deserialize validation logic in ProgressConfig (#16194)

### What does this PR try to resolve?

We have a custom `serde(deserialize_with = "progress_or_string")` to
support
`term.progress = "never" | "auto" | <progress table>`.

It was claimed we added in #8165 [^1] but actually never
worked,
because `Deserializer::deserialize_option` [^2] never called
`visit_str`.

This PR remove the custom `progress_or_string`
so that the deserialization logic can be baked in the type itself.

### How to test and review this PR?

I've tested 1.50 Cargo and `progress = "never"` failed with the same
reason as the newly added test.

[^1]:
#8165 (comment)
[^2]:
https://github.com/rust-lang/cargo/blob/c369b8c8d85a/src/cargo/util/config/de.rs#L135-L145

Author: weihanglo

src/cargo/util/context/schema.rs

@@ -13,7 +13,6 @@
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::ffi::OsStr;
-use std::fmt;
 
 use serde::Deserialize;
 use serde_untagged::UntaggedEnumVisitor;
@@ -342,16 +341,14 @@ pub enum FeatureUnification {
 /// color = "auto"
 /// progress.when = "auto"
 /// ```
-#[derive(Deserialize, Default)]
+#[derive(Debug, Deserialize, Default)]
 #[serde(rename_all = "kebab-case")]
 pub struct TermConfig {
     pub verbose: Option<bool>,
     pub quiet: Option<bool>,
     pub color: Option<String>,
     pub hyperlinks: Option<bool>,
     pub unicode: Option<bool>,
-    #[serde(default)]
-    #[serde(deserialize_with = "progress_or_string")]
     pub progress: Option<ProgressConfig>,
 }
 
@@ -369,10 +366,8 @@ pub struct TermConfig {
 /// [term]
 /// progress = { when = "always", width = 80 }
 /// ```
-#[derive(Debug, Default, Deserialize)]
-#[serde(rename_all = "kebab-case")]
+#[derive(Debug, Default)]
 pub struct ProgressConfig {
-    #[serde(default)]
     pub when: ProgressWhen,
     pub width: Option<usize>,
     /// Communicate progress status with a terminal
@@ -388,66 +383,39 @@ pub enum ProgressWhen {
     Always,
 }
 
-fn progress_or_string<'de, D>(deserializer: D) -> Result<Option<ProgressConfig>, D::Error>
-where
-    D: serde::de::Deserializer<'de>,
-{
-    struct ProgressVisitor;
-
-    impl<'de> serde::de::Visitor<'de> for ProgressVisitor {
-        type Value = Option<ProgressConfig>;
-
-        fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
-            formatter.write_str("a string (\"auto\" or \"never\") or a table")
-        }
-
-        fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
-        where
-            E: serde::de::Error,
-        {
-            match s {
-                "auto" => Ok(Some(ProgressConfig {
-                    when: ProgressWhen::Auto,
-                    width: None,
-                    term_integration: None,
-                })),
-                "never" => Ok(Some(ProgressConfig {
-                    when: ProgressWhen::Never,
-                    width: None,
-                    term_integration: None,
-                })),
-                "always" => Err(E::custom("\"always\" progress requires a `width` key")),
-                _ => Err(E::unknown_variant(s, &["auto", "never"])),
-            }
-        }
-
-        fn visit_none<E>(self) -> Result<Self::Value, E>
-        where
-            E: serde::de::Error,
-        {
-            Ok(None)
+// We need this custom deserialization for validadting the rule of
+// `when = "always"` requiring a `width` field.
+impl<'de> Deserialize<'de> for ProgressConfig {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        #[derive(Deserialize)]
+        #[serde(rename_all = "kebab-case")]
+        struct ProgressConfigInner {
+            #[serde(default)]
+            when: ProgressWhen,
+            width: Option<usize>,
+            term_integration: Option<bool>,
         }
 
-        fn visit_some<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
-        where
-            D: serde::de::Deserializer<'de>,
+        let pc = ProgressConfigInner::deserialize(deserializer)?;
+        if let ProgressConfigInner {
+            when: ProgressWhen::Always,
+            width: None,
+            ..
+        } = pc
         {
-            let pc = ProgressConfig::deserialize(deserializer)?;
-            if let ProgressConfig {
-                when: ProgressWhen::Always,
-                width: None,
-                ..
-            } = pc
-            {
-                return Err(serde::de::Error::custom(
-                    "\"always\" progress requires a `width` key",
-                ));
-            }
-            Ok(Some(pc))
+            return Err(serde::de::Error::custom(
+                "\"always\" progress requires a `width` key",
+            ));
         }
+        Ok(ProgressConfig {
+            when: pc.when,
+            width: pc.width,
+            term_integration: pc.term_integration,
+        })
     }
-
-    deserializer.deserialize_option(ProgressVisitor)
 }
 
 #[derive(Debug)]

tests/testsuite/progress.rs

@@ -156,3 +156,26 @@ fn never_progress() {
         .with_stderr_does_not_contain("[BUILDING] [..] [..]/4: [..]")
         .run();
 }
+
+#[cargo_test]
+fn plain_string_when_doesnt_work() {
+    let p = project()
+        .file(
+            ".cargo/config.toml",
+            r#"
+            [term]
+            progress = "never"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr_data(str![[r#"
+[ERROR] invalid configuration for key `term.progress`
+expected a table, but found a string for `term.progress` in [ROOT]/foo/.cargo/config.toml
+
+"#]])
+        .run();
+}