From 8a69e14c8bbbbd1da8bdefe3e30c60ff342ed863 Mon Sep 17 00:00:00 2001 From: Audun Halland Date: Mon, 29 Jan 2024 17:18:59 +0100 Subject: [PATCH] Avoid copying `Span` when evaluating validation rules (#1242) --- juniper/src/validation/multi_visitor.rs | 36 ++++++++++------ .../validation/rules/no_fragment_cycles.rs | 20 ++++++--- .../rules/no_undefined_variables.rs | 16 ++++--- .../validation/rules/no_unused_fragments.rs | 11 +++-- .../rules/unique_input_field_names.rs | 10 +++-- .../rules/variables_in_allowed_position.rs | 12 +++--- juniper/src/validation/traits.rs | 38 +++++++++++------ juniper/src/validation/visitor.rs | 42 +++++++++---------- 8 files changed, 115 insertions(+), 70 deletions(-) diff --git a/juniper/src/validation/multi_visitor.rs b/juniper/src/validation/multi_visitor.rs index cdefd9d4b..f6311668e 100644 --- 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,38 +178,46 @@ 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); } @@ -216,7 +225,7 @@ where fn enter_list_value( &mut self, ctx: &mut ValidatorContext<'a, S>, - l: Spanning<&'a Vec>>>, + l: SpannedInput<'a, Vec>>>, ) { self.0.enter_list_value(ctx, l); self.1.enter_list_value(ctx, l); @@ -224,7 +233,7 @@ where fn exit_list_value( &mut self, ctx: &mut ValidatorContext<'a, S>, - l: Spanning<&'a Vec>>>, + l: SpannedInput<'a, Vec>>>, ) { self.0.exit_list_value(ctx, l); self.1.exit_list_value(ctx, l); @@ -242,7 +251,7 @@ where fn enter_object_field( &mut self, ctx: &mut ValidatorContext<'a, S>, - f: &'a (Spanning, Spanning>), + f: (SpannedInput<'a, String>, SpannedInput<'a, InputValue>), ) { self.0.enter_object_field(ctx, f); self.1.enter_object_field(ctx, f); @@ -250,11 +259,12 @@ where fn exit_object_field( &mut self, ctx: &mut ValidatorContext<'a, S>, - f: &'a (Spanning, Spanning>), + f: (SpannedInput<'a, String>, SpannedInput<'a, InputValue>), ) { self.0.exit_object_field(ctx, f); self.1.exit_object_field(ctx, f); } } -type SpannedObject<'a, S> = Spanning<&'a Vec<(Spanning, Spanning>)>>; +type SpannedInput<'a, T> = Spanning<&'a T, &'a Span>; +type SpannedObject<'a, S> = SpannedInput<'a, Vec<(Spanning, Spanning>)>>; diff --git a/juniper/src/validation/rules/no_fragment_cycles.rs b/juniper/src/validation/rules/no_fragment_cycles.rs index d4ba2d3f7..965e4ef9b 100644 --- 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>>, + spreads: HashMap<&'a str, Vec>>, 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>>, + spreads: &'a HashMap<&'a str, Vec>>, errors: Vec, } @@ -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> { self.visited.insert(from); diff --git a/juniper/src/validation/rules/no_undefined_variables.rs b/juniper/src/validation/rules/no_undefined_variables.rs index 6b697ec0b..987923b6e 100644 --- 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, (SourcePosition, HashSet<&'a str>)>, - used_variables: HashMap, Vec>>, + used_variables: HashMap, Vec>>, current_scope: Option>, spreads: HashMap, 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>, visited: &mut HashSet>, ) { 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>, visited: &mut HashSet>, ) -> 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 index d728acb77..5f627f7df 100644 --- 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, Vec<&'a str>>, - defined_fragments: HashSet>, + defined_fragments: HashSet>, current_scope: Option>, } @@ -100,8 +103,10 @@ where f: &'a Spanning>, ) { 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 index a95f1d11e..83229088d 100644 --- 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, Spanning>), + (field_name, _): (SpannedInput<'a, String>, SpannedInput>), ) { 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, Spanning>)>>; +type SpannedInput<'a, T> = Spanning<&'a T, &'a Span>; +type SpannedObject<'a, S> = SpannedInput<'a, Vec<(Spanning, Spanning>)>>; 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 index 7e69469c6..1ec9b5665 100644 --- 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, HashSet<&'a str>>, - variable_usages: HashMap, Vec<(Spanning<&'a String>, Type<'a>)>>, + variable_usages: HashMap, Vec<(SpannedInput<'a, String>, Type<'a>)>>, #[allow(clippy::type_complexity)] variable_defs: HashMap, Vec<&'a (Spanning<&'a str>, VariableDefinition<'a, S>)>>, current_scope: Option>, @@ -160,7 +161,7 @@ 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()) @@ -168,10 +169,7 @@ where 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 index 5a615b110..b28ce0deb 100644 --- a/juniper/src/validation/traits.rs +++ b/juniper/src/validation/traits.rs @@ -6,6 +6,7 @@ use crate::{ parser::Spanning, validation::ValidatorContext, value::ScalarValue, + Span, }; #[doc(hidden)] @@ -103,28 +104,38 @@ where ) { } - fn enter_null_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<()>) {} - fn exit_null_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<()>) {} + fn enter_null_value(&mut self, _: &mut ValidatorContext<'a, S>, _: SpannedInput<'a, ()>) {} + fn exit_null_value(&mut self, _: &mut ValidatorContext<'a, S>, _: SpannedInput<'a, ()>) {} - fn enter_scalar_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<&'a S>) {} - fn exit_scalar_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<&'a S>) {} + fn enter_scalar_value(&mut self, _: &mut ValidatorContext<'a, S>, _: SpannedInput<'a, S>) {} + fn exit_scalar_value(&mut self, _: &mut ValidatorContext<'a, S>, _: SpannedInput<'a, S>) {} - fn enter_enum_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<&'a String>) {} - fn exit_enum_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<&'a String>) {} + fn enter_enum_value(&mut self, _: &mut ValidatorContext<'a, S>, _: SpannedInput<'a, String>) {} + fn exit_enum_value(&mut self, _: &mut ValidatorContext<'a, S>, _: SpannedInput<'a, String>) {} - fn enter_variable_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<&'a String>) {} - fn exit_variable_value(&mut self, _: &mut ValidatorContext<'a, S>, _: Spanning<&'a String>) {} + fn enter_variable_value( + &mut self, + _: &mut ValidatorContext<'a, S>, + _: SpannedInput<'a, String>, + ) { + } + fn exit_variable_value( + &mut self, + _: &mut ValidatorContext<'a, S>, + _: SpannedInput<'a, String>, + ) { + } fn enter_list_value( &mut self, _: &mut ValidatorContext<'a, S>, - _: Spanning<&'a Vec>>>, + _: SpannedInput<'a, Vec>>>, ) { } fn exit_list_value( &mut self, _: &mut ValidatorContext<'a, S>, - _: Spanning<&'a Vec>>>, + _: SpannedInput<'a, Vec>>>, ) { } @@ -134,15 +145,16 @@ where fn enter_object_field( &mut self, _: &mut ValidatorContext<'a, S>, - _: &'a (Spanning, Spanning>), + _: (SpannedInput<'a, String>, SpannedInput<'a, InputValue>), ) { } fn exit_object_field( &mut self, _: &mut ValidatorContext<'a, S>, - _: &'a (Spanning, Spanning>), + _: (SpannedInput<'a, String>, SpannedInput<'a, InputValue>), ) { } } -type SpannedObject<'a, S> = Spanning<&'a Vec<(Spanning, Spanning>)>>; +type SpannedInput<'a, T> = Spanning<&'a T, &'a Span>; +type SpannedObject<'a, S> = SpannedInput<'a, Vec<(Spanning, Spanning>)>>; diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs index 4bff7c9b6..17d20ad12 100644 --- a/juniper/src/validation/visitor.rs +++ b/juniper/src/validation/visitor.rs @@ -328,7 +328,7 @@ fn visit_input_value<'a, S, V>( match input_value.item { InputValue::Object(ref fields) => { - for field in fields { + for (key, value) in fields { let inner_type = ctx .current_input_type_literal() .and_then(|t| match *t { @@ -337,13 +337,13 @@ fn visit_input_value<'a, S, V>( } _ => None, }) - .and_then(|ct| ct.input_field_by_name(&field.0.item)) + .and_then(|ct| ct.input_field_by_name(&key.item)) .map(|f| &f.arg_type); ctx.with_pushed_input_type(inner_type, |ctx| { - v.enter_object_field(ctx, field); - visit_input_value(v, ctx, &field.1); - v.exit_object_field(ctx, field); + v.enter_object_field(ctx, (key.as_ref(), value.as_ref())); + visit_input_value(v, ctx, value); + v.exit_object_field(ctx, (key.as_ref(), value.as_ref())); }) } } @@ -377,15 +377,15 @@ fn enter_input_value<'a, S, V>( { use crate::InputValue::*; - let span = input_value.span; + let span = &input_value.span; - match input_value.item { - Null => v.enter_null_value(ctx, Spanning::new(span, ())), - Scalar(ref s) => v.enter_scalar_value(ctx, Spanning::new(span, s)), - Enum(ref s) => v.enter_enum_value(ctx, Spanning::new(span, s)), - Variable(ref s) => v.enter_variable_value(ctx, Spanning::new(span, s)), - List(ref l) => v.enter_list_value(ctx, Spanning::new(span, l)), - Object(ref o) => v.enter_object_value(ctx, Spanning::new(span, o)), + match &input_value.item { + Null => v.enter_null_value(ctx, Spanning { span, item: &() }), + Scalar(item) => v.enter_scalar_value(ctx, Spanning { span, item }), + Enum(item) => v.enter_enum_value(ctx, Spanning { span, item }), + Variable(item) => v.enter_variable_value(ctx, Spanning { span, item }), + List(item) => v.enter_list_value(ctx, Spanning { span, item }), + Object(item) => v.enter_object_value(ctx, Spanning { span, item }), } } @@ -399,14 +399,14 @@ fn exit_input_value<'a, S, V>( { use crate::InputValue::*; - let span = input_value.span; + let span = &input_value.span; - match input_value.item { - Null => v.exit_null_value(ctx, Spanning::new(span, ())), - Scalar(ref s) => v.exit_scalar_value(ctx, Spanning::new(span, s)), - Enum(ref s) => v.exit_enum_value(ctx, Spanning::new(span, s)), - Variable(ref s) => v.exit_variable_value(ctx, Spanning::new(span, s)), - List(ref l) => v.exit_list_value(ctx, Spanning::new(span, l)), - Object(ref o) => v.exit_object_value(ctx, Spanning::new(span, o)), + match &input_value.item { + Null => v.exit_null_value(ctx, Spanning { span, item: &() }), + Scalar(item) => v.exit_scalar_value(ctx, Spanning { span, item }), + Enum(item) => v.exit_enum_value(ctx, Spanning { span, item }), + Variable(item) => v.exit_variable_value(ctx, Spanning { span, item }), + List(item) => v.exit_list_value(ctx, Spanning { span, item }), + Object(item) => v.exit_object_value(ctx, Spanning { span, item }), } }