Avoid using owned String in deserializers (#11764)

## Summary This is the pattern I see in a variety of crates, and I believe this is preferred if you don't _need_ an owned `String`, since you can avoid the allocation. This could be pretty impactful for us?
astral-sh · Feb 25, 2025 · c37af94 · c37af94
1 parent 275db06
commit c37af94
Show file tree

Hide file tree

Showing 19 changed files with 350 additions and 104 deletions.
diff --git a/crates/uv-configuration/src/required_version.rs b/crates/uv-configuration/src/required_version.rs
@@ -1,3 +1,4 @@
+use std::fmt::Formatter;
 use std::str::FromStr;
 
 use uv_pep440::{Version, VersionSpecifier, VersionSpecifiers, VersionSpecifiersParseError};
@@ -49,8 +50,21 @@ impl schemars::JsonSchema for RequiredVersion {
 
 impl<'de> serde::Deserialize<'de> for RequiredVersion {
     fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-        let s = String::deserialize(deserializer)?;
-        Self::from_str(&s).map_err(serde::de::Error::custom)
+        struct Visitor;
+
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = RequiredVersion;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                RequiredVersion::from_str(v).map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-distribution-filename/src/wheel.rs b/crates/uv-distribution-filename/src/wheel.rs
@@ -299,8 +299,21 @@ impl<'de> Deserialize<'de> for WheelFilename {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        FromStr::from_str(&s).map_err(de::Error::custom)
+        struct Visitor;
+
+        impl de::Visitor<'_> for Visitor {
+            type Value = WheelFilename;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                WheelFilename::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-distribution-types/src/index_name.rs b/crates/uv-distribution-types/src/index_name.rs
@@ -58,7 +58,8 @@ impl<'de> serde::de::Deserialize<'de> for IndexName {
     where
         D: serde::de::Deserializer<'de>,
     {
-        IndexName::new(String::deserialize(deserializer)?).map_err(serde::de::Error::custom)
+        let s = String::deserialize(deserializer)?;
+        IndexName::new(s).map_err(serde::de::Error::custom)
     }
 }
 

diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs
@@ -171,8 +171,21 @@ impl<'de> serde::de::Deserialize<'de> for IndexUrl {
     where
         D: serde::de::Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        IndexUrl::from_str(&s).map_err(serde::de::Error::custom)
+        struct Visitor;
+
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = IndexUrl;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                IndexUrl::from_str(v).map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-git-types/src/oid.rs b/crates/uv-git-types/src/oid.rs
@@ -1,4 +1,4 @@
-use std::fmt::Display;
+use std::fmt::{Display, Formatter};
 use std::str::{self, FromStr};
 
 use thiserror::Error;
@@ -74,8 +74,21 @@ impl<'de> serde::Deserialize<'de> for GitOid {
     where
         D: serde::Deserializer<'de>,
     {
-        let value = String::deserialize(deserializer)?;
-        GitOid::from_str(&value).map_err(serde::de::Error::custom)
+        struct Visitor;
+
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = GitOid;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                GitOid::from_str(v).map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-normalize/src/extra_name.rs b/crates/uv-normalize/src/extra_name.rs
@@ -48,8 +48,25 @@ impl<'de> Deserialize<'de> for ExtraName {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        Self::from_str(&s).map_err(serde::de::Error::custom)
+        struct Visitor;
+
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = ExtraName;
+
+            fn expecting(&self, f: &mut Formatter) -> fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                ExtraName::from_str(v).map_err(serde::de::Error::custom)
+            }
+
+            fn visit_string<E: serde::de::Error>(self, v: String) -> Result<Self::Value, E> {
+                ExtraName::from_owned(v).map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-normalize/src/group_name.rs b/crates/uv-normalize/src/group_name.rs
@@ -1,5 +1,3 @@
-use std::fmt;
-use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 use std::sync::LazyLock;
 
@@ -41,8 +39,25 @@ impl<'de> Deserialize<'de> for GroupName {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        Self::from_str(&s).map_err(serde::de::Error::custom)
+        struct Visitor;
+
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = GroupName;
+
+            fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                GroupName::from_str(v).map_err(serde::de::Error::custom)
+            }
+
+            fn visit_string<E: serde::de::Error>(self, v: String) -> Result<Self::Value, E> {
+                GroupName::from_owned(v).map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 
@@ -55,8 +70,8 @@ impl Serialize for GroupName {
     }
 }
 
-impl Display for GroupName {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+impl std::fmt::Display for GroupName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         self.0.fmt(f)
     }
 }

diff --git a/crates/uv-normalize/src/package_name.rs b/crates/uv-normalize/src/package_name.rs
@@ -91,8 +91,25 @@ impl<'de> Deserialize<'de> for PackageName {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        Self::from_str(&s).map_err(serde::de::Error::custom)
+        struct Visitor;
+
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = PackageName;
+
+            fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                PackageName::from_str(v).map_err(serde::de::Error::custom)
+            }
+
+            fn visit_string<E: serde::de::Error>(self, v: String) -> Result<Self::Value, E> {
+                PackageName::from_owned(v).map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-pep440/src/version.rs b/crates/uv-pep440/src/version.rs
@@ -1,4 +1,5 @@
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+use std::fmt::Formatter;
 use std::num::NonZero;
 use std::ops::Deref;
 use std::sync::LazyLock;
@@ -725,14 +726,26 @@ impl Version {
     }
 }
 
-/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
 impl<'de> Deserialize<'de> for Version {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        FromStr::from_str(&s).map_err(de::Error::custom)
+        struct Visitor;
+
+        impl de::Visitor<'_> for Visitor {
+            type Value = Version;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                Version::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-pep440/src/version_specifier.rs b/crates/uv-pep440/src/version_specifier.rs
@@ -1,4 +1,5 @@
 use std::cmp::Ordering;
+use std::fmt::Formatter;
 use std::ops::Bound;
 use std::str::FromStr;
 
@@ -161,8 +162,21 @@ impl<'de> Deserialize<'de> for VersionSpecifiers {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        Self::from_str(&s).map_err(de::Error::custom)
+        struct Visitor;
+
+        impl de::Visitor<'_> for Visitor {
+            type Value = VersionSpecifiers;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                VersionSpecifiers::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 
@@ -172,7 +186,7 @@ impl Serialize for VersionSpecifiers {
     where
         S: Serializer,
     {
-        serializer.collect_str(
+        serializer.serialize_str(
             &self
                 .iter()
                 .map(ToString::to_string)
@@ -256,14 +270,26 @@ pub struct VersionSpecifier {
     pub(crate) version: Version,
 }
 
-/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
 impl<'de> Deserialize<'de> for VersionSpecifier {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        FromStr::from_str(&s).map_err(de::Error::custom)
+        struct Visitor;
+
+        impl de::Visitor<'_> for Visitor {
+            type Value = VersionSpecifier;
+
+            fn expecting(&self, f: &mut Formatter) -> std::fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                VersionSpecifier::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 

diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs
@@ -186,8 +186,24 @@ impl<'de, T: Pep508Url> Deserialize<'de> for Requirement<T> {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        FromStr::from_str(&s).map_err(de::Error::custom)
+        struct RequirementVisitor<T>(std::marker::PhantomData<T>);
+
+        impl<T: Pep508Url> serde::de::Visitor<'_> for RequirementVisitor<T> {
+            type Value = Requirement<T>;
+
+            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+                formatter.write_str("a string containing a PEP 508 requirement")
+            }
+
+            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                FromStr::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(RequirementVisitor(std::marker::PhantomData))
     }
 }
 

diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs
@@ -407,8 +407,21 @@ impl<'de> Deserialize<'de> for StringVersion {
     where
         D: Deserializer<'de>,
     {
-        let string = String::deserialize(deserializer)?;
-        Self::from_str(&string).map_err(de::Error::custom)
+        struct Visitor;
+
+        impl de::Visitor<'_> for Visitor {
+            type Value = StringVersion;
+
+            fn expecting(&self, f: &mut Formatter) -> fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                StringVersion::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }
 
@@ -651,8 +664,21 @@ impl<'de> Deserialize<'de> for MarkerTree {
     where
         D: Deserializer<'de>,
     {
-        let s = String::deserialize(deserializer)?;
-        FromStr::from_str(&s).map_err(de::Error::custom)
+        struct Visitor;
+
+        impl de::Visitor<'_> for Visitor {
+            type Value = MarkerTree;
+
+            fn expecting(&self, f: &mut Formatter) -> fmt::Result {
+                f.write_str("a string")
+            }
+
+            fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
+                MarkerTree::from_str(v).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(Visitor)
     }
 }