Skip to content

Commit ea5f9fa

Browse files
committed
Polish from_displayable_non_static() conversion
1 parent e0a8fe1 commit ea5f9fa

File tree

8 files changed

+212
-50
lines changed

8 files changed

+212
-50
lines changed

juniper/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ tap = { version = "1.0.1", optional = true }
8080
void = { version = "1.0.2", optional = true }
8181

8282
[dev-dependencies]
83+
arcstr = { version = "1.1", features = ["serde"] }
8384
bencher = "0.1.2"
8485
chrono = { version = "0.4.30", features = ["alloc"], default-features = false }
8586
compact_str = { version = "0.9", features = ["serde"] }

juniper/src/integrations/chrono.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ mod local_date_time {
184184
pub type DateTime<Tz> = chrono::DateTime<Tz>;
185185

186186
mod date_time {
187-
use std::fmt;
187+
use std::fmt::Display;
188188

189189
use chrono::{FixedOffset, SecondsFormat, TimeZone, Utc};
190190

@@ -193,7 +193,7 @@ mod date_time {
193193
pub(super) fn to_output<Tz>(v: &DateTime<Tz>) -> String
194194
where
195195
Tz: TimeZone,
196-
Tz::Offset: fmt::Display,
196+
Tz::Offset: Display,
197197
{
198198
v.with_timezone(&Utc)
199199
.to_rfc3339_opts(SecondsFormat::AutoSi, true)

juniper/src/integrations/jiff.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ mod time_zone_or_utc_offset {
327327
.to_string()
328328
},
329329
ToOwned::to_owned,
330-
) // TODO: Optimize via `Display`?
330+
)
331331
}
332332

333333
pub(super) fn from_input(s: &str) -> Result<TimeZoneOrUtcOffset, Box<str>> {
@@ -445,7 +445,7 @@ mod utc_offset {
445445
&jiff::Zoned::now().with_time_zone(jiff::tz::TimeZone::fixed(*v)),
446446
);
447447
tm.format(FORMAT, &mut buf).unwrap();
448-
buf // TODO: Optimize via `Display`?
448+
buf
449449
}
450450

451451
pub(super) fn from_input(s: &str) -> Result<UtcOffset, Box<str>> {

juniper/src/macros/helper/mod.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,23 @@ pub trait ToResultCall {
7575
fn __to_result_call(&self, input: Self::Input) -> Result<Self::Output, Self::Error>;
7676
}
7777

78-
impl<I, O> ToResultCall for fn(I) -> O {
78+
impl<I, O, E> ToResultCall for &fn(I) -> Result<O, E> {
7979
type Input = I;
8080
type Output = O;
81-
type Error = Infallible;
81+
type Error = E;
8282

8383
fn __to_result_call(&self, input: Self::Input) -> Result<Self::Output, Self::Error> {
84-
Ok(self(input))
84+
self(input)
8585
}
8686
}
8787

88-
impl<I, O, E> ToResultCall for &fn(I) -> Result<O, E> {
88+
impl<I, O> ToResultCall for fn(I) -> O {
8989
type Input = I;
9090
type Output = O;
91-
type Error = E;
91+
type Error = Infallible;
9292

9393
fn __to_result_call(&self, input: Self::Input) -> Result<Self::Output, Self::Error> {
94-
self(input)
94+
Ok(self(input))
9595
}
9696
}
9797

@@ -149,6 +149,6 @@ where
149149
type Input = I;
150150

151151
fn __to_scalar_value_call(&self, input: Self::Input) -> S {
152-
S::from_display(&self(input))
152+
S::from_displayable_non_static(&self(input))
153153
}
154154
}

juniper/src/types/scalars.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ impl<S: ScalarValue> ToScalarValue<S> for str {
312312

313313
impl<S> ToInputValue<S> for str
314314
where
315-
str: ToScalarValue<S>,
315+
Self: ToScalarValue<S>,
316316
{
317317
fn to_input_value(&self) -> InputValue<S> {
318318
InputValue::Scalar(self.to_scalar_value())

juniper/src/value/scalar.rs

Lines changed: 144 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -476,22 +476,158 @@ pub trait ScalarValue:
476476

477477
/// Creates this [`ScalarValue`] from the provided [`Display`]able type.
478478
///
479-
/// This method should be implemented if [`ScalarValue`] implementation uses some custom string
480-
/// type inside to enable efficient conversion from values of this type.
479+
/// # Usage
480+
///
481+
/// This method cannot work with non-`'static` types due to [`Any`] `'static` restriction. For
482+
/// non-`'static` types the [`ScalarValue::from_displayable_non_static()`] method should be used
483+
/// instead. However, the [`Any`] here allows implementors to specialize some conversions to be
484+
/// cheaper for their [`ScalarValue`] implementation, and so, using this method is preferred
485+
/// whenever is possible.
486+
///
487+
/// # Implementation
481488
///
482489
/// Default implementation allocates by converting [`ToString`] and [`From<String>`].
483490
///
484-
/// # Example
491+
/// This method should be implemented if [`ScalarValue`] implementation uses some custom string
492+
/// type inside to enable efficient conversion from values of this type.
485493
///
486-
/// See the [example in trait documentation](ScalarValue#example) for how it can be used.
494+
/// ```rust
495+
/// # use std::any::Any;
496+
/// #
497+
/// use arcstr::ArcStr;
498+
/// use derive_more::with_trait::{Display, From, TryInto};
499+
/// use juniper::ScalarValue;
500+
/// use serde::{Deserialize, Serialize};
501+
///
502+
/// #[derive(
503+
/// Clone, Debug, Deserialize, Display, From, PartialEq, ScalarValue, Serialize, TryInto,
504+
/// )]
505+
/// #[serde(untagged)]
506+
/// #[value(from_displayable_with = from_arcstr)]
507+
/// enum MyScalarValue {
508+
/// #[from]
509+
/// #[value(to_float, to_int)]
510+
/// Int(i32),
511+
///
512+
/// #[from]
513+
/// #[value(to_float)]
514+
/// Float(f64),
515+
///
516+
/// #[from(&str, String, ArcStr)]
517+
/// #[value(as_str, to_string)]
518+
/// String(ArcStr),
519+
///
520+
/// #[from]
521+
/// #[value(to_bool)]
522+
/// Boolean(bool),
523+
/// }
524+
///
525+
/// // Custom implementation of `ScalarValue::from_displayable()` method for specializing
526+
/// // an efficient conversions from `ArcStr` into `MyScalarValue`.
527+
/// fn from_arcstr<Str: Display + Any + ?Sized>(s: &Str) -> MyScalarValue {
528+
/// use juniper::AnyExt as _; // allows downcasting directly on types without `dyn`
529+
///
530+
/// if let Some(s) = s.downcast_ref::<ArcStr>() {
531+
/// MyScalarValue::String(s.clone()) // `Clone`ing `ArcStr` is cheap
532+
/// } else {
533+
/// // We do not override `ScalarValue::from_displayable_non_static()` here,
534+
/// // since `arcstr` crate doesn't provide API for efficient conversion into
535+
/// // an `ArcStr` for any `Display`able type, unfortunately.
536+
/// // The closest possible way is to use `arcstr::format!("{s}")` expression.
537+
/// // However, it actually expands to `ArcStr::from(fmt::format(format_args!("{s}")))`,
538+
/// // where `fmt::format()` allocates a `String`, and thus, is fully equivalent to the
539+
/// // default implementation, which does `.to_string().into()` conversion.
540+
/// MyScalarValue::from_displayable_non_static(s)
541+
/// }
542+
/// }
543+
/// #
544+
/// # // `derive_more::TryInto` is not capable for transitive conversions yet,
545+
/// # // so this impl is manual as a custom string type is used instead of `String`.
546+
/// # impl TryFrom<MyScalarValue> for String {
547+
/// # type Error = MyScalarValue;
548+
/// #
549+
/// # fn try_from(value: MyScalarValue) -> Result<Self, Self::Error> {
550+
/// # if let MyScalarValue::String(s) = value {
551+
/// # Ok(s.to_string())
552+
/// # } else {
553+
/// # Err(value)
554+
/// # }
555+
/// # }
556+
/// # }
557+
/// ```
487558
#[must_use]
488-
fn from_displayable<Str: Display + Any + ?Sized>(s: &Str) -> Self {
489-
s.to_string().into()
559+
fn from_displayable<T: Display + Any + ?Sized>(value: &T) -> Self {
560+
Self::from_displayable_non_static(value)
490561
}
491562

563+
/// Creates this [`ScalarValue`] from the provided non-`'static` [`Display`]able type.
564+
///
565+
/// # Usage
566+
///
567+
/// This method exists solely because [`Any`] requires `'static`, and so the
568+
/// [`ScalarValue::from_displayable()`] method cannot cover non-`'static` types. Always prefer
569+
/// to use the [`ScalarValue::from_displayable()`] method instead of this one, whenever it's
570+
/// possible, to allow possible cheap conversion specialization.
571+
///
572+
/// # Implementation
573+
///
574+
/// Default implementation allocates by converting [`ToString`] and [`From<String>`].
575+
///
576+
/// This method should be implemented if [`ScalarValue`] implementation uses some custom string
577+
/// type inside to create its values efficiently without intermediate [`String`]-conversion.
578+
///
579+
/// ```rust
580+
/// use compact_str::{CompactString, ToCompactString as _};
581+
/// use derive_more::with_trait::{Display, From, TryInto};
582+
/// use juniper::ScalarValue;
583+
/// use serde::{Deserialize, Serialize};
584+
///
585+
/// #[derive(
586+
/// Clone, Debug, Deserialize, Display, From, PartialEq, ScalarValue, Serialize, TryInto,
587+
/// )]
588+
/// #[serde(untagged)]
589+
/// #[value(from_displayable_non_static_with = to_compact_string)]
590+
/// enum MyScalarValue {
591+
/// #[from]
592+
/// #[value(to_float, to_int)]
593+
/// Int(i32),
594+
///
595+
/// #[from]
596+
/// #[value(to_float)]
597+
/// Float(f64),
598+
///
599+
/// #[from(&str, String, CompactString)]
600+
/// #[value(as_str, to_string)]
601+
/// String(CompactString),
602+
///
603+
/// #[from]
604+
/// #[value(to_bool)]
605+
/// Boolean(bool),
606+
/// }
607+
///
608+
/// // Custom implementation of `ScalarValue::from_displayable_non_static()` method
609+
/// // for efficient writing into a `CompactString` as a `MyScalarValue::String`.
610+
/// fn to_compact_string<T: Display + ?Sized>(v: &T) -> MyScalarValue {
611+
/// v.to_compact_string().into()
612+
/// }
613+
/// #
614+
/// # // `derive_more::TryInto` is not capable for transitive conversions yet,
615+
/// # // so this impl is manual as a custom string type is used instead of `String`.
616+
/// # impl TryFrom<MyScalarValue> for String {
617+
/// # type Error = MyScalarValue;
618+
/// #
619+
/// # fn try_from(value: MyScalarValue) -> Result<Self, Self::Error> {
620+
/// # if let MyScalarValue::String(s) = value {
621+
/// # Ok(s.into())
622+
/// # } else {
623+
/// # Err(value)
624+
/// # }
625+
/// # }
626+
/// # }
627+
/// ```
492628
#[must_use]
493-
fn from_display<Str: Display + ?Sized>(s: &Str) -> Self {
494-
s.to_string().into()
629+
fn from_displayable_non_static<T: Display + ?Sized>(value: &T) -> Self {
630+
value.to_string().into()
495631
}
496632
}
497633

juniper_codegen/src/graphql_scalar/mod.rs

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -732,20 +732,39 @@ impl Definition {
732732
fn impl_parse_scalar_value_tokens(&self) -> TokenStream {
733733
let scalar = &self.scalar;
734734

735-
let from_str = self.methods.expand_parse_scalar_value(scalar);
735+
let (ty, mut generics) = self.impl_self_and_generics(false);
736+
737+
let body = match &self.methods {
738+
Methods::Custom { parse_token, .. }
739+
| Methods::Delegated {
740+
parse_token: Some(parse_token),
741+
..
742+
} => {
743+
let parse_token = parse_token.expand_from_str(scalar);
744+
quote! { #parse_token }
745+
}
746+
Methods::Delegated { field, .. } => {
747+
let field_ty = field.ty();
748+
749+
generics.make_where_clause().predicates.push(parse_quote! {
750+
#field_ty: ::juniper::ParseScalarValue<#scalar>
751+
});
752+
753+
quote! {
754+
<#field_ty as ::juniper::ParseScalarValue<#scalar>>::from_str(token)
755+
}
756+
}
757+
};
736758

737-
let (ty, generics) = self.impl_self_and_generics(false);
738759
let (impl_gens, _, where_clause) = generics.split_for_impl();
739760

740761
quote! {
741762
#[automatically_derived]
742-
impl #impl_gens ::juniper::ParseScalarValue<#scalar> for #ty
743-
#where_clause
744-
{
763+
impl #impl_gens ::juniper::ParseScalarValue<#scalar> for #ty #where_clause {
745764
fn from_str(
746765
token: ::juniper::parser::ScalarToken<'_>,
747766
) -> ::juniper::ParseScalarResult<#scalar> {
748-
#from_str
767+
#body
749768
}
750769
}
751770
}
@@ -918,30 +937,6 @@ enum Methods {
918937
},
919938
}
920939

921-
impl Methods {
922-
/// Expands [`ParseScalarValue::from_str`] method.
923-
///
924-
/// [`ParseScalarValue::from_str`]: juniper::ParseScalarValue::from_str
925-
fn expand_parse_scalar_value(&self, scalar: &scalar::Type) -> TokenStream {
926-
match self {
927-
Self::Custom { parse_token, .. }
928-
| Self::Delegated {
929-
parse_token: Some(parse_token),
930-
..
931-
} => {
932-
let parse_token = parse_token.expand_from_str(scalar);
933-
quote! { #parse_token }
934-
}
935-
Self::Delegated { field, .. } => {
936-
let field_ty = field.ty();
937-
quote! {
938-
<#field_ty as ::juniper::ParseScalarValue<#scalar>>::from_str(token)
939-
}
940-
}
941-
}
942-
}
943-
}
944-
945940
/// Representation of [`ParseScalarValue::from_str`] method.
946941
///
947942
/// [`ParseScalarValue::from_str`]: juniper::ParseScalarValue::from_str

0 commit comments

Comments
 (0)