refactor: split two code paths in ValueDeserializer to enum

The MapAccess and Deserializer impls in ValueDeserializer
are actually tow distinct code paths. It is better to extract them to
two different value sources.

Author: weihanglo

src/cargo/util/context/de.rs

@@ -441,18 +441,34 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess {
     {
         match self.list_iter.next() {
             // TODO: add `def` to error?
-            Some(CV::String(value, def)) => {
+            Some(val @ CV::String(..)) => {
                 // This might be a String or a Value<String>.
-                // ValueDeserializer will handle figuring out which one it is.
-                let maybe_value_de = ValueDeserializer::new_with_string(value, def);
-                seed.deserialize(maybe_value_de).map(Some)
+                // ArrayItemDeserializer will handle figuring out which one it is.
+                seed.deserialize(ArrayItemDeserializer::new(val)).map(Some)
             }
             Some(val) => Err(ConfigError::expected(&self.key, "list of string", &val)),
             None => Ok(None),
         }
     }
 }
 
+/// Source of data for [`ValueDeserializer`]
+enum ValueSource<'gctx> {
+    /// The deserializer used to actually deserialize a Value struct.
+    Deserializer {
+        de: Deserializer<'gctx>,
+        definition: Definition,
+    },
+    /// A [`ConfigValue`](CV).
+    ///
+    /// This is used for situations where you can't address a string via a TOML key,
+    /// such as a string inside an array.
+    /// The [`ConfigSeqAccess`] doesn't know if the type it should deserialize to
+    /// is a `String` or `Value<String>`,
+    /// so [`ArrayItemDeserializer`] needs to be able to handle both.
+    ConfigValue(CV),
+}
+
 /// This is a deserializer that deserializes into a `Value<T>` for
 /// configuration.
 ///
@@ -462,18 +478,7 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess {
 /// See more comments in `value.rs` for the protocol used here.
 struct ValueDeserializer<'gctx> {
     hits: u32,
-    definition: Definition,
-    /// The deserializer, used to actually deserialize a Value struct.
-    /// This is `None` if deserializing a string.
-    de: Option<Deserializer<'gctx>>,
-    /// A string value to deserialize.
-    ///
-    /// This is used for situations where you can't address a string via a
-    /// TOML key, such as a string inside an array. The `ConfigSeqAccess`
-    /// doesn't know if the type it should deserialize to is a `String` or
-    /// `Value<String>`, so `ValueDeserializer` needs to be able to handle
-    /// both.
-    str_value: Option<String>,
+    source: ValueSource<'gctx>,
 }
 
 impl<'gctx> ValueDeserializer<'gctx> {
@@ -498,20 +503,24 @@ impl<'gctx> ValueDeserializer<'gctx> {
                 (_, None) => env_def,
             }
         };
+
         Ok(ValueDeserializer {
             hits: 0,
-            definition,
-            de: Some(de),
-            str_value: None,
+            source: ValueSource::Deserializer { de, definition },
         })
     }
 
-    fn new_with_string(s: String, definition: Definition) -> ValueDeserializer<'gctx> {
+    fn with_cv(cv: CV) -> ValueDeserializer<'gctx> {
         ValueDeserializer {
             hits: 0,
-            definition,
-            de: None,
-            str_value: Some(s),
+            source: ValueSource::ConfigValue(cv),
+        }
+    }
+
+    fn definition(&self) -> &Definition {
+        match &self.source {
+            ValueSource::Deserializer { definition, .. } => definition,
+            ValueSource::ConfigValue(cv) => cv.definition(),
         }
     }
 }
@@ -542,19 +551,19 @@ impl<'de, 'gctx> de::MapAccess<'de> for ValueDeserializer<'gctx> {
         // If this is the first time around we deserialize the `value` field
         // which is the actual deserializer
         if self.hits == 1 {
-            if let Some(de) = &self.de {
-                return seed
+            return match &self.source {
+                ValueSource::Deserializer { de, definition } => seed
                     .deserialize(de.clone())
-                    .map_err(|e| e.with_key_context(&de.key, Some(self.definition.clone())));
-            } else {
-                return seed
-                    .deserialize(self.str_value.as_ref().unwrap().clone().into_deserializer());
-            }
+                    .map_err(|e| e.with_key_context(&de.key, Some(definition.clone()))),
+                ValueSource::ConfigValue(cv) => {
+                    seed.deserialize(ArrayItemDeserializer::new(cv.clone()))
+                }
+            };
         }
 
         // ... otherwise we're deserializing the `definition` field, so we need
         // to figure out where the field we just deserialized was defined at.
-        match &self.definition {
+        match self.definition() {
             Definition::Path(path) => {
                 seed.deserialize(Tuple2Deserializer(0i32, path.to_string_lossy()))
             }
@@ -572,25 +581,45 @@ impl<'de, 'gctx> de::MapAccess<'de> for ValueDeserializer<'gctx> {
     }
 }
 
-// Deserializer is only implemented to handle deserializing a String inside a
-// sequence (like `Vec<String>` or `Vec<Value<String>>`). `Value<String>` is
-// handled by deserialize_struct, and the plain `String` is handled by all the
-// other functions here.
-impl<'de, 'gctx> de::Deserializer<'de> for ValueDeserializer<'gctx> {
+/// A deserializer for individual [`ConfigValue`](CV) items in arrays
+///
+/// This deserializer is only implemented to handle deserializing a String
+/// inside a sequence (like `Vec<String>` or `Vec<Value<String>>`).
+/// `Value<String>` is handled by `deserialize_struct` in [`ValueDeserializer`],
+/// and the plain `String` is handled by all the other functions here.
+#[derive(Clone)]
+struct ArrayItemDeserializer {
+    cv: CV,
+}
+
+impl ArrayItemDeserializer {
+    fn new(cv: CV) -> Self {
+        Self { cv }
+    }
+
+    fn into_inner(self) -> String {
+        match self.cv {
+            CV::String(s, _def) => s,
+            _ => unreachable!("string expected"),
+        }
+    }
+}
+
+impl<'de> de::Deserializer<'de> for ArrayItemDeserializer {
     type Error = ConfigError;
 
     fn deserialize_str<V>(self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_str(&self.str_value.expect("string expected"))
+        visitor.visit_str(&self.into_inner())
     }
 
     fn deserialize_string<V>(self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_string(self.str_value.expect("string expected"))
+        visitor.visit_string(self.into_inner())
     }
 
     fn deserialize_struct<V>(
@@ -607,7 +636,7 @@ impl<'de, 'gctx> de::Deserializer<'de> for ValueDeserializer<'gctx> {
         //
         // See more comments in `value.rs` for the protocol used here.
         if name == value::NAME && fields == value::FIELDS {
-            return visitor.visit_map(self);
+            return visitor.visit_map(ValueDeserializer::with_cv(self.cv));
         }
         unimplemented!("only strings and Value can be deserialized from a sequence");
     }
@@ -616,7 +645,7 @@ impl<'de, 'gctx> de::Deserializer<'de> for ValueDeserializer<'gctx> {
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_string(self.str_value.expect("string expected"))
+        visitor.visit_string(self.into_inner())
     }
 
     fn deserialize_ignored_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>