Avoid copying Span when evaluating validation rules (#1242)

audunhalland · web-flow · commit 8a69e14c8bbb · 2024-01-29T17:18:59.000+01:00
diff --git a/juniper/src/validation/multi_visitor.rs b/juniper/src/validation/multi_visitor.rs
@@ -6,6 +6,7 @@ use crate::{
     parser::Spanning,
     validation::{ValidatorContext, Visitor},
     value::ScalarValue,
+    Span,
 };
 
 #[doc(hidden)]
@@ -177,54 +178,62 @@ where
         self.1.exit_inline_fragment(ctx, f);
     }
 
-    fn enter_null_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: Spanning<()>) {
+    fn enter_null_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: SpannedInput<'a, ()>) {
         self.0.enter_null_value(ctx, n);
         self.1.enter_null_value(ctx, n);
     }
-    fn exit_null_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: Spanning<()>) {
+    fn exit_null_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: SpannedInput<'a, ()>) {
         self.0.exit_null_value(ctx, n);
         self.1.exit_null_value(ctx, n);
     }
 
-    fn enter_scalar_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: Spanning<&'a S>) {
+    fn enter_scalar_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: SpannedInput<'a, S>) {
         self.0.enter_scalar_value(ctx, n);
         self.1.enter_scalar_value(ctx, n);
     }
-    fn exit_scalar_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: Spanning<&'a S>) {
+    fn exit_scalar_value(&mut self, ctx: &mut ValidatorContext<'a, S>, n: SpannedInput<'a, S>) {
         self.0.exit_scalar_value(ctx, n);
         self.1.exit_scalar_value(ctx, n);
     }
 
-    fn enter_enum_value(&mut self, ctx: &mut ValidatorContext<'a, S>, s: Spanning<&'a String>) {
+    fn enter_enum_value(&mut self, ctx: &mut ValidatorContext<'a, S>, s: SpannedInput<'a, String>) {
         self.0.enter_enum_value(ctx, s);
         self.1.enter_enum_value(ctx, s);
     }
-    fn exit_enum_value(&mut self, ctx: &mut ValidatorContext<'a, S>, s: Spanning<&'a String>) {
+    fn exit_enum_value(&mut self, ctx: &mut ValidatorContext<'a, S>, s: SpannedInput<'a, String>) {
         self.0.exit_enum_value(ctx, s);
         self.1.exit_enum_value(ctx, s);
     }
 
-    fn enter_variable_value(&mut self, ctx: &mut ValidatorContext<'a, S>, s: Spanning<&'a String>) {
+    fn enter_variable_value(
+        &mut self,
+        ctx: &mut ValidatorContext<'a, S>,
+        s: SpannedInput<'a, String>,
+    ) {
         self.0.enter_variable_value(ctx, s);
         self.1.enter_variable_value(ctx, s);
     }
-    fn exit_variable_value(&mut self, ctx: &mut ValidatorContext<'a, S>, s: Spanning<&'a String>) {
+    fn exit_variable_value(
+        &mut self,
+        ctx: &mut ValidatorContext<'a, S>,
+        s: SpannedInput<'a, String>,
+    ) {
         self.0.exit_variable_value(ctx, s);
         self.1.exit_variable_value(ctx, s);
     }
 
     fn enter_list_value(
         &mut self,
         ctx: &mut ValidatorContext<'a, S>,
-        l: Spanning<&'a Vec<Spanning<InputValue<S>>>>,
+        l: SpannedInput<'a, Vec<Spanning<InputValue<S>>>>,
     ) {
         self.0.enter_list_value(ctx, l);
         self.1.enter_list_value(ctx, l);
     }
     fn exit_list_value(
         &mut self,
         ctx: &mut ValidatorContext<'a, S>,
-        l: Spanning<&'a Vec<Spanning<InputValue<S>>>>,
+        l: SpannedInput<'a, Vec<Spanning<InputValue<S>>>>,
     ) {
         self.0.exit_list_value(ctx, l);
         self.1.exit_list_value(ctx, l);
@@ -242,19 +251,20 @@ where
     fn enter_object_field(
         &mut self,
         ctx: &mut ValidatorContext<'a, S>,
-        f: &'a (Spanning<String>, Spanning<InputValue<S>>),
+        f: (SpannedInput<'a, String>, SpannedInput<'a, InputValue<S>>),
     ) {
         self.0.enter_object_field(ctx, f);
         self.1.enter_object_field(ctx, f);
     }
     fn exit_object_field(
         &mut self,
         ctx: &mut ValidatorContext<'a, S>,
-        f: &'a (Spanning<String>, Spanning<InputValue<S>>),
+        f: (SpannedInput<'a, String>, SpannedInput<'a, InputValue<S>>),
     ) {
         self.0.exit_object_field(ctx, f);
         self.1.exit_object_field(ctx, f);
     }
 }
 
-type SpannedObject<'a, S> = Spanning<&'a Vec<(Spanning<String>, Spanning<InputValue<S>>)>>;
+type SpannedInput<'a, T> = Spanning<&'a T, &'a Span>;
+type SpannedObject<'a, S> = SpannedInput<'a, Vec<(Spanning<String>, Spanning<InputValue<S>>)>>;
diff --git a/juniper/src/validation/rules/no_fragment_cycles.rs b/juniper/src/validation/rules/no_fragment_cycles.rs
@@ -5,6 +5,7 @@ use crate::{
     parser::Spanning,
     validation::{RuleError, ValidatorContext, Visitor},
     value::ScalarValue,
+    Span,
 };
 
 pub fn factory<'a>() -> NoFragmentCycles<'a> {
@@ -15,9 +16,11 @@ pub fn factory<'a>() -> NoFragmentCycles<'a> {
     }
 }
 
+type BorrowedSpanning<'a, T> = Spanning<&'a T, &'a Span>;
+
 pub struct NoFragmentCycles<'a> {
     current_fragment: Option<&'a str>,
-    spreads: HashMap<&'a str, Vec<Spanning<&'a str>>>,
+    spreads: HashMap<&'a str, Vec<BorrowedSpanning<'a, str>>>,
     fragment_order: Vec<&'a str>,
 }
 
@@ -73,16 +76,23 @@ where
             self.spreads
                 .entry(current_fragment)
                 .or_default()
-                .push(Spanning::new(spread.span, spread.item.name.item));
+                .push(BorrowedSpanning {
+                    item: spread.item.name.item,
+                    span: &spread.span,
+                });
         }
     }
 }
 
