Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Avoid copying Span when evaluating rules #1242

Merged
merged 3 commits into from
Jan 29, 2024
Merged

refactor: Avoid copying Span when evaluating rules #1242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50c66d0
refactor: Use Spanning<&T, &Span> in Visitor
audunhalland Jan 18, 2024
cd214e9
fix: lint
audunhalland Jan 23, 2024
59c5fa3
Minor corrections
tyranron Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions juniper/src/validation/multi_visitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::{
parser::Spanning,
validation::{ValidatorContext, Visitor},
value::ScalarValue,
Span,
};

#[doc(hidden)]
Expand Down Expand Up @@ -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);
Expand All @@ -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>>)>>;
20 changes: 15 additions & 5 deletions juniper/src/validation/rules/no_fragment_cycles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
parser::Spanning,
validation::{RuleError, ValidatorContext, Visitor},
value::ScalarValue,
Span,
};

pub fn factory<'a>() -> NoFragmentCycles<'a> {
Expand All @@ -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>,
}

Expand Down Expand Up @@ -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>,
}

Expand All @@ -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);
Expand Down
16 changes: 11 additions & 5 deletions juniper/src/validation/rules/no_undefined_variables.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{
parser::{SourcePosition, Spanning},
validation::{RuleError, ValidatorContext, Visitor},
value::ScalarValue,
Span,
};
use std::collections::{HashMap, HashSet};

Expand All @@ -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>>,
}
Expand All @@ -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();
Expand All @@ -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) {
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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(),
);
}
Expand Down
11 changes: 8 additions & 3 deletions juniper/src/validation/rules/no_unused_fragments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
parser::Spanning,
validation::{ValidatorContext, Visitor},
value::ScalarValue,
Span,
};

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Expand All @@ -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>>,
}

Expand Down Expand Up @@ -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(
Expand Down
10 changes: 6 additions & 4 deletions juniper/src/validation/rules/unique_input_field_names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
parser::{SourcePosition, Spanning},
validation::{ValidatorContext, Visitor},
value::ScalarValue,
Span,
};

pub struct UniqueInputFieldNames<'a> {
Expand Down Expand Up @@ -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],
);
}
Expand All @@ -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}\"")
Expand Down
12 changes: 6 additions & 6 deletions juniper/src/validation/rules/variables_in_allowed_position.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
parser::Spanning,
validation::{ValidatorContext, Visitor},
value::ScalarValue,
Span,
};

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
Expand All @@ -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>>,
Expand Down Expand Up @@ -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()));
}
}
}
Expand All @@ -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};
Expand Down
Loading
Loading