From fe921ac31df33f4cb4e3178e6a51f4c35a36bbe3 Mon Sep 17 00:00:00 2001 From: tyranron Date: Thu, 15 May 2025 19:29:39 +0200 Subject: [PATCH 01/11] Strip some redundant `.as_ref()`s --- juniper/src/schema/model.rs | 7 ++----- .../validation/rules/overlapping_fields_can_be_merged.rs | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 1176e926d..346214c67 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -397,16 +397,13 @@ impl SchemaType { Type::List(inner, expected_size) => { TypeType::List(Box::new(self.make_type(inner)), *expected_size) } - Type::Named(n) => self - .type_by_name(n.as_ref()) - .expect("Type not found in schema"), + Type::Named(n) => self.type_by_name(n).expect("Type not found in schema"), Type::NonNullList(inner, expected_size) => TypeType::NonNull(Box::new(TypeType::List( Box::new(self.make_type(inner)), *expected_size, ))), Type::NonNullNamed(n) => TypeType::NonNull(Box::new( - self.type_by_name(n.as_ref()) - .expect("Type not found in schema"), + self.type_by_name(n).expect("Type not found in schema"), )), } } diff --git a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs index 698f9fdc7..aa98b5391 100644 --- a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs +++ b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs @@ -570,8 +570,8 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> { } (Type::Named(n1), Type::Named(n2)) | (Type::NonNullNamed(n1), Type::NonNullNamed(n2)) => { - let ct1 = ctx.schema.concrete_type_by_name(n1.as_ref()); - let ct2 = ctx.schema.concrete_type_by_name(n2.as_ref()); + let ct1 = ctx.schema.concrete_type_by_name(n1); + let ct2 = ctx.schema.concrete_type_by_name(n2); if ct1.map(MetaType::is_leaf).unwrap_or(false) || ct2.map(MetaType::is_leaf).unwrap_or(false) From 3fc160feea6f5d1c1a3cee2699c8fd2807386330 Mon Sep 17 00:00:00 2001 From: tyranron Date: Tue, 20 May 2025 20:00:15 +0200 Subject: [PATCH 02/11] Remove `DynType` from `SchemaType::is_subtype()` --- juniper/CHANGELOG.md | 1 - juniper/src/ast.rs | 18 +++++++++++++- juniper/src/schema/model.rs | 24 +++++++++++-------- .../rules/variables_in_allowed_position.rs | 2 +- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md index 9278f9b9d..2e91cb670 100644 --- a/juniper/CHANGELOG.md +++ b/juniper/CHANGELOG.md @@ -52,7 +52,6 @@ All user visible changes to `juniper` crate will be documented in this file. Thi - Made `name` and `description` fields using `ArcStr`. - `SchemaType`: - Removed lifetime parameters. - - Made `is_subtype()` method accepting `DynType` instead of `Type`. - `RootNode`: - Removed lifetime parameters. - `Registry`: diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index a7ee27ef2..6b96d4dc6 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -13,7 +13,7 @@ use crate::{ /// Type literal in a syntax tree. /// /// This enum carries no semantic information and might refer to types that do not exist. -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug)] pub enum Type { /// `null`able named type, e.g. `String`. Named(N), @@ -32,6 +32,22 @@ pub enum Type { NonNullList(Box>, Option), } +impl Eq for Type where Self: PartialEq {} + +impl, N2: AsRef> PartialEq> for Type { + fn eq(&self, other: &Type) -> bool { + match (self, other) { + (Self::Named(n1), Type::Named(n2)) => n1.as_ref() == n2.as_ref(), + (Self::List(lhs, s1), Type::List(rhs, s2)) => s1 == s2 && lhs.as_ref() == rhs.as_ref(), + (Self::NonNullNamed(n1), Type::NonNullNamed(n2)) => n1.as_ref() == n2.as_ref(), + (Self::NonNullList(lhs, s1), Type::NonNullList(rhs, s2)) => { + s1 == s2 && lhs.as_ref() == rhs.as_ref() + } + _ => false, + } + } +} + impl fmt::Display for Type { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 346214c67..4f76a3f61 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -473,23 +473,27 @@ impl SchemaType { } /// If the type is a subtype of another type. - pub fn is_subtype<'b>(&self, sub_type: &DynType<'b>, super_type: &DynType<'b>) -> bool { - use DynType::*; + pub fn is_subtype( + &self, + sub_type: &Type>, + super_type: &Type>, + ) -> bool { + use Type::*; - if super_type.equals(sub_type) { + if super_type == sub_type { return true; } match (super_type, sub_type) { - (&NonNullNamed(ref super_name), &NonNullNamed(ref sub_name)) - | (&Named(ref super_name), &Named(ref sub_name)) - | (&Named(ref super_name), &NonNullNamed(ref sub_name)) => { + (NonNullNamed(super_name), NonNullNamed(sub_name)) + | (Named(super_name), Named(sub_name)) + | (Named(super_name), NonNullNamed(sub_name)) => { self.is_named_subtype(sub_name.as_ref(), super_name.as_ref()) } - (&NonNullList(super_inner, _), &NonNullList(sub_inner, _)) - | (&List(super_inner, _), &List(sub_inner, _)) - | (&List(super_inner, _), &NonNullList(sub_inner, _)) => { - self.is_subtype(&sub_inner.as_dyn_type(), &super_inner.as_dyn_type()) + (NonNullList(super_inner, _), NonNullList(sub_inner, _)) + | (List(super_inner, _), List(sub_inner, _)) + | (List(super_inner, _), NonNullList(sub_inner, _)) => { + self.is_subtype(sub_inner, super_inner) } _ => false, } diff --git a/juniper/src/validation/rules/variables_in_allowed_position.rs b/juniper/src/validation/rules/variables_in_allowed_position.rs index 8bbed58d9..c0bbb6a20 100644 --- a/juniper/src/validation/rules/variables_in_allowed_position.rs +++ b/juniper/src/validation/rules/variables_in_allowed_position.rs @@ -94,7 +94,7 @@ impl<'a, S: fmt::Debug> VariableInAllowedPosition<'a, S> { if !ctx .schema - .is_subtype(&expected_type.as_dyn_type(), var_type) + .is_subtype(&expected_type, var_type) { ctx.report_error( &error_message(var_name.item, expected_type, var_type), From 2c6fed0e1726991de316f3331c7674d5402248ae Mon Sep 17 00:00:00 2001 From: tyranron Date: Tue, 20 May 2025 20:45:29 +0200 Subject: [PATCH 03/11] Resculpt `ast::Type`, vol.1 --- juniper/src/ast.rs | 117 +++++++++++------- .../rules/variables_in_allowed_position.rs | 5 +- 2 files changed, 70 insertions(+), 52 deletions(-) diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index 6b96d4dc6..da4717482 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -10,64 +10,85 @@ use crate::{ value::{DefaultScalarValue, ScalarValue}, }; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum TypeModifier { + NonNull, + List(Option), +} + +pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>; + /// Type literal in a syntax tree. /// -/// This enum carries no semantic information and might refer to types that do not exist. +/// Carries no semantic information and might refer to types that don't exist. #[derive(Clone, Debug)] -pub enum Type { - /// `null`able named type, e.g. `String`. - Named(N), - - /// `null`able list type, e.g. `[String]`. - /// - /// The list itself is `null`able, the containing [`Type`] might be non-`null`. - List(Box>, Option), - - /// Non-`null` named type, e.g. `String!`. - NonNullNamed(N), - - /// Non-`null` list type, e.g. `[String]!`. - /// - /// The list itself is non-`null`, the containing [`Type`] might be `null`able. - NonNullList(Box>, Option), +pub struct Type> { + name: N, + modifiers: M, } -impl Eq for Type where Self: PartialEq {} +impl Eq for Type where Self: PartialEq {} -impl, N2: AsRef> PartialEq> for Type { - fn eq(&self, other: &Type) -> bool { - match (self, other) { - (Self::Named(n1), Type::Named(n2)) => n1.as_ref() == n2.as_ref(), - (Self::List(lhs, s1), Type::List(rhs, s2)) => s1 == s2 && lhs.as_ref() == rhs.as_ref(), - (Self::NonNullNamed(n1), Type::NonNullNamed(n2)) => n1.as_ref() == n2.as_ref(), - (Self::NonNullList(lhs, s1), Type::NonNullList(rhs, s2)) => { - s1 == s2 && lhs.as_ref() == rhs.as_ref() - } - _ => false, - } +impl PartialEq> for Type +where + N1: AsRef, + N2: AsRef, + M1: AsRef<[TypeModifier]>, + M2: AsRef<[TypeModifier]>, +{ + fn eq(&self, other: &Type) -> bool { + self.name.as_ref() == other.name.as_ref() + && self.modifiers.as_ref() == other.modifiers.as_ref() } } -impl fmt::Display for Type { +impl fmt::Display for Type +where + N: AsRef, + M: AsRef<[TypeModifier]>, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Named(n) => write!(f, "{n}"), - Self::NonNullNamed(n) => write!(f, "{n}!"), - Self::List(t, _) => write!(f, "[{t}]"), - Self::NonNullList(t, _) => write!(f, "[{t}]!"), + // PANIC: `self.inner().unwrap()` never panics if there is at least on `TypeModifier`. + match self.modifiers.as_ref().last() { + Some(TypeModifier::NonNull) => write!(f, "{}!", self.inner().unwrap()), + Some(TypeModifier::List(..)) => write!(f, "[{}]", self.inner().unwrap()), + None => write!(f, "{}", self.name.as_ref()), } } } -impl> Type { +impl, M> Type { + pub(crate) fn inner(&self) -> Option> + where + M: AsRef<[TypeModifier]>, + { + let modifiers = self.modifiers.as_ref(); + match modifiers.len() { + 0 => None, + n => Some(Type { + name: self.name.as_ref(), + modifiers: &modifiers[..n - 1], + }), + } + } + /// Returns the name of this named [`Type`]. /// /// Only applies to named [`Type`]s. Lists will return [`None`]. #[must_use] - pub fn name(&self) -> Option<&str> { - match self { - Self::Named(n) | Self::NonNullNamed(n) => Some(n.as_ref()), - Self::List(..) | Self::NonNullList(..) => None, + pub fn name(&self) -> Option<&str> + where + M: AsRef<[TypeModifier]>, + { + if self + .modifiers + .as_ref() + .iter() + .any(|m| matches!(m, TypeModifier::List(..))) + { + None + } else { + Some(self.name.as_ref()) } } @@ -76,18 +97,18 @@ impl> Type { /// All [`Type`] literals contain exactly one named type. #[must_use] pub fn innermost_name(&self) -> &str { - match self { - Self::Named(n) | Self::NonNullNamed(n) => n.as_ref(), - Self::List(l, ..) | Self::NonNullList(l, ..) => l.innermost_name(), - } + self.name.as_ref() } /// Indicates whether this [`Type`] can only represent non-`null` values. #[must_use] - pub fn is_non_null(&self) -> bool { - match self { - Self::NonNullList(..) | Self::NonNullNamed(..) => true, - Self::List(..) | Self::Named(..) => false, + pub fn is_non_null(&self) -> bool + where + M: AsRef<[TypeModifier]>, + { + match self.modifiers.as_ref().last() { + Some(TypeModifier::NonNull) => true, + Some(TypeModifier::List(..)) | None => false, } } } diff --git a/juniper/src/validation/rules/variables_in_allowed_position.rs b/juniper/src/validation/rules/variables_in_allowed_position.rs index c0bbb6a20..4ea31730a 100644 --- a/juniper/src/validation/rules/variables_in_allowed_position.rs +++ b/juniper/src/validation/rules/variables_in_allowed_position.rs @@ -92,10 +92,7 @@ impl<'a, S: fmt::Debug> VariableInAllowedPosition<'a, S> { (_, ty) => ty.clone(), }; - if !ctx - .schema - .is_subtype(&expected_type, var_type) - { + if !ctx.schema.is_subtype(&expected_type, var_type) { ctx.report_error( &error_message(var_name.item, expected_type, var_type), &[var_def_name.span.start, var_name.span.start], From b8027e7f4a69e8261079c411229dde9e59bca168 Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 21 May 2025 18:37:45 +0200 Subject: [PATCH 04/11] Resculpt `ast::Type`, vol.2 --- juniper/Cargo.toml | 1 + juniper/src/ast.rs | 102 ++++++++++++++++-- juniper/src/executor/mod.rs | 2 +- juniper/src/parser/document.rs | 16 ++- juniper/src/parser/tests/value.rs | 4 +- juniper/src/schema/meta.rs | 12 +-- juniper/src/schema/model.rs | 62 +++++------ .../src/schema/translate/graphql_parser.rs | 23 ++-- juniper/src/validation/context.rs | 4 +- .../rules/overlapping_fields_can_be_merged.rs | 36 ++++--- .../rules/variables_in_allowed_position.rs | 11 +- 11 files changed, 176 insertions(+), 97 deletions(-) diff --git a/juniper/Cargo.toml b/juniper/Cargo.toml index f4859d8c7..35defbbd4 100644 --- a/juniper/Cargo.toml +++ b/juniper/Cargo.toml @@ -61,6 +61,7 @@ rust_decimal = { version = "1.20", default-features = false, optional = true } ryu = { version = "1.0", optional = true } serde = { version = "1.0.122", features = ["derive"] } serde_json = { version = "1.0.18", features = ["std"], default-features = false, optional = true } +smallvec = "1.15" static_assertions = "1.1" time = { version = "0.3.37", features = ["formatting", "macros", "parsing"], optional = true } url = { version = "2.0", optional = true } diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index da4717482..c5a28a1b7 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -1,8 +1,8 @@ use std::{borrow::Cow, fmt, hash::Hash, slice, vec}; use arcstr::ArcStr; - use indexmap::IndexMap; +use smallvec::{SmallVec, smallvec}; use crate::{ executor::Variables, @@ -22,7 +22,7 @@ pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>; /// /// Carries no semantic information and might refer to types that don't exist. #[derive(Clone, Debug)] -pub struct Type> { +pub struct Type> { name: N, modifiers: M, } @@ -48,27 +48,56 @@ where M: AsRef<[TypeModifier]>, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // PANIC: `self.inner().unwrap()` never panics if there is at least on `TypeModifier`. - match self.modifiers.as_ref().last() { - Some(TypeModifier::NonNull) => write!(f, "{}!", self.inner().unwrap()), - Some(TypeModifier::List(..)) => write!(f, "[{}]", self.inner().unwrap()), + match self.modifier() { + Some(TypeModifier::NonNull) => write!(f, "{}!", self.inner()), + Some(TypeModifier::List(..)) => write!(f, "[{}]", self.inner()), None => write!(f, "{}", self.name.as_ref()), } } } +impl<'a, N, M> From<&'a Type> for BorrowedType<'a> +where + N: AsRef, + M: AsRef<[TypeModifier]>, +{ + fn from(value: &'a Type) -> Self { + Self { + name: value.name.as_ref(), + modifiers: value.modifiers.as_ref(), + } + } +} + +impl<'a> BorrowedType<'a> { + pub(crate) fn inner_borrowed(&self) -> BorrowedType<'a> { + let modifiers = self.modifiers.as_ref(); + match modifiers.len() { + 0 => unreachable!(), + n => Type { + name: self.name, + modifiers: &modifiers[..n - 1], + }, + } + } + + pub(crate) fn borrowed_name(&self) -> &'a str { + self.name + } +} + impl, M> Type { - pub(crate) fn inner(&self) -> Option> + pub(crate) fn inner(&self) -> BorrowedType<'_> where M: AsRef<[TypeModifier]>, { let modifiers = self.modifiers.as_ref(); match modifiers.len() { - 0 => None, - n => Some(Type { + 0 => unreachable!(), + n => Type { name: self.name.as_ref(), modifiers: &modifiers[..n - 1], - }), + }, } } @@ -100,6 +129,15 @@ impl, M> Type { self.name.as_ref() } + /// Returns the [`TypeModifier`] of this [`Type`], if any. + #[must_use] + pub fn modifier(&self) -> Option<&TypeModifier> + where + M: AsRef<[TypeModifier]>, + { + self.modifiers.as_ref().last() + } + /// Indicates whether this [`Type`] can only represent non-`null` values. #[must_use] pub fn is_non_null(&self) -> bool @@ -111,6 +149,50 @@ impl, M> Type { Some(TypeModifier::List(..)) | None => false, } } + + #[must_use] + pub fn is_list(&self) -> bool + where + M: AsRef<[TypeModifier]>, + { + match self.modifiers.as_ref().last() { + Some(TypeModifier::NonNull) => self.inner().is_non_null(), + Some(TypeModifier::List(..)) => true, + None => false, + } + } +} + +impl> Type { + /// Wraps this [`Type`] into the provided [`TypeModifier`]. + fn wrap(mut self, modifier: TypeModifier) -> Self { + self.modifiers.push(modifier); + self + } + + pub fn wrap_list(self, size_hint: Option) -> Self { + // TODO: assert? + self.wrap(TypeModifier::List(size_hint)) + } + + pub fn wrap_non_null(self) -> Self { + // TODO: assert? + self.wrap(TypeModifier::NonNull) + } + + pub fn nullable(mut self) -> Self { + if self.is_non_null() { + _ = self.modifiers.pop(); + } + self + } + + pub fn named(name: impl Into) -> Self { + Self { + name: name.into(), + modifiers: smallvec![], + } + } } /// A JSON-like value that can be passed into the query execution, either diff --git a/juniper/src/executor/mod.rs b/juniper/src/executor/mod.rs index cc02440a3..e3955bcad 100644 --- a/juniper/src/executor/mod.rs +++ b/juniper/src/executor/mod.rs @@ -1155,7 +1155,7 @@ impl Registry { if let Some(name) = T::name(info) { let validated_name = Name::new(name.clone()).unwrap(); if !self.types.contains_key(&name) { - self.insert_placeholder(validated_name.clone(), Type::NonNullNamed(name.clone())); + self.insert_placeholder(validated_name.clone(), Type::named(name.clone()).wrap_non_null()); let meta = T::meta(info, self); self.types.insert(validated_name, meta); } diff --git a/juniper/src/parser/document.rs b/juniper/src/parser/document.rs index 4bae92f22..20aa0f17c 100644 --- a/juniper/src/parser/document.rs +++ b/juniper/src/parser/document.rs @@ -517,10 +517,10 @@ pub fn parse_type<'a>(parser: &mut Parser<'a>) -> ParseResult> { Spanning::start_end( &start_span.start, &end_pos, - Type::List(Box::new(inner_type.item), None), + inner_type.item.wrap_list(None), ) } else { - parser.expect_name()?.map(Type::Named) + parser.expect_name()?.map(Type::named) }; Ok(match *parser.peek() { @@ -534,15 +534,13 @@ pub fn parse_type<'a>(parser: &mut Parser<'a>) -> ParseResult> { fn wrap_non_null<'a>( parser: &mut Parser<'a>, - inner: Spanning>, + mut inner: Spanning>, ) -> ParseResult> { let end_pos = &parser.expect(&Token::ExclamationMark)?.span.end; - let wrapped = match inner.item { - Type::Named(name) => Type::NonNullNamed(name), - Type::List(l, expected_size) => Type::NonNullList(l, expected_size), - ty @ (Type::NonNullList(..) | Type::NonNullNamed(..)) => ty, - }; + if !inner.item.is_non_null() { + inner.item = inner.item.wrap_non_null(); + } - Ok(Spanning::start_end(&inner.span.start, end_pos, wrapped)) + Ok(Spanning::start_end(&inner.span.start, end_pos, inner.item)) } diff --git a/juniper/src/parser/tests/value.rs b/juniper/src/parser/tests/value.rs index 66c3d547d..0ed7df4ce 100644 --- a/juniper/src/parser/tests/value.rs +++ b/juniper/src/parser/tests/value.rs @@ -173,8 +173,8 @@ fn input_value_literals() { ), ); let fields = [ - Argument::new("key", Type::NonNullNamed("Int".into())), - Argument::new("other", Type::NonNullNamed("Bar".into())), + Argument::new("key", Type::named("Int").wrap_non_null()), + Argument::new("other", Type::named("Bar").wrap_non_null()), ]; let meta = &MetaType::InputObject(InputObjectMeta::new::("foo", &fields)); assert_eq!( diff --git a/juniper/src/schema/meta.rs b/juniper/src/schema/meta.rs index 2d6e27d52..82d15a1d1 100644 --- a/juniper/src/schema/meta.rs +++ b/juniper/src/schema/meta.rs @@ -710,18 +710,12 @@ impl MetaType { | Self::Interface(InterfaceMeta { name, .. }) | Self::Object(ObjectMeta { name, .. }) | Self::Scalar(ScalarMeta { name, .. }) - | Self::Union(UnionMeta { name, .. }) => Type::NonNullNamed(name.clone()), + | Self::Union(UnionMeta { name, .. }) => Type::named(name.clone()).wrap_non_null(), Self::List(ListMeta { of_type, expected_size, - }) => Type::NonNullList(Box::new(of_type.clone()), *expected_size), - Self::Nullable(NullableMeta { of_type }) => match of_type { - Type::NonNullNamed(inner) => Type::Named(inner.clone()), - Type::NonNullList(inner, expected_size) => { - Type::List(inner.clone(), *expected_size) - } - ty @ (Type::List(..) | Type::Named(..)) => ty.clone(), - }, + }) => of_type.clone().wrap_list(*expected_size).wrap_non_null(), + Self::Nullable(NullableMeta { of_type }) => of_type.clone().nullable(), Self::Placeholder(PlaceholderMeta { of_type }) => of_type.clone(), } } diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 4f76a3f61..37711e0e7 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -7,7 +7,7 @@ use graphql_parser::schema::Document; use crate::{ GraphQLEnum, - ast::Type, + ast::{Type, TypeModifier, TypeModifier::NonNull}, executor::{Context, Registry}, schema::meta::{Argument, InterfaceMeta, MetaType, ObjectMeta, PlaceholderMeta, UnionMeta}, types::{base::GraphQLType, name::Name}, @@ -318,12 +318,14 @@ impl SchemaType { self.types.get(name.as_ref()) } - pub(crate) fn lookup_type(&self, tpe: &Type>) -> Option<&MetaType> { - match tpe { - Type::Named(name) | Type::NonNullNamed(name) => { - self.concrete_type_by_name(name.as_ref()) - } - Type::List(inner, ..) | Type::NonNullList(inner, ..) => self.lookup_type(inner), + pub(crate) fn lookup_type( + &self, + ty: &Type, impl AsRef<[TypeModifier]>>, + ) -> Option<&MetaType> { + if let Some(name) = ty.name() { + self.concrete_type_by_name(name) + } else { + self.lookup_type(&ty.inner()) } } @@ -392,19 +394,15 @@ impl SchemaType { } /// Make a type. - pub fn make_type(&self, ty: &Type>) -> TypeType { - match ty { - Type::List(inner, expected_size) => { - TypeType::List(Box::new(self.make_type(inner)), *expected_size) + pub fn make_type(&self, ty: &Type, impl AsRef<[TypeModifier]>>) -> TypeType { + match ty.modifier() { + Some(TypeModifier::NonNull) => TypeType::NonNull(Box::new(self.make_type(&ty.inner()))), + Some(TypeModifier::List(expected_size)) => { + TypeType::List(Box::new(self.make_type(&ty.inner())), *expected_size) } - Type::Named(n) => self.type_by_name(n).expect("Type not found in schema"), - Type::NonNullList(inner, expected_size) => TypeType::NonNull(Box::new(TypeType::List( - Box::new(self.make_type(inner)), - *expected_size, - ))), - Type::NonNullNamed(n) => TypeType::NonNull(Box::new( - self.type_by_name(n).expect("Type not found in schema"), - )), + None => self + .type_by_name(ty.innermost_name()) + .expect("Type not found in schema"), } } @@ -475,25 +473,27 @@ impl SchemaType { /// If the type is a subtype of another type. pub fn is_subtype( &self, - sub_type: &Type>, - super_type: &Type>, + sub_type: &Type, impl AsRef<[TypeModifier]>>, + super_type: &Type, impl AsRef<[TypeModifier]>>, ) -> bool { - use Type::*; + use TypeModifier::{List, NonNull}; if super_type == sub_type { return true; } - match (super_type, sub_type) { - (NonNullNamed(super_name), NonNullNamed(sub_name)) - | (Named(super_name), Named(sub_name)) - | (Named(super_name), NonNullNamed(sub_name)) => { - self.is_named_subtype(sub_name.as_ref(), super_name.as_ref()) + match (super_type.modifier(), sub_type.modifier()) { + (Some(NonNull), Some(NonNull)) => { + self.is_subtype(&sub_type.inner(), &super_type.inner()) + } + (None | Some(List(..)), Some(NonNull)) => { + self.is_subtype(&sub_type.inner(), super_type) + } + (Some(List(..)), Some(List(..))) => { + self.is_subtype(&sub_type.inner(), &super_type.inner()) } - (NonNullList(super_inner, _), NonNullList(sub_inner, _)) - | (List(super_inner, _), List(sub_inner, _)) - | (List(super_inner, _), NonNullList(sub_inner, _)) => { - self.is_subtype(sub_inner, super_inner) + (None, None) => { + self.is_named_subtype(sub_type.innermost_name(), super_type.innermost_name()) } _ => false, } diff --git a/juniper/src/schema/translate/graphql_parser.rs b/juniper/src/schema/translate/graphql_parser.rs index 9ad12d553..a29914ab0 100644 --- a/juniper/src/schema/translate/graphql_parser.rs +++ b/juniper/src/schema/translate/graphql_parser.rs @@ -14,7 +14,7 @@ use graphql_parser::{ }; use crate::{ - ast::{InputValue, Type}, + ast::{BorrowedType, InputValue, Type, TypeModifier}, schema::{ meta::{Argument, DeprecationStatus, EnumValue, Field, MetaType}, model::SchemaType, @@ -90,7 +90,7 @@ impl GraphQLParserTranslator { position: Pos::default(), description: input.description.as_deref().map(Into::into), name: From::from(input.name.as_str()), - value_type: GraphQLParserTranslator::translate_type(&input.arg_type), + value_type: GraphQLParserTranslator::translate_type((&input.arg_type).into()), default_value: input .default_value .as_ref() @@ -139,21 +139,18 @@ impl GraphQLParserTranslator { } } - fn translate_type<'a, T>(input: &'a Type>) -> ExternalType<'a, T> + fn translate_type<'a, T>(input: BorrowedType<'a>) -> ExternalType<'a, T> where T: Text<'a>, { - match input { - Type::List(x, ..) => { - ExternalType::ListType(GraphQLParserTranslator::translate_type(x).into()) + match input.modifier() { + Some(TypeModifier::NonNull) => { + ExternalType::NonNullType(Self::translate_type(input.inner_borrowed()).into()) } - Type::Named(x) => ExternalType::NamedType(From::from(x.as_ref())), - Type::NonNullList(x, ..) => ExternalType::NonNullType(Box::new( - ExternalType::ListType(Box::new(GraphQLParserTranslator::translate_type(x))), - )), - Type::NonNullNamed(x) => { - ExternalType::NonNullType(Box::new(ExternalType::NamedType(From::from(x.as_ref())))) + Some(TypeModifier::List(..)) => { + ExternalType::ListType(Self::translate_type(input.inner_borrowed()).into()) } + None => ExternalType::NamedType(input.borrowed_name().into()), } } @@ -276,7 +273,7 @@ impl GraphQLParserTranslator { name: From::from(input.name.as_str()), description: input.description.as_deref().map(Into::into), directives: generate_directives(&input.deprecation_status), - field_type: GraphQLParserTranslator::translate_type(&input.field_type), + field_type: GraphQLParserTranslator::translate_type((&input.field_type).into()), arguments, } } diff --git a/juniper/src/validation/context.rs b/juniper/src/validation/context.rs index bb10ce2b6..923a0c138 100644 --- a/juniper/src/validation/context.rs +++ b/juniper/src/validation/context.rs @@ -72,8 +72,8 @@ impl std::error::Error for RuleError {} impl<'a, S: Debug> ValidatorContext<'a, S> { #[doc(hidden)] - pub fn new(schema: &'a SchemaType, document: &Document<'a, S>) -> ValidatorContext<'a, S> { - ValidatorContext { + pub fn new(schema: &'a SchemaType, document: &Document<'a, S>) -> Self { + Self { errors: Vec::new(), schema, type_stack: Vec::new(), diff --git a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs index cd7732e73..284092a9c 100644 --- a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs +++ b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs @@ -3,7 +3,10 @@ use std::{borrow::Borrow, cell::RefCell, collections::HashMap, fmt::Debug, hash: use arcstr::ArcStr; use crate::{ - ast::{Arguments, Definition, Document, Field, Fragment, FragmentSpread, Selection, Type}, + ast::{ + Arguments, Definition, Document, Field, Fragment, FragmentSpread, Selection, Type, + TypeModifier, TypeModifier::NonNull, + }, parser::{SourcePosition, Spanning}, schema::meta::{Field as FieldType, MetaType}, validation::{ValidatorContext, Visitor}, @@ -552,24 +555,29 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> { )) } - fn is_type_conflict + PartialEq>( + fn is_type_conflict( ctx: &ValidatorContext<'a, S>, - t1: &Type, - t2: &Type, + t1: &Type, impl AsRef<[TypeModifier]>>, + t2: &Type, impl AsRef<[TypeModifier]>>, ) -> bool { - match (t1, t2) { - (Type::List(inner1, expected_size1), Type::List(inner2, expected_size2)) - | ( - Type::NonNullList(inner1, expected_size1), - Type::NonNullList(inner2, expected_size2), + match (t1.modifier(), t2.modifier()) { + (Some(TypeModifier::NonNull), Some(TypeModifier::NonNull)) => { + Self::is_type_conflict(ctx, &t1.inner(), &t2.inner()) + } + ( + Some(TypeModifier::List(expected_size1)), + Some(TypeModifier::List(expected_size2)), ) => { - if expected_size1 != expected_size2 { - return false; + if expected_size1 == expected_size2 { + Self::is_type_conflict(ctx, &t1.inner(), &t2.inner()) + } else { + false } - Self::is_type_conflict(ctx, inner1, inner2) } - (Type::Named(n1), Type::Named(n2)) - | (Type::NonNullNamed(n1), Type::NonNullNamed(n2)) => { + (None, None) => { + let n1 = t1.innermost_name(); + let n2 = t2.innermost_name(); + let ct1 = ctx.schema.concrete_type_by_name(n1); let ct2 = ctx.schema.concrete_type_by_name(n2); diff --git a/juniper/src/validation/rules/variables_in_allowed_position.rs b/juniper/src/validation/rules/variables_in_allowed_position.rs index 4ea31730a..bfa02d8e5 100644 --- a/juniper/src/validation/rules/variables_in_allowed_position.rs +++ b/juniper/src/validation/rules/variables_in_allowed_position.rs @@ -5,7 +5,7 @@ use std::{ use crate::{ Span, - ast::{Document, Fragment, FragmentSpread, Operation, Type, VariableDefinition}, + ast::{Document, Fragment, FragmentSpread, Operation, Type, TypeModifier, VariableDefinition}, parser::Spanning, schema::model::{AsDynType, DynType}, validation::{ValidatorContext, Visitor}, @@ -84,12 +84,11 @@ impl<'a, S: fmt::Debug> VariableInAllowedPosition<'a, S> { if let Some(&(var_def_name, var_def)) = var_defs.iter().find(|&&(n, _)| n.item == var_name.item) { - let expected_type = match (&var_def.default_value, &var_def.var_type.item) { - (&Some(_), Type::List(inner, expected_size)) => { - Type::NonNullList(inner.clone(), *expected_size) + let expected_type = match (&var_def.default_value, var_def.var_type.item.modifier()) { + (&Some(_), Some(TypeModifier::List(..)) | None) => { + var_def.var_type.item.clone().wrap_non_null() } - (&Some(_), Type::Named(inner)) => Type::NonNullNamed(*inner), - (_, ty) => ty.clone(), + (_, ty) => var_def.var_type.item.clone(), }; if !ctx.schema.is_subtype(&expected_type, var_type) { From d9d8f3a1495c859b136cd33a2201e3063f7b0eaf Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 21 May 2025 19:34:07 +0200 Subject: [PATCH 05/11] Resculpt `ast::Type`, vol.3 --- juniper/src/ast.rs | 9 +++ juniper/src/schema/model.rs | 80 ------------------- juniper/src/validation/context.rs | 29 ++++--- .../rules/variables_in_allowed_position.rs | 5 +- juniper/src/validation/visitor.rs | 50 +++++------- 5 files changed, 46 insertions(+), 127 deletions(-) diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index c5a28a1b7..a3e7dd2e0 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -18,6 +18,8 @@ pub enum TypeModifier { pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>; +impl Copy for BorrowedType<'_> {} + /// Type literal in a syntax tree. /// /// Carries no semantic information and might refer to types that don't exist. @@ -80,6 +82,13 @@ impl<'a> BorrowedType<'a> { }, } } + pub(crate) fn list_inner_borrowed(&self) -> Option> { + match self.modifiers.as_ref().last() { + Some(TypeModifier::NonNull) => self.inner_borrowed().list_inner_borrowed(), + Some(TypeModifier::List(..)) => Some(self.inner_borrowed()), + None => None, + } + } pub(crate) fn borrowed_name(&self) -> &'a str { self.name diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 37711e0e7..3dbc090ac 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -746,86 +746,6 @@ mod concrete_type_sort { } } -/// Allows seeing [`Type`] with different name/string representations -/// as the same type without allocation. -// TODO: Ideally this type should not exist, but the reason it currently does is that `Type` has a -// recursive design to allow arbitrary number of list wrappings. -// The list layout could instead be modelled as a modifier so that type becomes a tuple of -// (name, modifier). -// If `Type` is modelled like this it becomes easier to project it as a borrowed version of -// itself, i.e. [Type] vs [Type<&str>]. -#[derive(Clone, Copy, Debug)] -pub enum DynType<'a> { - Named(&'a str), - List(&'a dyn AsDynType, Option), - NonNullNamed(&'a str), - NonNullList(&'a dyn AsDynType, Option), -} - -impl<'a> DynType<'a> { - pub fn equals(&self, other: &DynType) -> bool { - match (self, other) { - (Self::Named(n0), DynType::Named(n1)) => n0 == n1, - (Self::List(t0, s0), DynType::List(t1, s1)) => { - t0.as_dyn_type().equals(&t1.as_dyn_type()) && s0 == s1 - } - (Self::NonNullNamed(n0), DynType::NonNullNamed(n1)) => n0 == n1, - (Self::NonNullList(t0, s0), DynType::NonNullList(t1, s1)) => { - t0.as_dyn_type().equals(&t1.as_dyn_type()) && s0 == s1 - } - _ => false, - } - } - - pub fn innermost_name(&self) -> &'a str { - match self { - Self::Named(n) | Self::NonNullNamed(n) => n, - Self::List(l, _) | Self::NonNullList(l, _) => l.as_dyn_type().innermost_name(), - } - } -} - -impl fmt::Display for DynType<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Named(n) => write!(f, "{n}"), - Self::NonNullNamed(n) => write!(f, "{n}!"), - Self::List(t, _) => write!(f, "[{}]", t.as_dyn_type()), - Self::NonNullList(t, _) => write!(f, "[{}]!", t.as_dyn_type()), - } - } -} - -/// Conversion of a [`Type`] into a [`DynType`]. -pub trait AsDynType: fmt::Debug { - /// Project this value as a [`DynType`]. - /// - /// Should not allocate memory. - fn as_dyn_type(&self) -> DynType<'_>; -} - -impl AsDynType for Type { - fn as_dyn_type(&self) -> DynType<'_> { - match self { - Self::Named(n) => DynType::Named(n.as_str()), - Self::List(t, s) => DynType::List(t.as_ref(), *s), - Self::NonNullNamed(n) => DynType::NonNullNamed(n.as_str()), - Self::NonNullList(t, s) => DynType::NonNullList(t.as_ref(), *s), - } - } -} - -impl AsDynType for Type<&str> { - fn as_dyn_type(&self) -> DynType<'_> { - match self { - Self::Named(n) => DynType::Named(n), - Self::List(t, s) => DynType::List(t.as_ref(), *s), - Self::NonNullNamed(n) => DynType::NonNullNamed(n), - Self::NonNullList(t, s) => DynType::NonNullList(t.as_ref(), *s), - } - } -} - #[cfg(test)] mod root_node_test { #[cfg(feature = "schema-language")] diff --git a/juniper/src/validation/context.rs b/juniper/src/validation/context.rs index 923a0c138..c36eccaf7 100644 --- a/juniper/src/validation/context.rs +++ b/juniper/src/validation/context.rs @@ -4,14 +4,11 @@ use std::{ }; use crate::{ - ast::{Definition, Document}, - schema::model::DynType, + ast::{BorrowedType, Definition, Document, Type, TypeModifier}, + parser::SourcePosition, + schema::{meta::MetaType, model::SchemaType}, }; -use crate::schema::{meta::MetaType, model::SchemaType}; - -use crate::parser::SourcePosition; - /// Query validation error #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] pub struct RuleError { @@ -24,9 +21,9 @@ pub struct ValidatorContext<'a, S: Debug + 'a> { pub schema: &'a SchemaType, errors: Vec, type_stack: Vec>>, - type_literal_stack: Vec>>, + type_literal_stack: Vec>>, input_type_stack: Vec>>, - input_type_literal_stack: Vec>>, + input_type_literal_stack: Vec>>, parent_type_stack: Vec>>, fragment_names: HashSet<&'a str>, } @@ -112,10 +109,12 @@ impl<'a, S: Debug> ValidatorContext<'a, S> { } #[doc(hidden)] - pub fn with_pushed_type(&mut self, t: Option>, f: F) -> R + pub fn with_pushed_type(&mut self, t: Option>>, f: F) -> R where F: FnOnce(&mut ValidatorContext<'a, S>) -> R, { + let t = t.map(Into::into); + if let Some(t) = t { self.type_stack .push(self.schema.concrete_type_by_name(t.innermost_name())); @@ -147,10 +146,16 @@ impl<'a, S: Debug> ValidatorContext<'a, S> { } #[doc(hidden)] - pub fn with_pushed_input_type(&mut self, t: Option>, f: F) -> R + pub fn with_pushed_input_type( + &mut self, + t: Option>>, + f: F, + ) -> R where F: FnOnce(&mut ValidatorContext<'a, S>) -> R, { + let t = t.map(Into::into); + if let Some(t) = t { self.input_type_stack .push(self.schema.concrete_type_by_name(t.innermost_name())); @@ -174,7 +179,7 @@ impl<'a, S: Debug> ValidatorContext<'a, S> { } #[doc(hidden)] - pub fn current_type_literal(&self) -> Option> { + pub fn current_type_literal(&self) -> Option> { match self.type_literal_stack.last() { Some(Some(t)) => Some(*t), _ => None, @@ -187,7 +192,7 @@ impl<'a, S: Debug> ValidatorContext<'a, S> { } #[doc(hidden)] - pub fn current_input_type_literal(&self) -> Option> { + pub fn current_input_type_literal(&self) -> Option> { match self.input_type_literal_stack.last() { Some(Some(t)) => Some(*t), _ => None, diff --git a/juniper/src/validation/rules/variables_in_allowed_position.rs b/juniper/src/validation/rules/variables_in_allowed_position.rs index bfa02d8e5..9358b274d 100644 --- a/juniper/src/validation/rules/variables_in_allowed_position.rs +++ b/juniper/src/validation/rules/variables_in_allowed_position.rs @@ -5,9 +5,8 @@ use std::{ use crate::{ Span, - ast::{Document, Fragment, FragmentSpread, Operation, Type, TypeModifier, VariableDefinition}, + ast::{Document, Fragment, FragmentSpread, Operation, BorrowedType, TypeModifier, VariableDefinition}, parser::Spanning, - schema::model::{AsDynType, DynType}, validation::{ValidatorContext, Visitor}, value::ScalarValue, }; @@ -29,7 +28,7 @@ pub fn factory<'a, S: fmt::Debug>() -> VariableInAllowedPosition<'a, S> { pub struct VariableInAllowedPosition<'a, S: fmt::Debug + 'a> { spreads: HashMap, HashSet<&'a str>>, - variable_usages: HashMap, Vec<(SpannedInput<'a, String>, DynType<'a>)>>, + variable_usages: HashMap, Vec<(SpannedInput<'a, String>, BorrowedType<'a>)>>, #[expect(clippy::type_complexity, reason = "readable enough")] variable_defs: HashMap, Vec<&'a (Spanning<&'a str>, VariableDefinition<'a, S>)>>, current_scope: Option>, diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs index 839bdf293..c25465662 100644 --- a/juniper/src/validation/visitor.rs +++ b/juniper/src/validation/visitor.rs @@ -2,11 +2,11 @@ use crate::{ ast::{ Arguments, Definition, Directive, Document, Field, Fragment, FragmentSpread, InlineFragment, InputValue, Operation, OperationType, Selection, VariableDefinitions, + Type, }, parser::Spanning, schema::{ meta::Argument, - model::{AsDynType, DynType}, }, validation::{ValidatorContext, Visitor, multi_visitor::MultiVisitorCons}, value::ScalarValue, @@ -32,7 +32,7 @@ where V: Visitor<'a, S>, { for def in d { - let def_type = match *def { + let def_type = match def { Definition::Fragment(Spanning { item: Fragment { @@ -40,7 +40,7 @@ where .. }, .. - }) => Some(DynType::NonNullNamed(name)), + }) => Some(Type::named(*name).wrap_non_null()), Definition::Operation(Spanning { item: Operation { @@ -48,9 +48,7 @@ where .. }, .. - }) => Some(DynType::NonNullNamed( - ctx.schema.concrete_query_type().name().unwrap(), - )), + }) => Some(ctx.schema.concrete_query_type().as_type()), Definition::Operation(Spanning { item: Operation { @@ -61,7 +59,7 @@ where }) => ctx .schema .concrete_mutation_type() - .map(|t| DynType::NonNullNamed(t.name().unwrap())), + .map(|t| t.as_type()), Definition::Operation(Spanning { item: Operation { @@ -72,10 +70,10 @@ where }) => ctx .schema .concrete_subscription_type() - .map(|t| DynType::NonNullNamed(t.name().unwrap())), + .map(|t| t.as_type()), }; - ctx.with_pushed_type(def_type, |ctx| { + ctx.with_pushed_type(def_type.as_ref(), |ctx| { enter_definition(v, ctx, def); visit_definition(v, ctx, def); exit_definition(v, ctx, def); @@ -135,7 +133,7 @@ fn visit_variable_definitions<'a, S, V>( for def in defs.item.iter() { let var_type = &def.1.var_type.item; - ctx.with_pushed_input_type(Some(var_type.as_dyn_type()), |ctx| { + ctx.with_pushed_input_type(Some(var_type), |ctx| { v.enter_variable_definition(ctx, def); if let Some(ref default_value) = def.1.default_value { @@ -196,8 +194,7 @@ fn visit_arguments<'a, S, V>( for argument in arguments.item.iter() { let arg_type = meta_args .and_then(|args| args.iter().find(|a| a.name == argument.0.item)) - .map(|a| &a.arg_type) - .map(|t| t.as_dyn_type()); + .map(|a| &a.arg_type); ctx.with_pushed_input_type(arg_type, |ctx| { v.enter_argument(ctx, argument); @@ -256,7 +253,7 @@ fn visit_field<'a, S, V>( .parent_type() .and_then(|t| t.field_by_name(field.item.name.item)); - let field_type = meta_field.map(|f| f.field_type.as_dyn_type()); + let field_type = meta_field.map(|f| &f.field_type); let field_args = meta_field.and_then(|f| f.arguments.as_ref()); ctx.with_pushed_type(field_type, |ctx| { @@ -291,7 +288,7 @@ fn visit_fragment_spread<'a, S, V>( fn visit_inline_fragment<'a, S, V>( v: &mut V, ctx: &mut ValidatorContext<'a, S>, - fragment: &'a Spanning>, + fragment: &'a Spanning>, ) where S: ScalarValue, V: Visitor<'a, S>, @@ -309,7 +306,7 @@ fn visit_inline_fragment<'a, S, V>( item: type_name, .. }) = fragment.item.type_condition { - ctx.with_pushed_type(Some(DynType::NonNullNamed(type_name)), visit_fn); + ctx.with_pushed_type(Some(&Type::<&str>::named(type_name).wrap_non_null()), visit_fn); } else { visit_fn(ctx); } @@ -325,20 +322,14 @@ fn visit_input_value<'a, S, V>( { enter_input_value(v, ctx, input_value); - match input_value.item { - InputValue::Object(ref fields) => { + match &input_value.item { + InputValue::Object(fields) => { for (key, value) in fields { let inner_type = ctx .current_input_type_literal() - .and_then(|t| match t { - DynType::NonNullNamed(name) | DynType::Named(name) => { - ctx.schema.concrete_type_by_name(name) - } - _ => None, - }) + .and_then(|t| t.name().and_then(|n| ctx.schema.concrete_type_by_name(n))) .and_then(|ct| ct.input_field_by_name(&key.item)) - .map(|f| &f.arg_type) - .map(|t| t.as_dyn_type()); + .map(|f| &f.arg_type); ctx.with_pushed_input_type(inner_type, |ctx| { v.enter_object_field(ctx, (key.as_ref(), value.as_ref())); @@ -347,13 +338,8 @@ fn visit_input_value<'a, S, V>( }) } } - InputValue::List(ref ls) => { - let inner_type = ctx.current_input_type_literal().and_then(|t| match t { - DynType::List(inner, _) | DynType::NonNullList(inner, _) => { - Some(inner.as_dyn_type()) - } - _ => None, - }); + InputValue::List(ls) => { + let inner_type = ctx.current_input_type_literal().and_then(|t| t.list_inner_borrowed()); ctx.with_pushed_input_type(inner_type, |ctx| { for value in ls { From 0d85e1a5e0c5308247939de3b5fea78edb8c9a3a Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 21 May 2025 22:33:14 +0200 Subject: [PATCH 06/11] Resculpt `ast::Type`, vol.4 (finally works!) --- juniper/src/ast.rs | 7 +++++++ juniper/src/validation/visitor.rs | 15 +++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index a3e7dd2e0..909df196b 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -72,6 +72,13 @@ where } impl<'a> BorrowedType<'a> { + pub(crate) fn non_null(name: &'a str) -> Self { + Self { + name, + modifiers: &[TypeModifier::NonNull], + } + } + pub(crate) fn inner_borrowed(&self) -> BorrowedType<'a> { let modifiers = self.modifiers.as_ref(); match modifiers.len() { diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs index c25465662..cf65f61d2 100644 --- a/juniper/src/validation/visitor.rs +++ b/juniper/src/validation/visitor.rs @@ -11,6 +11,7 @@ use crate::{ validation::{ValidatorContext, Visitor, multi_visitor::MultiVisitorCons}, value::ScalarValue, }; +use crate::ast::BorrowedType; #[doc(hidden)] pub fn visit<'a, A, B, S>( @@ -40,7 +41,7 @@ where .. }, .. - }) => Some(Type::named(*name).wrap_non_null()), + }) => Some(BorrowedType::non_null(name)), Definition::Operation(Spanning { item: Operation { @@ -48,7 +49,9 @@ where .. }, .. - }) => Some(ctx.schema.concrete_query_type().as_type()), + }) => Some(BorrowedType::non_null( + ctx.schema.concrete_query_type().name().unwrap(), + )), Definition::Operation(Spanning { item: Operation { @@ -59,7 +62,7 @@ where }) => ctx .schema .concrete_mutation_type() - .map(|t| t.as_type()), + .map(|t| BorrowedType::non_null(t.name().unwrap())), Definition::Operation(Spanning { item: Operation { @@ -70,10 +73,10 @@ where }) => ctx .schema .concrete_subscription_type() - .map(|t| t.as_type()), + .map(|t| BorrowedType::non_null(t.name().unwrap())), }; - ctx.with_pushed_type(def_type.as_ref(), |ctx| { + ctx.with_pushed_type(def_type, |ctx| { enter_definition(v, ctx, def); visit_definition(v, ctx, def); exit_definition(v, ctx, def); @@ -306,7 +309,7 @@ fn visit_inline_fragment<'a, S, V>( item: type_name, .. }) = fragment.item.type_condition { - ctx.with_pushed_type(Some(&Type::<&str>::named(type_name).wrap_non_null()), visit_fn); + ctx.with_pushed_type(Some(BorrowedType::non_null(type_name)), visit_fn); } else { visit_fn(ctx); } From 6a68740bb834fead2bddb8346564f21e41892711 Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 21 May 2025 22:44:20 +0200 Subject: [PATCH 07/11] Polishing, vol.1 --- juniper/src/ast.rs | 2 +- juniper/src/executor/mod.rs | 5 ++++- juniper/src/parser/document.rs | 6 +----- juniper/src/schema/model.rs | 2 +- juniper/src/schema/translate/graphql_parser.rs | 4 ++-- juniper/src/validation/context.rs | 4 ++-- .../rules/overlapping_fields_can_be_merged.rs | 2 +- .../rules/variables_in_allowed_position.rs | 18 +++++++++++------- juniper/src/validation/visitor.rs | 12 +++++------- 9 files changed, 28 insertions(+), 27 deletions(-) diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index 909df196b..8f3f30dfd 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -78,7 +78,7 @@ impl<'a> BorrowedType<'a> { modifiers: &[TypeModifier::NonNull], } } - + pub(crate) fn inner_borrowed(&self) -> BorrowedType<'a> { let modifiers = self.modifiers.as_ref(); match modifiers.len() { diff --git a/juniper/src/executor/mod.rs b/juniper/src/executor/mod.rs index e3955bcad..94b367f9f 100644 --- a/juniper/src/executor/mod.rs +++ b/juniper/src/executor/mod.rs @@ -1155,7 +1155,10 @@ impl Registry { if let Some(name) = T::name(info) { let validated_name = Name::new(name.clone()).unwrap(); if !self.types.contains_key(&name) { - self.insert_placeholder(validated_name.clone(), Type::named(name.clone()).wrap_non_null()); + self.insert_placeholder( + validated_name.clone(), + Type::named(name.clone()).wrap_non_null(), + ); let meta = T::meta(info, self); self.types.insert(validated_name, meta); } diff --git a/juniper/src/parser/document.rs b/juniper/src/parser/document.rs index 20aa0f17c..7e03acf4b 100644 --- a/juniper/src/parser/document.rs +++ b/juniper/src/parser/document.rs @@ -514,11 +514,7 @@ pub fn parse_type<'a>(parser: &mut Parser<'a>) -> ParseResult> { { let inner_type = parse_type(parser)?; let end_pos = parser.expect(&Token::BracketClose)?.span.end; - Spanning::start_end( - &start_span.start, - &end_pos, - inner_type.item.wrap_list(None), - ) + Spanning::start_end(&start_span.start, &end_pos, inner_type.item.wrap_list(None)) } else { parser.expect_name()?.map(Type::named) }; diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 3dbc090ac..5002b8e3b 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -7,7 +7,7 @@ use graphql_parser::schema::Document; use crate::{ GraphQLEnum, - ast::{Type, TypeModifier, TypeModifier::NonNull}, + ast::{Type, TypeModifier}, executor::{Context, Registry}, schema::meta::{Argument, InterfaceMeta, MetaType, ObjectMeta, PlaceholderMeta, UnionMeta}, types::{base::GraphQLType, name::Name}, diff --git a/juniper/src/schema/translate/graphql_parser.rs b/juniper/src/schema/translate/graphql_parser.rs index a29914ab0..cb67b329f 100644 --- a/juniper/src/schema/translate/graphql_parser.rs +++ b/juniper/src/schema/translate/graphql_parser.rs @@ -1,4 +1,4 @@ -use std::{boxed::Box, collections::BTreeMap}; +use std::collections::BTreeMap; use graphql_parser::{ Pos, @@ -14,7 +14,7 @@ use graphql_parser::{ }; use crate::{ - ast::{BorrowedType, InputValue, Type, TypeModifier}, + ast::{BorrowedType, InputValue, TypeModifier}, schema::{ meta::{Argument, DeprecationStatus, EnumValue, Field, MetaType}, model::SchemaType, diff --git a/juniper/src/validation/context.rs b/juniper/src/validation/context.rs index c36eccaf7..add815ca5 100644 --- a/juniper/src/validation/context.rs +++ b/juniper/src/validation/context.rs @@ -4,7 +4,7 @@ use std::{ }; use crate::{ - ast::{BorrowedType, Definition, Document, Type, TypeModifier}, + ast::{BorrowedType, Definition, Document}, parser::SourcePosition, schema::{meta::MetaType, model::SchemaType}, }; @@ -114,7 +114,7 @@ impl<'a, S: Debug> ValidatorContext<'a, S> { F: FnOnce(&mut ValidatorContext<'a, S>) -> R, { let t = t.map(Into::into); - + if let Some(t) = t { self.type_stack .push(self.schema.concrete_type_by_name(t.innermost_name())); diff --git a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs index 284092a9c..3aeedf888 100644 --- a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs +++ b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs @@ -5,7 +5,7 @@ use arcstr::ArcStr; use crate::{ ast::{ Arguments, Definition, Document, Field, Fragment, FragmentSpread, Selection, Type, - TypeModifier, TypeModifier::NonNull, + TypeModifier, }, parser::{SourcePosition, Spanning}, schema::meta::{Field as FieldType, MetaType}, diff --git a/juniper/src/validation/rules/variables_in_allowed_position.rs b/juniper/src/validation/rules/variables_in_allowed_position.rs index 9358b274d..e751d5ea5 100644 --- a/juniper/src/validation/rules/variables_in_allowed_position.rs +++ b/juniper/src/validation/rules/variables_in_allowed_position.rs @@ -5,7 +5,10 @@ use std::{ use crate::{ Span, - ast::{Document, Fragment, FragmentSpread, Operation, BorrowedType, TypeModifier, VariableDefinition}, + ast::{ + BorrowedType, Document, Fragment, FragmentSpread, Operation, TypeModifier, + VariableDefinition, + }, parser::Spanning, validation::{ValidatorContext, Visitor}, value::ScalarValue, @@ -83,12 +86,13 @@ impl<'a, S: fmt::Debug> VariableInAllowedPosition<'a, S> { if let Some(&(var_def_name, var_def)) = var_defs.iter().find(|&&(n, _)| n.item == var_name.item) { - let expected_type = match (&var_def.default_value, var_def.var_type.item.modifier()) { - (&Some(_), Some(TypeModifier::List(..)) | None) => { - var_def.var_type.item.clone().wrap_non_null() - } - (_, ty) => var_def.var_type.item.clone(), - }; + let expected_type = + match (&var_def.default_value, var_def.var_type.item.modifier()) { + (&Some(_), Some(TypeModifier::List(..)) | None) => { + var_def.var_type.item.clone().wrap_non_null() + } + _ => var_def.var_type.item.clone(), + }; if !ctx.schema.is_subtype(&expected_type, var_type) { ctx.report_error( diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs index cf65f61d2..f8af11359 100644 --- a/juniper/src/validation/visitor.rs +++ b/juniper/src/validation/visitor.rs @@ -1,17 +1,13 @@ use crate::{ ast::{ - Arguments, Definition, Directive, Document, Field, Fragment, FragmentSpread, + Arguments, BorrowedType, Definition, Directive, Document, Field, Fragment, FragmentSpread, InlineFragment, InputValue, Operation, OperationType, Selection, VariableDefinitions, - Type, }, parser::Spanning, - schema::{ - meta::Argument, - }, + schema::meta::Argument, validation::{ValidatorContext, Visitor, multi_visitor::MultiVisitorCons}, value::ScalarValue, }; -use crate::ast::BorrowedType; #[doc(hidden)] pub fn visit<'a, A, B, S>( @@ -342,7 +338,9 @@ fn visit_input_value<'a, S, V>( } } InputValue::List(ls) => { - let inner_type = ctx.current_input_type_literal().and_then(|t| t.list_inner_borrowed()); + let inner_type = ctx + .current_input_type_literal() + .and_then(|t| t.list_inner_borrowed()); ctx.with_pushed_input_type(inner_type, |ctx| { for value in ls { From e8c4b64dcbede7beb7234822152205b7c8bf3af5 Mon Sep 17 00:00:00 2001 From: tyranron Date: Thu, 22 May 2025 14:24:31 +0200 Subject: [PATCH 08/11] Polishing, vol.2 --- juniper/src/ast.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index 8f3f30dfd..feca68a5d 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -18,12 +18,10 @@ pub enum TypeModifier { pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>; -impl Copy for BorrowedType<'_> {} - /// Type literal in a syntax tree. /// /// Carries no semantic information and might refer to types that don't exist. -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub struct Type> { name: N, modifiers: M, @@ -125,16 +123,7 @@ impl, M> Type { where M: AsRef<[TypeModifier]>, { - if self - .modifiers - .as_ref() - .iter() - .any(|m| matches!(m, TypeModifier::List(..))) - { - None - } else { - Some(self.name.as_ref()) - } + (!self.is_list()).then(|| self.name.as_ref()) } /// Returns the innermost name of this [`Type`] by unpacking lists. From cc3e91453257c530f70745911e0d41cadd5125b8 Mon Sep 17 00:00:00 2001 From: tyranron Date: Thu, 22 May 2025 16:57:47 +0200 Subject: [PATCH 09/11] Polishing, vol.3 --- juniper/src/ast.rs | 169 ++++++++++-------- juniper/src/executor/mod.rs | 2 +- juniper/src/parser/document.rs | 2 +- juniper/src/parser/tests/value.rs | 4 +- juniper/src/schema/meta.rs | 4 +- juniper/src/schema/model.rs | 14 +- .../src/schema/translate/graphql_parser.rs | 23 ++- .../rules/overlapping_fields_can_be_merged.rs | 4 +- juniper/src/validation/visitor.rs | 2 +- 9 files changed, 121 insertions(+), 103 deletions(-) diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index feca68a5d..59c87dd31 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -2,28 +2,37 @@ use std::{borrow::Cow, fmt, hash::Hash, slice, vec}; use arcstr::ArcStr; use indexmap::IndexMap; -use smallvec::{SmallVec, smallvec}; +use smallvec::SmallVec; +#[cfg(doc)] +use self::TypeModifier::{List, NonNull}; use crate::{ executor::Variables, parser::Spanning, value::{DefaultScalarValue, ScalarValue}, }; +/// Possible modifiers in a [`Type`] literal. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum TypeModifier { + /// Non-`null` type (e.g. `!`). NonNull, + + /// List of types (e.g. `[]`). List(Option), } -pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>; - /// Type literal in a syntax tree. /// /// Carries no semantic information and might refer to types that don't exist. #[derive(Clone, Copy, Debug)] pub struct Type> { + /// Name of this [`Type`]. name: N, + + /// Modifiers of this [`Type`]. + /// + /// The first one is the innermost one. modifiers: M, } @@ -49,8 +58,8 @@ where { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.modifier() { - Some(TypeModifier::NonNull) => write!(f, "{}!", self.inner()), - Some(TypeModifier::List(..)) => write!(f, "[{}]", self.inner()), + Some(TypeModifier::NonNull) => write!(f, "{}!", self.borrow_inner()), + Some(TypeModifier::List(..)) => write!(f, "[{}]", self.borrow_inner()), None => write!(f, "{}", self.name.as_ref()), } } @@ -69,45 +78,16 @@ where } } -impl<'a> BorrowedType<'a> { - pub(crate) fn non_null(name: &'a str) -> Self { - Self { - name, - modifiers: &[TypeModifier::NonNull], - } - } - - pub(crate) fn inner_borrowed(&self) -> BorrowedType<'a> { - let modifiers = self.modifiers.as_ref(); - match modifiers.len() { - 0 => unreachable!(), - n => Type { - name: self.name, - modifiers: &modifiers[..n - 1], - }, - } - } - pub(crate) fn list_inner_borrowed(&self) -> Option> { - match self.modifiers.as_ref().last() { - Some(TypeModifier::NonNull) => self.inner_borrowed().list_inner_borrowed(), - Some(TypeModifier::List(..)) => Some(self.inner_borrowed()), - None => None, - } - } - - pub(crate) fn borrowed_name(&self) -> &'a str { - self.name - } -} - -impl, M> Type { - pub(crate) fn inner(&self) -> BorrowedType<'_> - where - M: AsRef<[TypeModifier]>, - { +impl, M: AsRef<[TypeModifier]>> Type { + /// Borrows the inner [`Type`] of this modified [`Type`], removing its topmost [`TypeModifier`]. + /// + /// # Panics + /// + /// If this [`Type`] has no [`TypeModifier`]s. + pub(crate) fn borrow_inner(&self) -> BorrowedType<'_> { let modifiers = self.modifiers.as_ref(); match modifiers.len() { - 0 => unreachable!(), + 0 => panic!("no inner `Type` available"), n => Type { name: self.name.as_ref(), modifiers: &modifiers[..n - 1], @@ -115,88 +95,125 @@ impl, M> Type { } } - /// Returns the name of this named [`Type`]. + /// Returns the name of this [`Type`]. /// - /// Only applies to named [`Type`]s. Lists will return [`None`]. + /// [`List`]s will return [`None`]. #[must_use] - pub fn name(&self) -> Option<&str> - where - M: AsRef<[TypeModifier]>, - { + pub fn name(&self) -> Option<&str> { (!self.is_list()).then(|| self.name.as_ref()) } - /// Returns the innermost name of this [`Type`] by unpacking lists. + /// Returns the innermost name of this [`Type`] by unpacking [`List`]s. /// - /// All [`Type`] literals contain exactly one named type. + /// All [`Type`] literals contain exactly one name. #[must_use] pub fn innermost_name(&self) -> &str { self.name.as_ref() } - /// Returns the [`TypeModifier`] of this [`Type`], if any. + /// Returns the topmost [`TypeModifier`] of this [`Type`], if any. #[must_use] - pub fn modifier(&self) -> Option<&TypeModifier> - where - M: AsRef<[TypeModifier]>, - { - self.modifiers.as_ref().last() + pub fn modifier(&self) -> Option<&TypeModifier> { + self.modifiers().last() } - /// Indicates whether this [`Type`] can only represent non-`null` values. + /// Returns [`TypeModifier`]s of this [`Type`], if any. + /// + /// The first one is the innermost one. #[must_use] - pub fn is_non_null(&self) -> bool - where - M: AsRef<[TypeModifier]>, - { + pub fn modifiers(&self) -> &[TypeModifier] { + self.modifiers.as_ref() + } + + /// Indicates whether this [`Type`] is [`NonNull`]. + #[must_use] + pub fn is_non_null(&self) -> bool { match self.modifiers.as_ref().last() { Some(TypeModifier::NonNull) => true, Some(TypeModifier::List(..)) | None => false, } } + /// Indicates whether this [`Type`] represents a [`List`] (either `null`able or [`NonNull`]). #[must_use] - pub fn is_list(&self) -> bool - where - M: AsRef<[TypeModifier]>, - { + pub fn is_list(&self) -> bool { match self.modifiers.as_ref().last() { - Some(TypeModifier::NonNull) => self.inner().is_non_null(), + Some(TypeModifier::NonNull) => self.borrow_inner().is_non_null(), Some(TypeModifier::List(..)) => true, None => false, } } } -impl> Type { +impl Type { + /// Creates a new `null`able [`Type`] literal from the provided `name`. + #[must_use] + pub fn nullable(name: impl Into) -> Self { + Self { + name: name.into(), + modifiers: M::default(), + } + } +} + +impl> Type { /// Wraps this [`Type`] into the provided [`TypeModifier`]. fn wrap(mut self, modifier: TypeModifier) -> Self { - self.modifiers.push(modifier); + self.modifiers.extend([modifier]); self } - pub fn wrap_list(self, size_hint: Option) -> Self { - // TODO: assert? - self.wrap(TypeModifier::List(size_hint)) + /// Wraps this [`Type`] into a [`List`] with the provided `expected_size`, if any. + #[must_use] + pub fn wrap_list(self, expected_size: Option) -> Self { + self.wrap(TypeModifier::List(expected_size)) } + /// Wraps this [`Type`] as a [`NonNull`] one. + #[must_use] pub fn wrap_non_null(self) -> Self { - // TODO: assert? self.wrap(TypeModifier::NonNull) } +} - pub fn nullable(mut self) -> Self { +impl> Type { + /// Strips this [`Type`] from [`NonNull`], returning it as a `null`able one. + pub(crate) fn into_nullable(mut self) -> Self { if self.is_non_null() { _ = self.modifiers.pop(); } self } +} - pub fn named(name: impl Into) -> Self { +/// Borrowed variant of a [`Type`] literal. +pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>; + +impl<'a> BorrowedType<'a> { + /// Creates a [`NonNull`] [`BorrowedType`] literal from the provided `name`. + pub(crate) fn non_null(name: &'a str) -> Self { Self { - name: name.into(), - modifiers: smallvec![], + name, + modifiers: &[TypeModifier::NonNull], + } + } + + /// Borrows the inner [`Type`] of this [`List`] [`Type`], if it represents one. + pub(crate) fn borrow_list_inner(&self) -> Option { + let mut out = None; + for (n, m) in self.modifiers.iter().enumerate().rev() { + match m { + TypeModifier::NonNull => {} + TypeModifier::List(..) => { + out = Some(Self { + name: self.name, + modifiers: &self.modifiers[..n], + }); + break; + } + } } + out } } diff --git a/juniper/src/executor/mod.rs b/juniper/src/executor/mod.rs index 94b367f9f..c5711694f 100644 --- a/juniper/src/executor/mod.rs +++ b/juniper/src/executor/mod.rs @@ -1157,7 +1157,7 @@ impl Registry { if !self.types.contains_key(&name) { self.insert_placeholder( validated_name.clone(), - Type::named(name.clone()).wrap_non_null(), + Type::nullable(name.clone()).wrap_non_null(), ); let meta = T::meta(info, self); self.types.insert(validated_name, meta); diff --git a/juniper/src/parser/document.rs b/juniper/src/parser/document.rs index 7e03acf4b..fdb4922f9 100644 --- a/juniper/src/parser/document.rs +++ b/juniper/src/parser/document.rs @@ -516,7 +516,7 @@ pub fn parse_type<'a>(parser: &mut Parser<'a>) -> ParseResult> { let end_pos = parser.expect(&Token::BracketClose)?.span.end; Spanning::start_end(&start_span.start, &end_pos, inner_type.item.wrap_list(None)) } else { - parser.expect_name()?.map(Type::named) + parser.expect_name()?.map(Type::nullable) }; Ok(match *parser.peek() { diff --git a/juniper/src/parser/tests/value.rs b/juniper/src/parser/tests/value.rs index 0ed7df4ce..9fa710a67 100644 --- a/juniper/src/parser/tests/value.rs +++ b/juniper/src/parser/tests/value.rs @@ -173,8 +173,8 @@ fn input_value_literals() { ), ); let fields = [ - Argument::new("key", Type::named("Int").wrap_non_null()), - Argument::new("other", Type::named("Bar").wrap_non_null()), + Argument::new("key", Type::nullable("Int").wrap_non_null()), + Argument::new("other", Type::nullable("Bar").wrap_non_null()), ]; let meta = &MetaType::InputObject(InputObjectMeta::new::("foo", &fields)); assert_eq!( diff --git a/juniper/src/schema/meta.rs b/juniper/src/schema/meta.rs index 82d15a1d1..769f37539 100644 --- a/juniper/src/schema/meta.rs +++ b/juniper/src/schema/meta.rs @@ -710,12 +710,12 @@ impl MetaType { | Self::Interface(InterfaceMeta { name, .. }) | Self::Object(ObjectMeta { name, .. }) | Self::Scalar(ScalarMeta { name, .. }) - | Self::Union(UnionMeta { name, .. }) => Type::named(name.clone()).wrap_non_null(), + | Self::Union(UnionMeta { name, .. }) => Type::nullable(name.clone()).wrap_non_null(), Self::List(ListMeta { of_type, expected_size, }) => of_type.clone().wrap_list(*expected_size).wrap_non_null(), - Self::Nullable(NullableMeta { of_type }) => of_type.clone().nullable(), + Self::Nullable(NullableMeta { of_type }) => of_type.clone().into_nullable(), Self::Placeholder(PlaceholderMeta { of_type }) => of_type.clone(), } } diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 5002b8e3b..962ed07b4 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -325,7 +325,7 @@ impl SchemaType { if let Some(name) = ty.name() { self.concrete_type_by_name(name) } else { - self.lookup_type(&ty.inner()) + self.lookup_type(&ty.borrow_inner()) } } @@ -396,9 +396,11 @@ impl SchemaType { /// Make a type. pub fn make_type(&self, ty: &Type, impl AsRef<[TypeModifier]>>) -> TypeType { match ty.modifier() { - Some(TypeModifier::NonNull) => TypeType::NonNull(Box::new(self.make_type(&ty.inner()))), + Some(TypeModifier::NonNull) => { + TypeType::NonNull(Box::new(self.make_type(&ty.borrow_inner()))) + } Some(TypeModifier::List(expected_size)) => { - TypeType::List(Box::new(self.make_type(&ty.inner())), *expected_size) + TypeType::List(Box::new(self.make_type(&ty.borrow_inner())), *expected_size) } None => self .type_by_name(ty.innermost_name()) @@ -484,13 +486,13 @@ impl SchemaType { match (super_type.modifier(), sub_type.modifier()) { (Some(NonNull), Some(NonNull)) => { - self.is_subtype(&sub_type.inner(), &super_type.inner()) + self.is_subtype(&sub_type.borrow_inner(), &super_type.borrow_inner()) } (None | Some(List(..)), Some(NonNull)) => { - self.is_subtype(&sub_type.inner(), super_type) + self.is_subtype(&sub_type.borrow_inner(), super_type) } (Some(List(..)), Some(List(..))) => { - self.is_subtype(&sub_type.inner(), &super_type.inner()) + self.is_subtype(&sub_type.borrow_inner(), &super_type.borrow_inner()) } (None, None) => { self.is_named_subtype(sub_type.innermost_name(), super_type.innermost_name()) diff --git a/juniper/src/schema/translate/graphql_parser.rs b/juniper/src/schema/translate/graphql_parser.rs index cb67b329f..b92faafc9 100644 --- a/juniper/src/schema/translate/graphql_parser.rs +++ b/juniper/src/schema/translate/graphql_parser.rs @@ -14,7 +14,7 @@ use graphql_parser::{ }; use crate::{ - ast::{BorrowedType, InputValue, TypeModifier}, + ast::{InputValue, Type, TypeModifier}, schema::{ meta::{Argument, DeprecationStatus, EnumValue, Field, MetaType}, model::SchemaType, @@ -90,7 +90,7 @@ impl GraphQLParserTranslator { position: Pos::default(), description: input.description.as_deref().map(Into::into), name: From::from(input.name.as_str()), - value_type: GraphQLParserTranslator::translate_type((&input.arg_type).into()), + value_type: GraphQLParserTranslator::translate_type(&input.arg_type), default_value: input .default_value .as_ref() @@ -139,19 +139,18 @@ impl GraphQLParserTranslator { } } - fn translate_type<'a, T>(input: BorrowedType<'a>) -> ExternalType<'a, T> + fn translate_type<'a, T>(input: &'a Type) -> ExternalType<'a, T> where T: Text<'a>, { - match input.modifier() { - Some(TypeModifier::NonNull) => { - ExternalType::NonNullType(Self::translate_type(input.inner_borrowed()).into()) - } - Some(TypeModifier::List(..)) => { - ExternalType::ListType(Self::translate_type(input.inner_borrowed()).into()) - } - None => ExternalType::NamedType(input.borrowed_name().into()), + let mut ty = ExternalType::NamedType(input.innermost_name().into()); + for m in input.modifiers() { + ty = match m { + TypeModifier::NonNull => ExternalType::NonNullType(ty.into()), + TypeModifier::List(..) => ExternalType::ListType(ty.into()), + }; } + ty } fn translate_meta<'a, S, T>(input: &'a MetaType) -> ExternalTypeDefinition<'a, T> @@ -273,7 +272,7 @@ impl GraphQLParserTranslator { name: From::from(input.name.as_str()), description: input.description.as_deref().map(Into::into), directives: generate_directives(&input.deprecation_status), - field_type: GraphQLParserTranslator::translate_type((&input.field_type).into()), + field_type: GraphQLParserTranslator::translate_type(&input.field_type), arguments, } } diff --git a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs index 3aeedf888..2849d6350 100644 --- a/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs +++ b/juniper/src/validation/rules/overlapping_fields_can_be_merged.rs @@ -562,14 +562,14 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> { ) -> bool { match (t1.modifier(), t2.modifier()) { (Some(TypeModifier::NonNull), Some(TypeModifier::NonNull)) => { - Self::is_type_conflict(ctx, &t1.inner(), &t2.inner()) + Self::is_type_conflict(ctx, &t1.borrow_inner(), &t2.borrow_inner()) } ( Some(TypeModifier::List(expected_size1)), Some(TypeModifier::List(expected_size2)), ) => { if expected_size1 == expected_size2 { - Self::is_type_conflict(ctx, &t1.inner(), &t2.inner()) + Self::is_type_conflict(ctx, &t1.borrow_inner(), &t2.borrow_inner()) } else { false } diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs index f8af11359..b9a35c1c6 100644 --- a/juniper/src/validation/visitor.rs +++ b/juniper/src/validation/visitor.rs @@ -340,7 +340,7 @@ fn visit_input_value<'a, S, V>( InputValue::List(ls) => { let inner_type = ctx .current_input_type_literal() - .and_then(|t| t.list_inner_borrowed()); + .and_then(|t| t.borrow_list_inner()); ctx.with_pushed_input_type(inner_type, |ctx| { for value in ls { From 5a3f5ee1804cbb41c86535f2f58b9ae8c3d5c5e6 Mon Sep 17 00:00:00 2001 From: tyranron Date: Thu, 22 May 2025 17:06:50 +0200 Subject: [PATCH 10/11] Polishing, vol.4 --- juniper/src/schema/model.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 962ed07b4..50328f351 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -395,17 +395,16 @@ impl SchemaType { /// Make a type. pub fn make_type(&self, ty: &Type, impl AsRef<[TypeModifier]>>) -> TypeType { - match ty.modifier() { - Some(TypeModifier::NonNull) => { - TypeType::NonNull(Box::new(self.make_type(&ty.borrow_inner()))) - } - Some(TypeModifier::List(expected_size)) => { - TypeType::List(Box::new(self.make_type(&ty.borrow_inner())), *expected_size) - } - None => self - .type_by_name(ty.innermost_name()) - .expect("Type not found in schema"), + let mut out = self + .type_by_name(ty.innermost_name()) + .expect("Type not found in schema"); + for m in ty.modifiers() { + out = match m { + TypeModifier::NonNull => TypeType::NonNull(out.into()), + TypeModifier::List(expected_size) => TypeType::List(out.into(), *expected_size), + }; } + out } /// Get a list of directives. From 484c26d0a2666f2ebb69fcaf484a01d0fd653582 Mon Sep 17 00:00:00 2001 From: tyranron Date: Thu, 22 May 2025 17:12:41 +0200 Subject: [PATCH 11/11] Mention in CHANGELOG --- juniper/CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md index d79cfba75..fa99dae72 100644 --- a/juniper/CHANGELOG.md +++ b/juniper/CHANGELOG.md @@ -35,6 +35,12 @@ All user visible changes to `juniper` crate will be documented in this file. Thi - `ast::Type`: - Removed lifetime parameters. - Made it generic over string type. + - Remade as a struct with methods: ([#1322]) + - Added `modifier()` and `modifiers()` methods returning `TypeModifier`. + - Added `is_list()` method. + - Added `wrap_list()` and `wrap_non_null() methods. + - Added `nullable()` constructor. + - Added `BorrowedType` representation. - `MetaType`: - Removed lifetime parameters. - Made `name()`, `description()` and `specified_by_url()` methods returning `ArcStr`. @@ -102,6 +108,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi [#1293]: /../../pull/1293 [#1311]: /../../pull/1311 [#1318]: /../../pull/1318 +[#1322]: /../../pull/1322 [#1323]: /../../pull/1323 [1b1fc618]: /../../commit/1b1fc61879ffdd640d741e187dc20678bf7ab295 [20609366]: /../../commit/2060936635609b0186d46d8fbd06eb30fce660e3