-type CycleDetectorState<'a> = (&'a str, Vec<&'a Spanning<&'a str>>, HashMap<&'a str, usize>);
+type CycleDetectorState<'a> = (
+    &'a str,
+    Vec<&'a BorrowedSpanning<'a, str>>,
+    HashMap<&'a str, usize>,
+);
 
 struct CycleDetector<'a> {
     visited: HashSet<&'a str>,
-    spreads: &'a HashMap<&'a str, Vec<Spanning<&'a str>>>,
+    spreads: &'a HashMap<&'a str, Vec<BorrowedSpanning<'a, str>>>,
     errors: Vec<RuleError>,
 }
 
@@ -103,7 +113,7 @@ impl<'a> CycleDetector<'a> {
     fn detect_from_inner(
         &mut self,
         from: &'a str,
-        path: Vec<&'a Spanning<&'a str>>,
+        path: Vec<&'a BorrowedSpanning<'a, str>>,
         mut path_indices: HashMap<&'a str, usize>,
     ) -> Vec<CycleDetectorState<'a>> {
         self.visited.insert(from);
diff --git a/juniper/src/validation/rules/no_undefined_variables.rs b/juniper/src/validation/rules/no_undefined_variables.rs
@@ -3,6 +3,7 @@ use crate::{
     parser::{SourcePosition, Spanning},
     validation::{RuleError, ValidatorContext, Visitor},
     value::ScalarValue,
+    Span,
 };
 use std::collections::{HashMap, HashSet};
 
@@ -21,9 +22,11 @@ pub fn factory<'a>() -> NoUndefinedVariables<'a> {
     }
 }
 
+type BorrowedSpanning<'a, T> = Spanning<&'a T, &'a Span>;
+
 pub struct NoUndefinedVariables<'a> {
     defined_variables: HashMap<Option<&'a str>, (SourcePosition, HashSet<&'a str>)>,
-    used_variables: HashMap<Scope<'a>, Vec<Spanning<&'a str>>>,
+    used_variables: HashMap<Scope<'a>, Vec<BorrowedSpanning<'a, str>>>,
     current_scope: Option<Scope<'a>>,
     spreads: HashMap<Scope<'a>, Vec<&'a str>>,
 }
