From ba8bf8283c9774326d83bb3aa7e73e6d581103a3 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:54:16 +0200 Subject: [PATCH] feat(json-abi): allow `serde_json::from_{value,reader}` (#684) * feat(json-abi): allow `serde_json::from_{value,reader}` * chore: clippy --- crates/json-abi/src/internal_type.rs | 91 +++++++++++++----------- crates/json-abi/src/param.rs | 102 ++++++++++++++++++--------- crates/json-abi/tests/abi.rs | 3 + crates/json-abi/tests/it/test.rs | 9 +-- 4 files changed, 124 insertions(+), 81 deletions(-) diff --git a/crates/json-abi/src/internal_type.rs b/crates/json-abi/src/internal_type.rs index 164467dfad..3afe4092ce 100644 --- a/crates/json-abi/src/internal_type.rs +++ b/crates/json-abi/src/internal_type.rs @@ -39,25 +39,6 @@ pub enum InternalType { }, } -impl From> for InternalType { - #[inline] - fn from(borrowed: BorrowedInternalType<'_>) -> Self { - match borrowed { - BorrowedInternalType::AddressPayable(s) => Self::AddressPayable(s.to_string()), - BorrowedInternalType::Contract(s) => Self::Contract(s.to_string()), - BorrowedInternalType::Enum { contract, ty } => { - Self::Enum { contract: contract.map(String::from), ty: ty.to_string() } - } - BorrowedInternalType::Struct { contract, ty } => { - Self::Struct { contract: contract.map(String::from), ty: ty.to_string() } - } - BorrowedInternalType::Other { contract, ty } => { - Self::Other { contract: contract.map(String::from), ty: ty.to_string() } - } - } - } -} - impl fmt::Display for InternalType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.as_borrowed().fmt(f) @@ -65,20 +46,16 @@ impl fmt::Display for InternalType { } impl Serialize for InternalType { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { + #[inline] + fn serialize(&self, serializer: S) -> Result { self.as_borrowed().serialize(serializer) } } impl<'de> Deserialize<'de> for InternalType { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - deserializer.deserialize_str(ItVisitor).map(Into::into) + #[inline] + fn deserialize>(deserializer: D) -> Result { + deserializer.deserialize_str(ItVisitor) } } @@ -86,7 +63,7 @@ impl InternalType { /// Parse a string into an instance, taking ownership of data #[inline] pub fn parse(s: &str) -> Option { - BorrowedInternalType::parse(s).map(Into::into) + BorrowedInternalType::parse(s).map(BorrowedInternalType::into_owned) } /// True if the instance is a `struct` variant. @@ -248,7 +225,7 @@ impl Serialize for BorrowedInternalType<'_> { impl<'de: 'a, 'a> Deserialize<'de> for BorrowedInternalType<'a> { #[inline] fn deserialize>(deserializer: D) -> Result { - deserializer.deserialize_str(ItVisitor) + deserializer.deserialize_str(BorrowedItVisitor) } } @@ -278,33 +255,61 @@ impl<'a> BorrowedInternalType<'a> { Some(Self::Other { contract: None, ty: v }) } } + + pub(crate) fn into_owned(self) -> InternalType { + match self { + Self::AddressPayable(s) => InternalType::AddressPayable(s.to_string()), + Self::Contract(s) => InternalType::Contract(s.to_string()), + Self::Enum { contract, ty } => { + InternalType::Enum { contract: contract.map(String::from), ty: ty.to_string() } + } + Self::Struct { contract, ty } => { + InternalType::Struct { contract: contract.map(String::from), ty: ty.to_string() } + } + Self::Other { contract, ty } => { + InternalType::Other { contract: contract.map(String::from), ty: ty.to_string() } + } + } + } } +const VISITOR_EXPECTED: &str = "a valid internal type"; + pub(crate) struct ItVisitor; impl<'de> Visitor<'de> for ItVisitor { + type Value = InternalType; + + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(VISITOR_EXPECTED) + } + + fn visit_str(self, v: &str) -> Result { + BorrowedInternalType::parse(v) + .map(BorrowedInternalType::into_owned) + .ok_or_else(|| E::invalid_value(serde::de::Unexpected::Str(v), &VISITOR_EXPECTED)) + } +} + +const BORROWED_VISITOR_EXPECTED: &str = "a valid borrowed internal type"; + +pub(crate) struct BorrowedItVisitor; + +impl<'de> Visitor<'de> for BorrowedItVisitor { type Value = BorrowedInternalType<'de>; - fn expecting(&self, formatter: &mut alloc::fmt::Formatter<'_>) -> alloc::fmt::Result { - write!(formatter, "a valid internal type") + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(BORROWED_VISITOR_EXPECTED) } fn visit_borrowed_str(self, v: &'de str) -> Result { BorrowedInternalType::parse(v).ok_or_else(|| { - E::invalid_value(serde::de::Unexpected::Str(v), &"a valid internal type") + E::invalid_value(serde::de::Unexpected::Str(v), &BORROWED_VISITOR_EXPECTED) }) } - fn visit_str(self, _v: &str) -> Result - where - E: serde::de::Error, - { - // `from_reader` copies the bytes into a Vec before calling this - // method. Because the lifetime is unspecified, we can't borrow from it. - // As a result, we don't support `from_reader`. - Err(serde::de::Error::custom( - "Using serde_json::from_reader is not supported. Instead, buffer the reader contents into a string, as in alloy_json_abi::JsonAbi::load.", - )) + fn visit_str(self, v: &str) -> Result { + Err(E::invalid_value(serde::de::Unexpected::Str(v), &BORROWED_VISITOR_EXPECTED)) } } diff --git a/crates/json-abi/src/param.rs b/crates/json-abi/src/param.rs index a0104adba1..2868c5bcb4 100644 --- a/crates/json-abi/src/param.rs +++ b/crates/json-abi/src/param.rs @@ -3,11 +3,7 @@ use crate::{ utils::{mk_eparam, mk_param, validate_identifier}, InternalType, }; -use alloc::{ - borrow::{Cow, ToOwned}, - string::String, - vec::Vec, -}; +use alloc::{borrow::Cow, string::String, vec::Vec}; use core::{fmt, str::FromStr}; use parser::{Error, ParameterSpecifier, TypeSpecifier}; use serde::{de::Unexpected, Deserialize, Deserializer, Serialize, Serializer}; @@ -58,14 +54,14 @@ impl fmt::Display for Param { impl<'de> Deserialize<'de> for Param { fn deserialize>(deserializer: D) -> Result { - BorrowedParam::deserialize(deserializer).and_then(|inner| { + ParamInner::deserialize(deserializer).and_then(|inner| { if inner.indexed.is_none() { inner.validate_fields()?; Ok(Self { - name: inner.name.to_owned(), - ty: inner.ty.to_owned(), - internal_type: inner.internal_type.map(Into::into), - components: inner.components.into_owned(), + name: inner.name, + ty: inner.ty, + internal_type: inner.internal_type, + components: inner.components, }) } else { Err(serde::de::Error::custom("indexed is not supported in params")) @@ -271,8 +267,8 @@ impl Param { } #[inline] - fn as_inner(&self) -> BorrowedParam<'_> { - BorrowedParam { + fn as_inner(&self) -> BorrowedParamInner<'_> { + BorrowedParamInner { name: &self.name, ty: &self.ty, indexed: None, @@ -350,14 +346,14 @@ impl fmt::Display for EventParam { impl<'de> Deserialize<'de> for EventParam { fn deserialize>(deserializer: D) -> Result { - BorrowedParam::deserialize(deserializer).and_then(|inner| { + ParamInner::deserialize(deserializer).and_then(|inner| { inner.validate_fields()?; Ok(Self { - name: inner.name.to_owned(), - ty: inner.ty.to_owned(), + name: inner.name, + ty: inner.ty, indexed: inner.indexed.unwrap_or(false), internal_type: inner.internal_type.map(Into::into), - components: inner.components.into_owned(), + components: inner.components, }) }) } @@ -564,8 +560,8 @@ impl EventParam { } #[inline] - fn as_inner(&self) -> BorrowedParam<'_> { - BorrowedParam { + fn as_inner(&self) -> BorrowedParamInner<'_> { + BorrowedParamInner { name: &self.name, ty: &self.ty, indexed: Some(self.indexed), @@ -575,8 +571,47 @@ impl EventParam { } } +#[derive(Deserialize)] +struct ParamInner { + #[serde(default)] + name: String, + #[serde(rename = "type")] + ty: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + indexed: Option, + #[serde(rename = "internalType", default, skip_serializing_if = "Option::is_none")] + internal_type: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + components: Vec, +} + +impl Serialize for ParamInner { + #[inline] + fn serialize(&self, serializer: S) -> Result { + self.as_borrowed().serialize(serializer) + } +} + +impl ParamInner { + #[inline] + fn validate_fields(&self) -> Result<(), E> { + self.as_borrowed().validate_fields() + } + + #[inline] + fn as_borrowed(&self) -> BorrowedParamInner<'_> { + BorrowedParamInner { + name: &self.name, + ty: &self.ty, + indexed: self.indexed, + internal_type: self.internal_type.as_ref().map(InternalType::as_borrowed), + components: Cow::Borrowed(&self.components), + } + } +} + #[derive(Serialize, Deserialize)] -struct BorrowedParam<'a> { +struct BorrowedParamInner<'a> { #[serde(default)] name: &'a str, #[serde(rename = "type")] @@ -589,8 +624,7 @@ struct BorrowedParam<'a> { components: Cow<'a, [Param]>, } -impl BorrowedParam<'_> { - #[inline(always)] +impl BorrowedParamInner<'_> { fn validate_fields(&self) -> Result<(), E> { validate_identifier!(self.name); @@ -623,22 +657,26 @@ mod tests { use super::*; #[test] - fn param_from_str() { + fn param_from_json() { let param = r#"{ "internalType": "string", "name": "reason", "type": "string" }"#; - let param = serde_json::from_str::(param).unwrap(); - assert_eq!( - param, - Param { - name: "reason".into(), - ty: "string".into(), - internal_type: Some(InternalType::Other { contract: None, ty: "string".into() }), - components: vec![], - } - ); + let expected = Param { + name: "reason".into(), + ty: "string".into(), + internal_type: Some(InternalType::Other { contract: None, ty: "string".into() }), + components: vec![], + }; + + assert_eq!(serde_json::from_str::(param).unwrap(), expected); + + let param_value = serde_json::from_str::(param).unwrap(); + assert_eq!(serde_json::from_value::(param_value).unwrap(), expected); + + let reader = std::io::Cursor::new(param); + assert_eq!(serde_json::from_reader::<_, Param>(reader).unwrap(), expected); } #[test] diff --git a/crates/json-abi/tests/abi.rs b/crates/json-abi/tests/abi.rs index 19fe60c01f..544b8c7e08 100644 --- a/crates/json-abi/tests/abi.rs +++ b/crates/json-abi/tests/abi.rs @@ -48,6 +48,9 @@ fn abi_test(s: &str, path: &str, run_solc: bool) { assert_eq!(len, abi2.len()); assert_eq!(abi1, abi2); + let abi_items2: Vec> = serde_json::from_reader(std::io::Cursor::new(s)).unwrap(); + assert_eq!(abi_items2, abi_items); + #[cfg(all(feature = "std", feature = "serde_json"))] load_test(path, &abi1); to_sol_test(path, &abi1, run_solc); diff --git a/crates/json-abi/tests/it/test.rs b/crates/json-abi/tests/it/test.rs index babd7695c6..c9192e047b 100644 --- a/crates/json-abi/tests/it/test.rs +++ b/crates/json-abi/tests/it/test.rs @@ -67,15 +67,12 @@ fn test_constructor() { #[test] #[cfg_attr(miri, ignore = "no fs")] -fn no_from_reader() { +fn from_reader() { let path = "abi/Abiencoderv2Test.json"; let file_path: String = format!("tests/{path}"); let file: File = File::open(file_path).unwrap(); let buffer: BufReader = BufReader::new(file); - let res = serde_json::from_reader::<_, JsonAbi>(buffer); - assert!(res.is_err()); - assert!( - format!("{}", res.unwrap_err()).contains("Using serde_json::from_reader is not supported.") - ); + let abi = serde_json::from_reader::<_, JsonAbi>(buffer).unwrap(); + assert_eq!(abi.len(), 1); }