Strip potential redundant allocations from jiff and time GraphQLScalar implementations

tyranron · tyranron · commit c98b6d5ed7e7 · 2025-06-30T18:24:15.000+03:00
diff --git a/juniper/src/integrations/jiff.rs b/juniper/src/integrations/jiff.rs
@@ -77,7 +77,9 @@ use crate::{ScalarValue, graphql_scalar};
 pub type LocalDate = jiff::civil::Date;
 
 mod local_date {
-    use super::*;
+    use std::fmt::Display;
+
+    use super::LocalDate;
 
     /// Format of a [`LocalDate` scalar][1].
     ///
@@ -114,7 +116,9 @@ mod local_date {
 pub type LocalTime = jiff::civil::Time;
 
 mod local_time {
-    use super::*;
+    use std::fmt::Display;
+
+    use super::LocalTime;
 
     /// Full format of a [`LocalTime` scalar][1].
     ///
@@ -172,7 +176,9 @@ mod local_time {
 pub type LocalDateTime = jiff::civil::DateTime;
 
 mod local_date_time {
-    use super::*;
+    use std::fmt::Display;
+
+    use super::LocalDateTime;
 
     /// Format of a [`LocalDateTime` scalar][1].
     ///
@@ -208,7 +214,9 @@ mod local_date_time {
 pub type DateTime = jiff::Timestamp;
 
 mod date_time {
-    use super::*;
+    use std::fmt::Display;
+
+    use super::DateTime;
 
     /// Format of a [`DateTime` scalar][1].
     ///
@@ -310,24 +318,27 @@ mod duration {
 pub type TimeZoneOrUtcOffset = jiff::tz::TimeZone;
 
 mod time_zone_or_utc_offset {
-    use super::*;
+    use std::fmt::Display;
+
+    use super::{TimeZoneOrUtcOffset, TimeZoneParsingError, utc_offset};
+    use crate::util::Either;
 
     /// Format of a [`TimeZoneOrUtcOffset`] scalar.
     const FORMAT: &str = "%:Q";
 
-    pub(super) fn to_output(v: &TimeZoneOrUtcOffset) -> String {
-        v.iana_name().map_or_else(
-            || {
-                // If no IANA time zone identifier is available, fall back to displaying the time
-                // offset directly (using format `[+-]HH:MM[:SS]` from RFC 9557, e.g. `+05:30`).
-                // See: https://github.com/graphql-rust/juniper/pull/1278#discussion_r1719161686
+    pub(super) fn to_output(v: &TimeZoneOrUtcOffset) -> impl Display {
+        if let Some(name) = v.iana_name() {
+            Either::Left(name)
+        } else {
+            // If no IANA time zone identifier is available, fall back to displaying the time
+            // offset directly (using format `[+-]HH:MM[:SS]` from RFC 9557, e.g. `+05:30`).
+            // See: https://github.com/graphql-rust/juniper/pull/1278#discussion_r1719161686
+            Either::Right(
                 jiff::Zoned::now()
                     .with_time_zone(v.clone())
-                    .strftime(FORMAT)
-                    .to_string()
-            },
-            ToOwned::to_owned,
-        )
+                    .strftime(FORMAT),
+            )
+        }
     }
 
     pub(super) fn from_input(s: &str) -> Result<TimeZoneOrUtcOffset, Box<str>> {
@@ -424,28 +435,39 @@ mod time_zone {
 pub type UtcOffset = jiff::tz::Offset;
 
 mod utc_offset {
-    use super::*;
+    use std::fmt::{self, Display};
+
+    use jiff::fmt::{StdFmtWrite, strtime::BrokenDownTime};
+
+    use super::UtcOffset;
 
     /// Format of a [`UtcOffset` scalar][1].
     ///
     /// [1]: https://graphql-scalars.dev/docs/scalars/utc-offset
     const FORMAT: &str = "%:z";
 
     pub(super) fn utc_offset_from_str(value: &str) -> Result<jiff::tz::Offset, jiff::Error> {
-        let tm = jiff::fmt::strtime::BrokenDownTime::parse(FORMAT, value)?;
+        let tm = BrokenDownTime::parse(FORMAT, value)?;
         let offset = tm
             .offset()
             .expect("successful %:z parsing guarantees offset");
         Ok(offset)
     }
 
-    pub(super) fn to_output(v: &UtcOffset) -> String {
-        let mut buf = String::new();
-        let tm = jiff::fmt::strtime::BrokenDownTime::from(
+    pub(super) fn to_output(v: &UtcOffset) -> impl Display {
+        struct LazyFmt(BrokenDownTime);
+
+        impl Display for LazyFmt {
+            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+                self.0
+                    .format(FORMAT, StdFmtWrite(f))
+                    .map_err(|_| fmt::Error)
+            }
+        }
+
+        LazyFmt(BrokenDownTime::from(
             &jiff::Zoned::now().with_time_zone(jiff::tz::TimeZone::fixed(*v)),
-        );
-        tm.format(FORMAT, &mut buf).unwrap();
-        buf
+        ))
     }
 
     pub(super) fn from_input(s: &str) -> Result<UtcOffset, Box<str>> {
diff --git a/juniper/src/integrations/time.rs b/juniper/src/integrations/time.rs
@@ -22,6 +22,10 @@
 //! [s4]: https://graphql-scalars.dev/docs/scalars/date-time
 //! [s5]: https://graphql-scalars.dev/docs/scalars/utc-offset
 
+use std::{
+    fmt::{self, Display},
+    io, str,
+};
 use time::{
     format_description::{BorrowedFormatItem, well_known::Rfc3339},
     macros::format_description,
@@ -56,9 +60,17 @@ mod local_date {
     /// [1]: https://graphql-scalars.dev/docs/scalars/local-date
     const FORMAT: &[BorrowedFormatItem<'_>] = format_description!("[year]-[month]-[day]");
 
-    pub(super) fn to_output(v: &LocalDate) -> String {
-        v.format(FORMAT)
-            .unwrap_or_else(|e| panic!("failed to format `LocalDate`: {e}"))
+    impl Display for LazyFmt<&LocalDate> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            self.0
+                .format_into(&mut IoAdapter(f), FORMAT)
+                .map_err(|_| fmt::Error)
+                .map(drop)
+        }
+    }
+
+    pub(super) fn to_output(v: &LocalDate) -> impl Display {
+        LazyFmt(v)
     }
 
     pub(super) fn from_input(s: &str) -> Result<LocalDate, Box<str>> {
@@ -106,13 +118,24 @@ mod local_time {
     /// [1]: https://graphql-scalars.dev/docs/scalars/local-time
     const FORMAT_NO_SECS: &[BorrowedFormatItem<'_>] = format_description!("[hour]:[minute]");
 
-    pub(super) fn to_output(v: &LocalTime) -> String {
-        if v.millisecond() == 0 {
-            v.format(FORMAT_NO_MILLIS)
-        } else {
-            v.format(FORMAT)
+    impl Display for LazyFmt<&LocalTime> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            self.0
+                .format_into(
+                    &mut IoAdapter(f),
+                    if self.0.millisecond() == 0 {
+                        FORMAT_NO_MILLIS
+                    } else {
+                        FORMAT
+                    },
+                )
+                .map_err(|_| fmt::Error)
+                .map(drop)
         }
-        .unwrap_or_else(|e| panic!("failed to format `LocalTime`: {e}"))
+    }
+
+    pub(super) fn to_output(v: &LocalTime) -> impl Display {
+        LazyFmt(v)
     }
 
     pub(super) fn from_input(s: &str) -> Result<LocalTime, Box<str>> {
@@ -150,9 +173,17 @@ mod local_date_time {
     const FORMAT: &[BorrowedFormatItem<'_>] =
         format_description!("[year]-[month]-[day]T[hour]:[minute]:[second]");
 
-    pub(super) fn to_output(v: &LocalDateTime) -> String {
-        v.format(FORMAT)
-            .unwrap_or_else(|e| panic!("failed to format `LocalDateTime`: {e}"))
+    impl Display for LazyFmt<&LocalDateTime> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            self.0
+                .format_into(&mut IoAdapter(f), FORMAT)
+                .map_err(|_| fmt::Error)
+                .map(drop)
+        }
+    }
+
+    pub(super) fn to_output(v: &LocalDateTime) -> impl Display {
+        LazyFmt(v)
     }
 
     pub(super) fn from_input(s: &str) -> Result<LocalDateTime, Box<str>> {
@@ -183,10 +214,18 @@ pub type DateTime = time::OffsetDateTime;
 mod date_time {
     use super::*;
 
-    pub(super) fn to_output(v: &DateTime) -> String {
-        v.to_offset(UtcOffset::UTC)
-            .format(&Rfc3339)
-            .unwrap_or_else(|e| panic!("failed to format `DateTime`: {e}"))
+    impl Display for LazyFmt<&DateTime> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            self.0
+                .to_offset(UtcOffset::UTC)
+                .format_into(&mut IoAdapter(f), &Rfc3339)
+                .map_err(|_| fmt::Error)
+                .map(drop)
+        }
+    }
+
+    pub(super) fn to_output(v: &DateTime) -> impl Display {
+        LazyFmt(v)
     }
 
     pub(super) fn from_input(s: &str) -> Result<DateTime, Box<str>> {
@@ -222,9 +261,17 @@ pub type UtcOffset = time::UtcOffset;
 mod utc_offset {
     use super::*;
 
-    pub(super) fn to_output(v: &UtcOffset) -> String {
-        v.format(UTC_OFFSET_FORMAT)
-            .unwrap_or_else(|e| panic!("failed to format `UtcOffset`: {e}"))
+    impl Display for LazyFmt<&UtcOffset> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            self.0
+                .format_into(&mut IoAdapter(f), UTC_OFFSET_FORMAT)
+                .map_err(|_| fmt::Error)
+                .map(drop)
+        }
+    }
+
+    pub(super) fn to_output(v: &UtcOffset) -> impl Display {
+        LazyFmt(v)
     }
 
     pub(super) fn from_input(s: &str) -> Result<UtcOffset, Box<str>> {
@@ -233,6 +280,30 @@ mod utc_offset {
     }
 }
 
+// TODO: Remove once time-rs/time#375 is resolved:
+//       https://github.com/time-rs/time/issues/375
+/// [`io::Write`] adapter for [`fmt::Formatter`].
+///
+/// Required because [`time`] crate cannot write to [`fmt::Write`], only to [`io::Write`].
+struct IoAdapter<'a, 'b>(&'a mut fmt::Formatter<'b>);
+
+impl io::Write for IoAdapter<'_, '_> {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        let s = str::from_utf8(buf).map_err(io::Error::other)?;
+        match self.0.write_str(s) {
+            Ok(_) => Ok(s.len()),
+            Err(e) => Err(io::Error::other(e)),
+        }
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        Ok(())
+    }
+}
+
+/// Wrapper over [`time`] crate types to [`Display`] their format lazily.
+struct LazyFmt<T>(T);
+
 #[cfg(test)]
 mod local_date_test {
     use time::macros::date;
diff --git a/juniper/src/util.rs b/juniper/src/util.rs
@@ -1,5 +1,7 @@
 use std::borrow::Cow;
 
+use derive_more::with_trait::Display;
+
 /// Convert string to camel case.
 ///
 /// Note: needs to be public because several macros use it.
@@ -51,3 +53,13 @@ fn test_to_camel_case() {
     assert_eq!(&to_camel_case("a")[..], "a");
     assert_eq!(&to_camel_case("")[..], "");
 }
+
+/// Combination of two [`Display`]able types into a single one.
+#[derive(Display)]
+pub(crate) enum Either<L, R> {
+    /// Left value of the first type.
+    Left(L),
+
+    /// Right value of the second type.
+    Right(R),
+}
diff --git a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs
@@ -8,6 +8,7 @@ use crate::{
     ast::{Arguments, Definition, Document, Field, Fragment, FragmentSpread, Selection, Type},
     parser::{SourcePosition, Spanning},
     schema::meta::{Field as FieldType, MetaType},
+    util::Either,
     validation::{ValidatorContext, Visitor},
     value::ScalarValue,
 };
@@ -748,15 +749,9 @@ fn error_message(reason_name: &str, reason: &ConflictReasonMessage) -> String {
 }
 
 fn format_reason(reason: &ConflictReasonMessage) -> impl Display {
-    #[derive(Display)]
-    enum Either<A, B> {
-        A(A),
-        B(B),
-    }
-
     match reason {
-        ConflictReasonMessage::Message(name) => Either::A(name),
-        ConflictReasonMessage::Nested(nested) => Either::B(nested.iter().format_with(
+        ConflictReasonMessage::Message(name) => Either::Left(name),
+        ConflictReasonMessage::Nested(nested) => Either::Right(nested.iter().format_with(
             " and ",
             |ConflictReason(name, subreason), f| {
                 f(&format_args!(
