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?
This commit is contained in:
Charlie Marsh 2025-02-25 06:28:16 -08:00 committed by GitHub
parent 275db0668d
commit c37af945b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 350 additions and 104 deletions

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -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))
}
}

View file

@ -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)
}
}

View file

@ -157,8 +157,21 @@ impl<'de> Deserialize<'de> for LenientVersionSpecifiers {
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 = LenientVersionSpecifiers;
fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
f.write_str("a string")
}
fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
LenientVersionSpecifiers::from_str(v).map_err(de::Error::custom)
}
}
deserializer.deserialize_str(Visitor)
}
}

View file

@ -65,13 +65,37 @@ fn deserialize_version_specifiers_lenient<'de, D>(
where
D: Deserializer<'de>,
{
let maybe_string: Option<String> = Option::deserialize(deserializer)?;
let Some(string) = maybe_string else {
return Ok(None);
};
Ok(Some(
LenientVersionSpecifiers::from_str(&string).map(VersionSpecifiers::from),
))
struct Visitor;
impl<'de> serde::de::Visitor<'de> for Visitor {
type Value = Option<Result<VersionSpecifiers, VersionSpecifiersParseError>>;
fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
f.write_str("a string representing a version specifier")
}
fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
Ok(Some(
LenientVersionSpecifiers::from_str(v).map(VersionSpecifiers::from),
))
}
fn visit_some<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
where
D: Deserializer<'de>,
{
deserializer.deserialize_str(Visitor)
}
fn visit_none<E>(self) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(None)
}
}
deserializer.deserialize_option(Visitor)
}
#[derive(Debug, Clone)]

View file

@ -63,8 +63,21 @@ impl schemars::JsonSchema for PythonVersion {
impl<'de> serde::Deserialize<'de> for PythonVersion {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let s = String::deserialize(deserializer)?;
PythonVersion::from_str(&s).map_err(serde::de::Error::custom)
struct Visitor;
impl serde::de::Visitor<'_> for Visitor {
type Value = PythonVersion;
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> {
PythonVersion::from_str(v).map_err(serde::de::Error::custom)
}
}
deserializer.deserialize_str(Visitor)
}
}

View file

@ -2,7 +2,7 @@ use std::borrow::Cow;
use std::collections::{BTreeMap, BTreeSet, VecDeque};
use std::convert::Infallible;
use std::error::Error;
use std::fmt::{Debug, Display};
use std::fmt::{Debug, Display, Formatter};
use std::io;
use std::path::{Path, PathBuf};
use std::str::FromStr;
@ -4361,12 +4361,25 @@ impl Display for Hash {
}
impl<'de> serde::Deserialize<'de> for Hash {
fn deserialize<D>(d: D) -> Result<Hash, D::Error>
fn deserialize<D>(deserializer: D) -> Result<Hash, D::Error>
where
D: serde::de::Deserializer<'de>,
{
let string = String::deserialize(d)?;
string.parse().map_err(serde::de::Error::custom)
struct Visitor;
impl serde::de::Visitor<'_> for Visitor {
type Value = Hash;
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> {
Hash::from_str(v).map_err(serde::de::Error::custom)
}
}
deserializer.deserialize_str(Visitor)
}
}

View file

@ -85,26 +85,21 @@ impl serde::Serialize for SmallString {
impl<'de> serde::Deserialize<'de> for SmallString {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let s = deserializer.deserialize_str(SmallStringVisitor)?;
Ok(s)
}
}
struct Visitor;
struct SmallStringVisitor;
impl serde::de::Visitor<'_> for Visitor {
type Value = SmallString;
impl serde::de::Visitor<'_> for SmallStringVisitor {
type Value = SmallString;
fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
f.write_str("a string")
}
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> {
Ok(v.into())
}
}
fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
Ok(v.into())
}
fn visit_string<E: serde::de::Error>(self, v: String) -> Result<Self::Value, E> {
Ok(v.into())
deserializer.deserialize_str(Visitor)
}
}

View file

@ -7,6 +7,7 @@
//! Then lowers them into a dependency specification.
use std::collections::BTreeMap;
use std::fmt::Formatter;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::str::FromStr;
@ -713,8 +714,39 @@ pub struct ToolUvWorkspace {
}
/// (De)serialize globs as strings.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
pub struct SerdePattern(#[serde(with = "serde_from_and_to_string")] pub Pattern);
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SerdePattern(Pattern);
impl serde::ser::Serialize for SerdePattern {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::ser::Serializer,
{
self.0.as_str().serialize(serializer)
}
}
impl<'de> serde::Deserialize<'de> for SerdePattern {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
struct Visitor;
impl serde::de::Visitor<'_> for Visitor {
type Value = SerdePattern;
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> {
Pattern::from_str(v)
.map(SerdePattern)
.map_err(serde::de::Error::custom)
}
}
deserializer.deserialize_str(Visitor)
}
}
#[cfg(feature = "schemars")]
impl schemars::JsonSchema for SerdePattern {
@ -1585,30 +1617,3 @@ pub enum DependencyType {
/// A dependency in `dependency-groups.{0}`.
Group(GroupName),
}
/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
mod serde_from_and_to_string {
use std::fmt::Display;
use std::str::FromStr;
use serde::{de, Deserialize, Deserializer, Serializer};
pub(super) fn serialize<T, S>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
where
T: Display,
S: Serializer,
{
serializer.collect_str(value)
}
pub(super) fn deserialize<'de, T, D>(deserializer: D) -> Result<T, D::Error>
where
T: FromStr,
T::Err: Display,
D: Deserializer<'de>,
{
String::deserialize(deserializer)?
.parse()
.map_err(de::Error::custom)
}
}

View file

@ -4131,23 +4131,22 @@ fn override_dependency_from_workspace_invalid_syntax() -> Result<()> {
----- stderr -----
warning: Failed to parse `pyproject.toml` during settings discovery:
TOML parse error at line 9, column 29
|
9 | override-dependencies = [
| ^
TOML parse error at line 10, column 7
|
10 | "werkzeug=2.3.0"
| ^^^^^^^^^^^^^^^^
no such comparison operator "=", must be one of ~= == != <= >= < > ===
werkzeug=2.3.0
^^^^^^
error: Failed to parse: `pyproject.toml`
Caused by: TOML parse error at line 9, column 29
|
9 | override-dependencies = [
| ^
Caused by: TOML parse error at line 10, column 7
|
10 | "werkzeug=2.3.0"
| ^^^^^^^^^^^^^^^^
no such comparison operator "=", must be one of ~= == != <= >= < > ===
werkzeug=2.3.0
^^^^^^
"###
);