@@ -33,7 +36,7 @@ impl<'a> NoUndefinedVariables<'a> {
         &'a self,
         scope: &Scope<'a>,
         defined: &HashSet<&'a str>,
-        unused: &mut Vec<&'a Spanning<&'a str>>,
+        unused: &mut Vec<BorrowedSpanning<'a, str>>,
         visited: &mut HashSet<Scope<'a>>,
     ) {
         let mut to_visit = Vec::new();
@@ -59,7 +62,7 @@ impl<'a> NoUndefinedVariables<'a> {
         &'a self,
         scope: &Scope<'a>,
         defined: &HashSet<&'a str>,
-        unused: &mut Vec<&'a Spanning<&'a str>>,
+        unused: &mut Vec<BorrowedSpanning<'a, str>>,
         visited: &mut HashSet<Scope<'a>>,
     ) -> Option<&'a Vec<&'a str>> {
         if visited.contains(scope) {
@@ -71,7 +74,7 @@ impl<'a> NoUndefinedVariables<'a> {
         if let Some(used_vars) = self.used_variables.get(scope) {
             for var in used_vars {
                 if !defined.contains(&var.item) {
-                    unused.push(var);
+                    unused.push(*var);
                 }
             }
         }
@@ -164,7 +167,10 @@ where
                         .item
                         .referenced_variables()
                         .iter()
-                        .map(|&var_name| Spanning::new(value.span, var_name))
+                        .map(|&var_name| BorrowedSpanning {
+                            span: &value.span,
+                            item: var_name,
+                        })
                         .collect(),
                 );
         }
diff --git a/juniper/src/validation/rules/no_unused_fragments.rs b/juniper/src/validation/rules/no_unused_fragments.rs
@@ -5,6 +5,7 @@ use crate::{
     parser::Spanning,
     validation::{ValidatorContext, Visitor},
     value::ScalarValue,
+    Span,
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -21,9 +22,11 @@ pub fn factory<'a>() -> NoUnusedFragments<'a> {
     }
 }
 
+type BorrowedSpanning<'a, T> = Spanning<&'a T, &'a Span>;
+
 pub struct NoUnusedFragments<'a> {
     spreads: HashMap<Scope<'a>, Vec<&'a str>>,
-    defined_fragments: HashSet<Spanning<&'a str>>,
+    defined_fragments: HashSet<BorrowedSpanning<'a, str>>,
     current_scope: Option<Scope<'a>>,
 }
 
@@ -100,8 +103,10 @@ where
         f: &'a Spanning<Fragment<S>>,
     ) {
         self.current_scope = Some(Scope::Fragment(f.item.name.item));
-        self.defined_fragments
-            .insert(Spanning::new(f.span, f.item.name.item));
+        self.defined_fragments.insert(BorrowedSpanning {
+            span: &f.span,
+            item: f.item.name.item,
+        });
     }
 
     fn enter_fragment_spread(
diff --git a/juniper/src/validation/rules/unique_input_field_names.rs b/juniper/src/validation/rules/unique_input_field_names.rs
@@ -5,6 +5,7 @@ use crate::{
     parser::{SourcePosition, Spanning},
     validation::{ValidatorContext, Visitor},
     value::ScalarValue,
+    Span,
 };
 
 pub struct UniqueInputFieldNames<'a> {
@@ -32,13 +33,13 @@ where
     fn enter_object_field(
         &mut self,
         ctx: &mut ValidatorContext<'a, S>,
-        (field_name, _): &'a (Spanning<String>, Spanning<InputValue<S>>),
+        (field_name, _): (SpannedInput<'a, String>, SpannedInput<InputValue<S>>),
     ) {
         if let Some(ref mut known_names) = self.known_name_stack.last_mut() {
-            match known_names.entry(&field_name.item) {
+            match known_names.entry(field_name.item) {
                 Entry::Occupied(e) => {
                     ctx.report_error(
-                        &error_message(&field_name.item),
+                        &error_message(field_name.item),
                         &[*e.get(), field_name.span.start],
                     );
                 }
@@ -50,7 +51,8 @@ where
     }
 }
 
-type SpannedObject<'a, S> = Spanning<&'a Vec<(Spanning<String>, Spanning<InputValue<S>>)>>;
+type SpannedInput<'a, T> = Spanning<&'a T, &'a Span>;
+type SpannedObject<'a, S> = SpannedInput<'a, Vec<(Spanning<String>, Spanning<InputValue<S>>)>>;
 
 fn error_message(field_name: &str) -> String {
     format!("There can only be one input field named \"{field_name}\"")
diff --git a/juniper/src/validation/rules/variables_in_allowed_position.rs b/juniper/src/validation/rules/variables_in_allowed_position.rs
@@ -9,6 +9,7 @@ use crate::{
     parser::Spanning,
     validation::{ValidatorContext, Visitor},
     value::ScalarValue,
+    Span,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -28,7 +29,7 @@ pub fn factory<'a, S: fmt::Debug>() -> VariableInAllowedPosition<'a, S> {
 
 pub struct VariableInAllowedPosition<'a, S: fmt::Debug + 'a> {
     spreads: HashMap<Scope<'a>, HashSet<&'a str>>,
-    variable_usages: HashMap<Scope<'a>, Vec<(Spanning<&'a String>, Type<'a>)>>,
+    variable_usages: HashMap<Scope<'a>, Vec<(SpannedInput<'a, String>, Type<'a>)>>,
     #[allow(clippy::type_complexity)]
     variable_defs: HashMap<Scope<'a>, Vec<&'a (Spanning<&'a str>, VariableDefinition<'a, S>)>>,
     current_scope: Option<Scope<'a>>,
@@ -160,18 +161,15 @@ where
     fn enter_variable_value(
         &mut self,
         ctx: &mut ValidatorContext<'a, S>,
-        var_name: Spanning<&'a String>,
+        var_name: SpannedInput<'a, String>,
     ) {
         if let (Some(scope), Some(input_type)) =
             (&self.current_scope, ctx.current_input_type_literal())
         {
             self.variable_usages
                 .entry(scope.clone())
                 .or_default()
-                .push((
-                    Spanning::new(var_name.span, var_name.item),
-                    input_type.clone(),
-                ));
+                .push((var_name, input_type.clone()));
         }
     }
 }
@@ -186,6 +184,8 @@ fn error_message(
     )
 }
 
+type SpannedInput<'a, T> = Spanning<&'a T, &'a Span>;
+
 #[cfg(test)]
 mod tests {
     use super::{error_message, factory};
diff --git a/juniper/src/validation/traits.rs b/juniper/src/validation/traits.rs
diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs
