diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 23db8667..5c9db788 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -5,9 +5,11 @@ //! it into Rust types. #![forbid(unsafe_code)] +use crate::types::{Name, OperationType}; use pest::error::LineColLocation; use pest::RuleType; -use std::fmt; +use serde::{Serialize, Serializer}; +use std::fmt::{self, Display, Formatter}; pub use parse::{parse_query, parse_schema}; pub use pos::{Pos, Positioned}; @@ -18,27 +20,108 @@ mod parse; mod pos; /// Parser error. -#[derive(Debug, PartialEq)] -pub struct Error { - /// The position at which the error occurred. - pub pos: Pos, - /// The error message. - pub message: String, +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum Error { + /// A syntax error occurred. + Syntax { + /// The message of the error, nicely formatted with newlines. + message: String, + /// The start position of the error. + start: Pos, + /// The end position of the error, if present. + end: Option, + }, + /// The schema contained multiple query, mutation or subscription roots. + MultipleRoots { + /// The type of root that was duplicated. + root: OperationType, + /// The position of the schema. + schema: Pos, + /// The position of the second root. + pos: Pos, + }, + /// The schema contained no query root. + MissingQueryRoot { + /// The position of the schema. + pos: Pos, + }, + /// Multiple operations were found in a document with an anonymous one. + MultipleOperations { + /// The position of the anonymous operation. + anonymous: Pos, + /// The position of the other operation. + operation: Pos, + }, + /// An operation is defined multiple times in a document. + OperationDuplicated { + /// The name of the operation. + operation: Name, + /// The position of the first definition. + first: Pos, + /// The position of the second definition. + second: Pos, + }, + /// A fragment is defined multiple times in a document. + FragmentDuplicated { + /// The name of the fragment. + fragment: Name, + /// The position of the first definition. + first: Pos, + /// The position of the second definition. + second: Pos, + }, + /// The document does not contain any operation. + MissingOperation, } impl Error { - /// Create a new error with the given position and message. - pub fn new(message: impl Into, pos: Pos) -> Self { - Self { - pos, - message: message.into(), + /// Get an iterator over the positions of the error. + /// + /// The iterator is ordered from most important to least important position. + #[must_use] + pub fn positions(&self) -> ErrorPositions { + match self { + Self::Syntax { + start, + end: Some(end), + .. + } => ErrorPositions::new_2(*start, *end), + Self::Syntax { start, .. } => ErrorPositions::new_1(*start), + Self::MultipleRoots { schema, pos, .. } => ErrorPositions::new_2(*pos, *schema), + Self::MissingQueryRoot { pos } => ErrorPositions::new_1(*pos), + Self::MultipleOperations { + anonymous, + operation, + } => ErrorPositions::new_2(*anonymous, *operation), + Self::OperationDuplicated { first, second, .. } => { + ErrorPositions::new_2(*second, *first) + } + Self::FragmentDuplicated { first, second, .. } => { + ErrorPositions::new_2(*second, *first) + } + Self::MissingOperation => ErrorPositions::new_0(), } } } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(&self.message) +impl Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + Self::Syntax { message, .. } => f.write_str(&message), + Self::MissingQueryRoot { .. } => f.write_str("schema definition is missing query root"), + Self::MultipleRoots { root, .. } => { + write!(f, "multiple {} roots in schema definition", root) + } + Self::MultipleOperations { .. } => f.write_str("document contains multiple operations"), + Self::OperationDuplicated { operation, .. } => { + write!(f, "operation {} is defined twice", operation) + } + Self::FragmentDuplicated { fragment, .. } => { + write!(f, "fragment {} is defined twice", fragment) + } + Self::MissingOperation => f.write_str("document does not contain an operation"), + } } } @@ -46,17 +129,100 @@ impl std::error::Error for Error {} impl From> for Error { fn from(err: pest::error::Error) -> Self { - Error { - pos: { - match err.line_col { - LineColLocation::Pos((line, column)) - | LineColLocation::Span((line, column), _) => Pos { line, column }, - } - }, + let (start, end) = match err.line_col { + LineColLocation::Pos(at) => (at, None), + LineColLocation::Span(start, end) => (start, Some(end)), + }; + + Error::Syntax { message: err.to_string(), + start: Pos::from(start), + end: end.map(Pos::from), } } } /// An alias for `Result`. pub type Result = std::result::Result; + +/// An iterator over the positions inside an error. +/// +/// Constructed from the `Error::postions` function. +#[derive(Debug, Clone)] +pub struct ErrorPositions(ErrorPositionsInner); + +impl ErrorPositions { + fn new_0() -> Self { + Self(ErrorPositionsInner::None) + } + fn new_1(a: Pos) -> Self { + Self(ErrorPositionsInner::One(a)) + } + fn new_2(a: Pos, b: Pos) -> Self { + Self(ErrorPositionsInner::Two(a, b)) + } +} + +impl Iterator for ErrorPositions { + type Item = Pos; + + fn next(&mut self) -> Option { + match self.0 { + ErrorPositionsInner::Two(a, b) => { + self.0 = ErrorPositionsInner::One(b); + Some(a) + } + ErrorPositionsInner::One(a) => { + self.0 = ErrorPositionsInner::None; + Some(a) + } + ErrorPositionsInner::None => None, + } + } + + fn size_hint(&self) -> (usize, Option) { + let len = self.len(); + (len, Some(len)) + } +} + +impl DoubleEndedIterator for ErrorPositions { + fn next_back(&mut self) -> Option { + match self.0 { + ErrorPositionsInner::Two(a, b) => { + self.0 = ErrorPositionsInner::One(a); + Some(b) + } + ErrorPositionsInner::One(a) => { + self.0 = ErrorPositionsInner::None; + Some(a) + } + ErrorPositionsInner::None => None, + } + } +} + +impl std::iter::FusedIterator for ErrorPositions {} + +impl ExactSizeIterator for ErrorPositions { + fn len(&self) -> usize { + match self.0 { + ErrorPositionsInner::Two(_, _) => 2, + ErrorPositionsInner::One(_) => 1, + ErrorPositionsInner::None => 0, + } + } +} + +impl Serialize for ErrorPositions { + fn serialize(&self, serializer: S) -> std::result::Result { + serializer.collect_seq(self.clone()) + } +} + +#[derive(Debug, Clone, Copy)] +enum ErrorPositionsInner { + Two(Pos, Pos), + One(Pos), + None, +} diff --git a/parser/src/parse/executable.rs b/parser/src/parse/executable.rs index 9a262f90..7ed3f273 100644 --- a/parser/src/parse/executable.rs +++ b/parser/src/parse/executable.rs @@ -7,52 +7,133 @@ use super::*; /// Fails if the query is not a valid GraphQL document. pub fn parse_query>(input: T) -> Result { let mut pc = PositionCalculator::new(input.as_ref()); - Ok(parse_executable_document( + + let items = parse_definition_items( exactly_one(GraphQLParser::parse( Rule::executable_document, input.as_ref(), )?), &mut pc, - )?) -} + )?; -fn parse_executable_document( - pair: Pair, - pc: &mut PositionCalculator, -) -> Result { - debug_assert_eq!(pair.as_rule(), Rule::executable_document); + let mut operations = None; + let mut fragments: HashMap<_, Positioned> = HashMap::new(); + + for item in items { + match item { + DefinitionItem::Operation(item) => { + if let Some(name) = item.node.name { + let operations = operations + .get_or_insert_with(|| DocumentOperations::Multiple(HashMap::new())); + let operations = match operations { + DocumentOperations::Single(anonymous) => { + return Err(Error::MultipleOperations { + anonymous: anonymous.pos, + operation: item.pos, + }) + } + DocumentOperations::Multiple(operations) => operations, + }; + + match operations.entry(name.node) { + hash_map::Entry::Occupied(entry) => { + let (name, first) = entry.remove_entry(); + return Err(Error::OperationDuplicated { + operation: name, + first: first.pos, + second: item.pos, + }); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(Positioned::new(item.node.definition, item.pos)); + } + } + } else { + match operations { + Some(operations) => { + return Err(Error::MultipleOperations { + anonymous: item.pos, + operation: match operations { + DocumentOperations::Single(single) => single.pos, + DocumentOperations::Multiple(map) => { + map.values().next().unwrap().pos + } + }, + }); + } + None => { + operations = Some(DocumentOperations::Single(Positioned::new( + item.node.definition, + item.pos, + ))); + } + } + } + } + DefinitionItem::Fragment(item) => match fragments.entry(item.node.name.node) { + hash_map::Entry::Occupied(entry) => { + let (name, first) = entry.remove_entry(); + return Err(Error::FragmentDuplicated { + fragment: name, + first: first.pos, + second: item.pos, + }); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(Positioned::new(item.node.definition, item.pos)); + } + }, + } + } Ok(ExecutableDocument { - definitions: pair - .into_inner() - .filter(|pair| pair.as_rule() != Rule::EOI) - .map(|pair| parse_executable_definition(pair, pc)) - .collect::>()?, + operations: operations.ok_or(Error::MissingOperation)?, + fragments, }) } -fn parse_executable_definition( +fn parse_definition_items( pair: Pair, pc: &mut PositionCalculator, -) -> Result { +) -> Result> { + debug_assert_eq!(pair.as_rule(), Rule::executable_document); + + Ok(pair + .into_inner() + .filter(|pair| pair.as_rule() != Rule::EOI) + .map(|pair| parse_definition_item(pair, pc)) + .collect::>()?) +} + +enum DefinitionItem { + Operation(Positioned), + Fragment(Positioned), +} + +fn parse_definition_item(pair: Pair, pc: &mut PositionCalculator) -> Result { debug_assert_eq!(pair.as_rule(), Rule::executable_definition); let pair = exactly_one(pair.into_inner()); Ok(match pair.as_rule() { Rule::operation_definition => { - ExecutableDefinition::Operation(parse_operation_definition(pair, pc)?) + DefinitionItem::Operation(parse_operation_definition_item(pair, pc)?) } Rule::fragment_definition => { - ExecutableDefinition::Fragment(parse_fragment_definition(pair, pc)?) + DefinitionItem::Fragment(parse_fragment_definition_item(pair, pc)?) } _ => unreachable!(), }) } -fn parse_operation_definition( +struct OperationDefinitionItem { + name: Option>, + definition: OperationDefinition, +} + +fn parse_operation_definition_item( pair: Pair, pc: &mut PositionCalculator, -) -> Result> { +) -> Result> { debug_assert_eq!(pair.as_rule(), Rule::operation_definition); let pos = pc.step(&pair); @@ -60,12 +141,14 @@ fn parse_operation_definition( Ok(Positioned::new( match pair.as_rule() { Rule::named_operation_definition => parse_named_operation_definition(pair, pc)?, - Rule::selection_set => OperationDefinition { - ty: OperationType::Query, + Rule::selection_set => OperationDefinitionItem { name: None, - variable_definitions: Vec::new(), - directives: Vec::new(), - selection_set: parse_selection_set(pair, pc)?, + definition: OperationDefinition { + ty: OperationType::Query, + variable_definitions: Vec::new(), + directives: Vec::new(), + selection_set: parse_selection_set(pair, pc)?, + }, }, _ => unreachable!(), }, @@ -76,7 +159,7 @@ fn parse_operation_definition( fn parse_named_operation_definition( pair: Pair, pc: &mut PositionCalculator, -) -> Result { +) -> Result { debug_assert_eq!(pair.as_rule(), Rule::named_operation_definition); let mut pairs = pair.into_inner(); @@ -91,12 +174,14 @@ fn parse_named_operation_definition( debug_assert_eq!(pairs.next(), None); - Ok(OperationDefinition { - ty: ty.node, + Ok(OperationDefinitionItem { name, - variable_definitions: variable_definitions.unwrap_or_default(), - directives, - selection_set, + definition: OperationDefinition { + ty: ty.node, + variable_definitions: variable_definitions.unwrap_or_default(), + directives, + selection_set, + }, }) } @@ -259,10 +344,15 @@ fn parse_inline_fragment( )) } -fn parse_fragment_definition( +struct FragmentDefinitionItem { + name: Positioned, + definition: FragmentDefinition, +} + +fn parse_fragment_definition_item( pair: Pair, pc: &mut PositionCalculator, -) -> Result> { +) -> Result> { debug_assert_eq!(pair.as_rule(), Rule::fragment_definition); let pos = pc.step(&pair); @@ -276,11 +366,13 @@ fn parse_fragment_definition( debug_assert_eq!(pairs.next(), None); Ok(Positioned::new( - FragmentDefinition { + FragmentDefinitionItem { name, - type_condition, - directives, - selection_set, + definition: FragmentDefinition { + type_condition, + directives, + selection_set, + }, }, pos, )) @@ -310,6 +402,8 @@ mod tests { fn test_parser() { for entry in fs::read_dir("tests/executables").unwrap() { if let Ok(entry) = entry { + eprintln!("Parsing file {}", entry.path().display()); + GraphQLParser::parse( Rule::executable_document, &fs::read_to_string(entry.path()).unwrap(), @@ -323,6 +417,8 @@ mod tests { fn test_parser_ast() { for entry in fs::read_dir("tests/executables").unwrap() { if let Ok(entry) = entry { + eprintln!("Parsing and transforming file {}", entry.path().display()); + parse_query(fs::read_to_string(entry.path()).unwrap()).unwrap(); } } diff --git a/parser/src/parse/mod.rs b/parser/src/parse/mod.rs index dce8885b..f15a59f9 100644 --- a/parser/src/parse/mod.rs +++ b/parser/src/parse/mod.rs @@ -8,6 +8,7 @@ use crate::{Error, Result}; use pest::iterators::{Pair, Pairs}; use pest::Parser; use pest_derive::Parser; +use std::collections::hash_map::{self, HashMap}; use utils::*; mod executable; diff --git a/parser/src/parse/service.rs b/parser/src/parse/service.rs index aba93b2c..7ad49f87 100644 --- a/parser/src/parse/service.rs +++ b/parser/src/parse/service.rs @@ -73,23 +73,15 @@ fn parse_schema_definition( let name = parse_name(pairs.next().unwrap(), pc)?; match operation_type.node { - OperationType::Query => { - if query.is_some() { - return Err(operation_type.error_here("multiple query roots")); - } - query = Some(name); - } - OperationType::Mutation => { - if mutation.is_some() { - return Err(operation_type.error_here("multiple mutation roots")); - } - mutation = Some(name); - } - OperationType::Subscription => { - if subscription.is_some() { - return Err(operation_type.error_here("multiple subscription roots")); - } - subscription = Some(name); + OperationType::Query if query.is_none() => query = Some(name), + OperationType::Mutation if mutation.is_none() => mutation = Some(name), + OperationType::Subscription if subscription.is_none() => subscription = Some(name), + _ => { + return Err(Error::MultipleRoots { + root: operation_type.node, + schema: pos, + pos: operation_type.pos, + }) } } @@ -97,7 +89,7 @@ fn parse_schema_definition( } if !extend && query.is_none() { - return Err(Error::new("missing query root", pos)); + return Err(Error::MissingQueryRoot { pos }); } Ok(Positioned::new( diff --git a/parser/src/pos.rs b/parser/src/pos.rs index 23ee0056..1acff47e 100644 --- a/parser/src/pos.rs +++ b/parser/src/pos.rs @@ -1,8 +1,6 @@ -use crate::Error; use pest::iterators::Pair; use pest::RuleType; -use serde::ser::SerializeMap; -use serde::{Serialize, Serializer}; +use serde::{Deserialize, Serialize}; use std::borrow::{Borrow, BorrowMut}; use std::cmp::Ordering; use std::fmt; @@ -10,7 +8,10 @@ use std::hash::{Hash, Hasher}; use std::str::Chars; /// Original position of an element in source code. -#[derive(PartialOrd, Ord, PartialEq, Eq, Clone, Copy, Default, Hash)] +/// +/// You can serialize and deserialize it to the GraphQL `locations` format +/// ([reference](https://spec.graphql.org/June2018/#sec-Errors)). +#[derive(PartialOrd, Ord, PartialEq, Eq, Clone, Copy, Default, Hash, Serialize, Deserialize)] pub struct Pos { /// One-based line number. pub line: usize, @@ -31,15 +32,9 @@ impl fmt::Display for Pos { } } -impl Serialize for Pos { - fn serialize(&self, serializer: S) -> std::result::Result - where - S: Serializer, - { - let mut map = serializer.serialize_map(Some(2))?; - map.serialize_entry("line", &self.line)?; - map.serialize_entry("column", &self.column)?; - map.end() +impl From<(usize, usize)> for Pos { + fn from((line, column): (usize, usize)) -> Self { + Self { line, column } } } @@ -79,10 +74,6 @@ impl Positioned { pub fn map(self, f: impl FnOnce(T) -> U) -> Positioned { Positioned::new(f(self.node), self.pos) } - - pub(crate) fn error_here(&self, message: impl Into) -> Error { - Error::new(message, self.pos) - } } impl fmt::Display for Positioned { diff --git a/parser/src/types/executable.rs b/parser/src/types/executable.rs index 82e72cf1..0d0b1477 100644 --- a/parser/src/types/executable.rs +++ b/parser/src/types/executable.rs @@ -7,93 +7,89 @@ use super::*; /// [Reference](https://spec.graphql.org/June2018/#ExecutableDocument). #[derive(Debug, Clone)] pub struct ExecutableDocument { - /// The definitions of the document. - pub definitions: Vec, -} - -impl ExecutableDocument { - /// Convert the document into an [`ExecutableDocumentData`](struct.ExecutableDocumentData.html). - /// Will return `None` if there is no operation in the document. - /// - /// The `operation_name` parameter, if set, makes sure that the main operation of the document, - /// if named, will have that name. - #[must_use] - pub fn into_data(self, operation_name: Option<&str>) -> Option { - let mut operation = None; - let mut fragments = HashMap::new(); - - for definition in self.definitions { - match definition { - ExecutableDefinition::Operation(op) - if operation_name - .zip(op.node.name.as_ref()) - .map_or(false, |(required_name, op_name)| { - required_name != op_name.node - }) => {} - ExecutableDefinition::Operation(op) => { - operation.get_or_insert(op); - } - ExecutableDefinition::Fragment(fragment) => { - fragments.insert(fragment.node.name.node.clone(), fragment); - } - } - } - operation.map(|operation| ExecutableDocumentData { - operation, - fragments, - }) - } -} - -/// The data of an executable document. This is a [`ExecutableDocument`](struct.ExecutableDocument.html) with at least -/// one operation, and any number of fragments. -#[derive(Debug, Clone)] -pub struct ExecutableDocumentData { - /// The main operation of the document. - pub operation: Positioned, + /// The operations of the document. + pub operations: DocumentOperations, /// The fragments of the document. pub fragments: HashMap>, } -/// An executable definition in a query; a query, mutation, subscription or fragment definition. +/// The operations of a GraphQL document. /// -/// [Reference](https://spec.graphql.org/June2018/#ExecutableDefinition). +/// There is either one anonymous operation or many named operations. #[derive(Debug, Clone)] -pub enum ExecutableDefinition { - /// The definition of an operation. - Operation(Positioned), - /// The definition of a fragment. - Fragment(Positioned), +pub enum DocumentOperations { + /// The document contains a single anonymous operation. + Single(Positioned), + /// The document contains many named operations. + Multiple(HashMap>), } -impl ExecutableDefinition { - /// Get the position of the definition. +impl DocumentOperations { + /// Iterate over the operations of the document. #[must_use] - pub fn pos(&self) -> Pos { - match self { - Self::Operation(op) => op.pos, - Self::Fragment(frag) => frag.pos, + pub fn iter(&self) -> OperationsIter<'_> { + OperationsIter(match self { + Self::Single(op) => OperationsIterInner::Single(Some(op)), + Self::Multiple(ops) => OperationsIterInner::Multiple(ops.iter()), + }) + } +} + +// TODO: This is not implemented as I would like to later implement IntoIterator for +// DocumentOperations (not a reference) without having a breaking change. +// +//impl<'a> IntoIterator for &'a DocumentOperations { +// type Item = &'a Positioned; +// type IntoIter = OperationsIter<'a>; +// +// fn into_iter(self) -> Self::IntoIter { +// self.iter() +// } +//} + +/// An iterator over the operations of a document. +#[derive(Debug, Clone)] +pub struct OperationsIter<'a>(OperationsIterInner<'a>); + +impl<'a> Iterator for OperationsIter<'a> { + type Item = (Option<&'a Name>, &'a Positioned); + + fn next(&mut self) -> Option { + match &mut self.0 { + OperationsIterInner::Single(op) => op.take().map(|op| (None, op)), + OperationsIterInner::Multiple(iter) => iter.next().map(|(name, op)| (Some(name), op)), } } - /// Get a reference to the directives of the definition. - #[must_use] - pub fn directives(&self) -> &Vec> { - match self { - Self::Operation(op) => &op.node.directives, - Self::Fragment(frag) => &frag.node.directives, - } - } - /// Get a mutable reference to the directives of the definition. - #[must_use] - pub fn directives_mut(&mut self) -> &mut Vec> { - match self { - Self::Operation(op) => &mut op.node.directives, - Self::Fragment(frag) => &mut frag.node.directives, + fn size_hint(&self) -> (usize, Option) { + let size = self.len(); + (size, Some(size)) + } +} + +impl<'a> std::iter::FusedIterator for OperationsIter<'a> {} + +impl<'a> ExactSizeIterator for OperationsIter<'a> { + fn len(&self) -> usize { + match &self.0 { + OperationsIterInner::Single(opt) => { + if opt.is_some() { + 1 + } else { + 0 + } + } + OperationsIterInner::Multiple(iter) => iter.len(), } } } +#[derive(Debug, Clone)] +enum OperationsIterInner<'a> { + Single(Option<&'a Positioned>), + Multiple(hash_map::Iter<'a, Name, Positioned>), +} + /// A GraphQL operation, such as `mutation($content:String!) { makePost(content: $content) { id } }`. /// /// [Reference](https://spec.graphql.org/June2018/#OperationDefinition). @@ -101,8 +97,6 @@ impl ExecutableDefinition { pub struct OperationDefinition { /// The type of operation. pub ty: OperationType, - /// The name of the operation. - pub name: Option>, /// The variable definitions. pub variable_definitions: Vec>, /// The operation's directives. @@ -249,8 +243,6 @@ pub struct InlineFragment { /// [Reference](https://spec.graphql.org/June2018/#FragmentDefinition). #[derive(Debug, Clone)] pub struct FragmentDefinition { - /// The name of the fragment. Any name is allowed except `on`. - pub name: Positioned, /// The type this fragment operates on. pub type_condition: Positioned, /// Directives in the fragment. diff --git a/parser/src/types/mod.rs b/parser/src/types/mod.rs index b7df041a..f561697a 100644 --- a/parser/src/types/mod.rs +++ b/parser/src/types/mod.rs @@ -6,12 +6,12 @@ //! //! This follows the [June 2018 edition of the GraphQL spec](https://spec.graphql.org/June2018/). -use crate::pos::{Pos, Positioned}; +use crate::pos::Positioned; use serde::de::{Deserializer, Error as _, Unexpected}; use serde::ser::{Error as _, Serializer}; use serde::{Deserialize, Serialize}; use std::borrow::Borrow; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{hash_map, BTreeMap, HashMap}; use std::convert::{TryFrom, TryInto}; use std::fmt::{self, Display, Formatter, Write}; use std::fs::File; diff --git a/parser/tests/executables/fragment.graphql b/parser/tests/executables/fragment.graphql index 724403bc..84ee95b3 100644 --- a/parser/tests/executables/fragment.graphql +++ b/parser/tests/executables/fragment.graphql @@ -2,3 +2,4 @@ fragment frag on Friend { __typename node } +{ __typename } diff --git a/parser/tests/executables/kitchen-sink.graphql b/parser/tests/executables/kitchen-sink.graphql index ff4a05c4..19d06b0d 100644 --- a/parser/tests/executables/kitchen-sink.graphql +++ b/parser/tests/executables/kitchen-sink.graphql @@ -49,7 +49,7 @@ fragment frag on Friend { foo(size: $size, bar: $b, obj: {key: "value"}) } -{ +query otherQuery { unnamed(truthy: true, falsey: false, nullish: null), query } diff --git a/parser/tests/executables/kitchen-sink_canonical.graphql b/parser/tests/executables/kitchen-sink_canonical.graphql index bad98b71..acb4e36a 100644 --- a/parser/tests/executables/kitchen-sink_canonical.graphql +++ b/parser/tests/executables/kitchen-sink_canonical.graphql @@ -44,7 +44,7 @@ fragment frag on Friend { foo(size: $size, bar: $b, obj: {key: "value"}) } -{ +query otherQuery { unnamed(truthy: true, falsey: false, nullish: null) query } diff --git a/src/context.rs b/src/context.rs index b3f8fa3e..ebeaa9ef 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,7 +1,8 @@ use crate::base::Type; use crate::extensions::Extensions; use crate::parser::types::{ - Directive, ExecutableDocumentData, Field, Name, SelectionSet, Value as InputValue, + Directive, Field, FragmentDefinition, Name, OperationDefinition, SelectionSet, + Value as InputValue, }; use crate::schema::SchemaEnv; use crate::{FieldResult, InputValueType, Lookahead, Pos, Positioned, QueryError, Result, Value}; @@ -9,7 +10,7 @@ use fnv::FnvHashMap; use serde::ser::{SerializeSeq, Serializer}; use serde::{Deserialize, Serialize}; use std::any::{Any, TypeId}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::convert::TryFrom; use std::fmt::{self, Debug, Display, Formatter}; use std::ops::Deref; @@ -242,7 +243,8 @@ pub struct ContextBase<'a, T> { pub struct QueryEnvInner { pub extensions: spin::Mutex, pub variables: Variables, - pub document: ExecutableDocumentData, + pub operation: Positioned, + pub fragments: HashMap>, pub ctx_data: Arc, } @@ -263,13 +265,15 @@ impl QueryEnv { pub fn new( extensions: spin::Mutex, variables: Variables, - document: ExecutableDocumentData, + operation: Positioned, + fragments: HashMap>, ctx_data: Arc, ) -> QueryEnv { QueryEnv(Arc::new(QueryEnvInner { extensions, variables, - document, + operation, + fragments, ctx_data, })) } @@ -372,7 +376,6 @@ impl<'a, T> ContextBase<'a, T> { fn var_value(&self, name: &str, pos: Pos) -> Result { self.query_env - .document .operation .node .variable_definitions @@ -519,6 +522,6 @@ impl<'a> ContextBase<'a, &'a Positioned> { /// } /// ``` pub fn look_ahead(&self) -> Lookahead { - Lookahead::new(&self.query_env.document, &self.item.node) + Lookahead::new(&self.query_env.fragments, &self.item.node) } } diff --git a/src/error.rs b/src/error.rs index 51bdf211..e42dacf7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -192,9 +192,9 @@ pub enum QueryError { object: String, }, - /// An operation was missing from the query. - #[error("Missing operation")] - MissingOperation, + /// `operation_name` in the request was required but not provided. + #[error("Operation name required in request")] + RequiredOperationName, /// The operation name was unknown. #[error("Unknown operation named \"{name}\"")] diff --git a/src/extensions/logger.rs b/src/extensions/logger.rs index eab4e180..bda39786 100644 --- a/src/extensions/logger.rs +++ b/src/extensions/logger.rs @@ -1,5 +1,5 @@ use crate::extensions::{Extension, ResolveInfo}; -use crate::parser::types::{ExecutableDefinition, ExecutableDocument, OperationType, Selection}; +use crate::parser::types::{ExecutableDocument, OperationType, Selection}; use crate::{Error, Variables}; use itertools::Itertools; use log::{error, info, trace}; @@ -34,13 +34,10 @@ impl Extension for Logger { fn parse_end(&mut self, document: &ExecutableDocument) { let is_schema = document - .definitions + .operations .iter() - .filter_map(|definition| match definition { - ExecutableDefinition::Operation(operation) if operation.node.ty == OperationType::Query => Some(operation), - _ => None, - }) - .any(|operation| operation.node.selection_set.node.items.iter().any(|selection| matches!(&selection.node, Selection::Field(field) if field.node.name.node == "__schema"))); + .filter(|(_, operation)| operation.node.ty == OperationType::Query) + .any(|(_, operation)| operation.node.selection_set.node.items.iter().any(|selection| matches!(&selection.node, Selection::Field(field) if field.node.name.node == "__schema"))); if is_schema { self.enabled = false; @@ -67,7 +64,19 @@ impl Extension for Logger { fn error(&mut self, err: &Error) { match err { Error::Parse(err) => { - error!(target: "async-graphql", "[ParseError] id: \"{}\", pos: [{}:{}], query: \"{}\", variables: {}, {}", self.id, err.pos.line, err.pos.column, self.query, self.variables, err) + error!( + target: "async-graphql", "[ParseError] id: \"{}\", {}query: \"{}\", variables: {}, {}", + self.id, + if let Some(pos) = err.positions().next() { + // TODO: Make this more efficient + format!("pos: [{}:{}], ", pos.line, pos.column) + } else { + String::new() + }, + self.query, + self.variables, + err + ) } Error::Query { pos, path, err } => { if let Some(path) = path { diff --git a/src/look_ahead.rs b/src/look_ahead.rs index 70380f71..532e2e80 100644 --- a/src/look_ahead.rs +++ b/src/look_ahead.rs @@ -1,15 +1,20 @@ -use crate::parser::types::{ExecutableDocumentData, Field, Selection, SelectionSet}; +use crate::parser::types::{Field, FragmentDefinition, Name, Selection, SelectionSet}; +use crate::Positioned; +use std::collections::HashMap; /// A selection performed by a query. pub struct Lookahead<'a> { - document: &'a ExecutableDocumentData, + fragments: &'a HashMap>, field: Option<&'a Field>, } impl<'a> Lookahead<'a> { - pub(crate) fn new(document: &'a ExecutableDocumentData, field: &'a Field) -> Self { + pub(crate) fn new( + fragments: &'a HashMap>, + field: &'a Field, + ) -> Self { Self { - document, + fragments, field: Some(field), } } @@ -21,10 +26,10 @@ impl<'a> Lookahead<'a> { /// represents `{ b }`. pub fn field(&self, name: &str) -> Self { Self { - document: self.document, + fragments: self.fragments, field: self .field - .and_then(|field| find(self.document, &field.selection_set.node, name)), + .and_then(|field| find(self.fragments, &field.selection_set.node, name)), } } @@ -36,7 +41,7 @@ impl<'a> Lookahead<'a> { } fn find<'a>( - document: &'a ExecutableDocumentData, + fragments: &'a HashMap>, selection_set: &'a SelectionSet, name: &str, ) -> Option<&'a Field> { @@ -52,12 +57,11 @@ fn find<'a>( } } Selection::InlineFragment(fragment) => { - find(document, &fragment.node.selection_set.node, name) + find(fragments, &fragment.node.selection_set.node, name) } - Selection::FragmentSpread(spread) => document - .fragments + Selection::FragmentSpread(spread) => fragments .get(&spread.node.fragment_name.node) - .and_then(|fragment| find(document, &fragment.node.selection_set.node, name)), + .and_then(|fragment| find(fragments, &fragment.node.selection_set.node, name)), }) } diff --git a/src/resolver_utils/object.rs b/src/resolver_utils/object.rs index 8980c90b..8a322491 100644 --- a/src/resolver_utils/object.rs +++ b/src/resolver_utils/object.rs @@ -220,11 +220,8 @@ impl<'a> Fields<'a> { let (type_condition, selection_set) = match selection { Selection::Field(_) => unreachable!(), Selection::FragmentSpread(spread) => { - let fragment = ctx - .query_env - .document - .fragments - .get(&spread.node.fragment_name.node); + let fragment = + ctx.query_env.fragments.get(&spread.node.fragment_name.node); let fragment = match fragment { Some(fragment) => fragment, None => { diff --git a/src/schema.rs b/src/schema.rs index 4762a835..8a46221d 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -2,21 +2,23 @@ use crate::context::{Data, ResolveId}; use crate::extensions::{BoxExtension, ErrorLogger, Extension, Extensions}; use crate::model::__DirectiveLocation; use crate::parser::parse_query; -use crate::parser::types::OperationType; +use crate::parser::types::{ + DocumentOperations, FragmentDefinition, Name, OperationDefinition, OperationType, +}; use crate::registry::{MetaDirective, MetaInputValue, Registry}; use crate::resolver_utils::{resolve_object, resolve_object_serial, ObjectType}; use crate::subscription::collect_subscription_streams; use crate::types::QueryRoot; use crate::validation::{check_rules, CheckResult, ValidationMode}; use crate::{ - BatchRequest, BatchResponse, CacheControl, ContextBase, Error, Pos, QueryEnv, QueryError, - Request, Response, Result, SubscriptionType, Type, Variables, ID, + BatchRequest, BatchResponse, CacheControl, ContextBase, Error, Pos, Positioned, QueryEnv, + QueryError, Request, Response, Result, SubscriptionType, Type, Variables, ID, }; -use async_graphql_parser::types::ExecutableDocumentData; use futures::stream::{self, Stream, StreamExt}; use indexmap::map::IndexMap; use itertools::Itertools; use std::any::Any; +use std::collections::HashMap; use std::ops::Deref; use std::sync::atomic::AtomicUsize; use std::sync::Arc; @@ -302,11 +304,14 @@ where Self::build(query, mutation, subscription).finish() } + // TODO: Remove the allow + #[allow(clippy::type_complexity)] fn prepare_request( &self, request: &Request, ) -> Result<( - ExecutableDocumentData, + Positioned, + HashMap>, CacheControl, spin::Mutex, )> { @@ -356,54 +361,66 @@ where } } - let document = match document.into_data(request.operation_name.as_deref()) { - Some(document) => document, - None => { - let err = if let Some(operation_name) = &request.operation_name { - Error::Query { - pos: Pos::default(), - path: None, - err: QueryError::UnknownOperationNamed { - name: operation_name.to_string(), - }, - } - } else { - Error::Query { - pos: Pos::default(), - path: None, - err: QueryError::MissingOperation, - } - }; + let operation = if let Some(operation_name) = &request.operation_name { + match document.operations { + DocumentOperations::Single(_) => None, + DocumentOperations::Multiple(mut operations) => { + operations.remove(operation_name.as_str()) + } + } + .ok_or_else(|| QueryError::UnknownOperationNamed { + name: operation_name.clone(), + }) + } else { + match document.operations { + DocumentOperations::Single(operation) => Ok(operation), + DocumentOperations::Multiple(map) if map.len() == 1 => { + Ok(map.into_iter().next().unwrap().1) + } + DocumentOperations::Multiple(_) => Err(QueryError::RequiredOperationName), + } + }; + let operation = match operation { + Ok(operation) => operation, + Err(e) => { + let err = e.into_error(Pos::default()); extensions.lock().error(&err); return Err(err); } }; - Ok((document, cache_control, extensions)) + Ok((operation, document.fragments, cache_control, extensions)) } async fn execute_once( &self, - document: ExecutableDocumentData, + operation: Positioned, + fragments: HashMap>, extensions: spin::Mutex, variables: Variables, ctx_data: Data, ) -> Response { // execute let inc_resolve_id = AtomicUsize::default(); - let env = QueryEnv::new(extensions, variables, document, Arc::new(ctx_data)); + let env = QueryEnv::new( + extensions, + variables, + operation, + fragments, + Arc::new(ctx_data), + ); let ctx = ContextBase { path_node: None, resolve_id: ResolveId::root(), inc_resolve_id: &inc_resolve_id, - item: &env.document.operation.node.selection_set, + item: &env.operation.node.selection_set, schema_env: &self.env, query_env: &env, }; env.extensions.lock().execution_start(); - let data = match &env.document.operation.node.ty { + let data = match &env.operation.node.ty { OperationType::Query => resolve_object(&ctx, &self.query).await, OperationType::Mutation => resolve_object_serial(&ctx, &self.mutation).await, OperationType::Subscription => { @@ -426,8 +443,14 @@ where pub async fn execute(&self, request: impl Into) -> Response { let request = request.into(); match self.prepare_request(&request) { - Ok((document, cache_control, extensions)) => self - .execute_once(document, extensions, request.variables, request.data) + Ok((operation, fragments, cache_control, extensions)) => self + .execute_once( + operation, + fragments, + extensions, + request.variables, + request.data, + ) .await .cache_control(cache_control), Err(e) => Response::from_error(e), @@ -456,7 +479,7 @@ where async_stream::stream! { let request = request.into(); - let (document, cache_control, extensions) = match schema.prepare_request(&request) { + let (operation, fragments, cache_control, extensions) = match schema.prepare_request(&request) { Ok(res) => res, Err(err) => { yield Response::from(err); @@ -464,9 +487,9 @@ where } }; - if document.operation.node.ty != OperationType::Subscription { + if operation.node.ty != OperationType::Subscription { yield schema - .execute_once(document, extensions, request.variables, request.data) + .execute_once(operation, fragments, extensions, request.variables, request.data) .await .cache_control(cache_control); return; @@ -476,14 +499,15 @@ where let env = QueryEnv::new( extensions, request.variables, - document, + operation, + fragments, ctx_data, ); let ctx = env.create_context( &schema.env, None, - &env.document.operation.node.selection_set, + &env.operation.node.selection_set, &resolve_id, ); diff --git a/src/serialize_resp.rs b/src/serialize_resp.rs index 7cd3bb58..9a568d9f 100644 --- a/src/serialize_resp.rs +++ b/src/serialize_resp.rs @@ -33,8 +33,8 @@ impl Serialize for Error { Error::Parse(err) => { let mut seq = serializer.serialize_seq(Some(1))?; seq.serialize_element(&serde_json::json! ({ - "message": err.message, - "locations": [{"line": err.pos.line, "column": err.pos.column}] + "message": err.to_string(), + "locations": err.positions(), }))?; seq.end() } diff --git a/src/subscription.rs b/src/subscription.rs index 11aaa782..8e7813e2 100644 --- a/src/subscription.rs +++ b/src/subscription.rs @@ -41,7 +41,6 @@ pub(crate) fn collect_subscription_streams<'a, T: SubscriptionType + Send + Sync Selection::FragmentSpread(fragment_spread) => { if let Some(fragment) = ctx .query_env - .document .fragments .get(&fragment_spread.node.fragment_name.node) { diff --git a/src/validation/mod.rs b/src/validation/mod.rs index 17a3ee07..279ef13e 100644 --- a/src/validation/mod.rs +++ b/src/validation/mod.rs @@ -51,13 +51,10 @@ pub fn check_rules( .with(rules::NoFragmentCycles::default()) .with(rules::KnownFragmentNames) .with(rules::KnownTypeNames) - .with(rules::LoneAnonymousOperation::default()) .with(rules::NoUndefinedVariables::default()) .with(rules::NoUnusedFragments::default()) .with(rules::NoUnusedVariables::default()) .with(rules::UniqueArgumentNames::default()) - .with(rules::UniqueFragmentNames::default()) - .with(rules::UniqueOperationNames::default()) .with(rules::UniqueVariableNames::default()) .with(rules::VariablesAreInputTypes) .with(rules::VariableInAllowedPosition::default()) diff --git a/src/validation/rules/fields_on_correct_type.rs b/src/validation/rules/fields_on_correct_type.rs index c0b73fb5..3496a6e4 100644 --- a/src/validation/rules/fields_on_correct_type.rs +++ b/src/validation/rules/fields_on_correct_type.rs @@ -68,6 +68,7 @@ mod tests { __typename name } + { __typename } "#, ); } @@ -81,6 +82,7 @@ mod tests { tn : __typename otherName : name } + { __typename } "#, ); } @@ -94,6 +96,7 @@ mod tests { __typename name } + { __typename } "#, ); } @@ -106,6 +109,7 @@ mod tests { fragment interfaceFieldSelection on Pet { otherName : name } + { __typename } "#, ); } @@ -118,6 +122,7 @@ mod tests { fragment lyingAliasSelection on Dog { name : nickname } + { __typename } "#, ); } @@ -130,6 +135,7 @@ mod tests { fragment unknownSelection on UnknownType { unknownField } + { __typename } "#, ); } @@ -146,6 +152,7 @@ mod tests { } } } + { __typename } "#, ); } @@ -158,6 +165,7 @@ mod tests { fragment fieldNotDefined on Dog { meowVolume } + { __typename } "#, ); } @@ -172,6 +180,7 @@ mod tests { deeper_unknown_field } } + { __typename } "#, ); } @@ -186,6 +195,7 @@ mod tests { unknown_field } } + { __typename } "#, ); } @@ -200,6 +210,7 @@ mod tests { meowVolume } } + { __typename } "#, ); } @@ -212,6 +223,7 @@ mod tests { fragment aliasedFieldTargetNotDefined on Dog { volume : mooVolume } + { __typename } "#, ); } @@ -224,6 +236,7 @@ mod tests { fragment aliasedLyingFieldTargetNotDefined on Dog { barkVolume : kawVolume } + { __typename } "#, ); } @@ -236,6 +249,7 @@ mod tests { fragment notDefinedOnInterface on Pet { tailLength } + { __typename } "#, ); } @@ -248,6 +262,7 @@ mod tests { fragment definedOnImplementorsButNotInterface on Pet { nickname } + { __typename } "#, ); } @@ -260,6 +275,7 @@ mod tests { fragment definedOnImplementorsButNotInterface on Pet { __typename } + { __typename } "#, ); } @@ -272,6 +288,7 @@ mod tests { fragment definedOnImplementorsQueriedOnUnion on CatOrDog { name } + { __typename } "#, ); } @@ -290,6 +307,7 @@ mod tests { name } } + { __typename } "#, ); } @@ -307,6 +325,7 @@ mod tests { name } } + { __typename } "#, ); } diff --git a/src/validation/rules/fragments_on_composite_types.rs b/src/validation/rules/fragments_on_composite_types.rs index 387d5379..ec4985e4 100644 --- a/src/validation/rules/fragments_on_composite_types.rs +++ b/src/validation/rules/fragments_on_composite_types.rs @@ -1,4 +1,4 @@ -use crate::parser::types::{FragmentDefinition, InlineFragment, TypeCondition}; +use crate::parser::types::{FragmentDefinition, InlineFragment, Name}; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::Positioned; @@ -9,16 +9,16 @@ impl<'a> Visitor<'a> for FragmentsOnCompositeTypes { fn enter_fragment_definition( &mut self, ctx: &mut VisitorContext<'a>, + name: &'a Name, fragment_definition: &'a Positioned, ) { if let Some(current_type) = ctx.current_type() { if !current_type.is_composite() { - let TypeCondition { on: name } = &fragment_definition.node.type_condition.node; ctx.report_error( vec![fragment_definition.pos], format!( "Fragment \"{}\" cannot condition non composite type \"{}\"", - fragment_definition.node.name, name + name, fragment_definition.node.type_condition.node.on.node, ), ); } @@ -60,6 +60,7 @@ mod tests { fragment validFragment on Dog { barks } + { __typename } "#, ); } @@ -72,6 +73,7 @@ mod tests { fragment validFragment on Pet { name } + { __typename } "#, ); } @@ -86,6 +88,7 @@ mod tests { barks } } + { __typename } "#, ); } @@ -100,6 +103,7 @@ mod tests { name } } + { __typename } "#, ); } @@ -112,6 +116,7 @@ mod tests { fragment validFragment on CatOrDog { __typename } + { __typename } "#, ); } @@ -124,6 +129,7 @@ mod tests { fragment scalarFragment on Boolean { bad } + { __typename } "#, ); } @@ -136,6 +142,7 @@ mod tests { fragment scalarFragment on FurColor { bad } + { __typename } "#, ); } @@ -148,6 +155,7 @@ mod tests { fragment inputFragment on ComplexInput { stringField } + { __typename } "#, ); } @@ -162,6 +170,7 @@ mod tests { barks } } + { __typename } "#, ); } diff --git a/src/validation/rules/known_argument_names.rs b/src/validation/rules/known_argument_names.rs index f9207883..39349b7d 100644 --- a/src/validation/rules/known_argument_names.rs +++ b/src/validation/rules/known_argument_names.rs @@ -128,6 +128,7 @@ mod tests { fragment argOnRequiredArg on Dog { doesKnowCommand(dogCommand: SIT) } + { __typename } "#, ); } @@ -140,6 +141,7 @@ mod tests { fragment multipleArgs on ComplicatedArgs { multipleReqs(req1: 1, req2: 2) } + { __typename } "#, ); } @@ -152,6 +154,7 @@ mod tests { fragment argOnUnknownField on Dog { unknownField(unknownArg: SIT) } + { __typename } "#, ); } @@ -164,6 +167,7 @@ mod tests { fragment multipleArgsReverseOrder on ComplicatedArgs { multipleReqs(req2: 2, req1: 1) } + { __typename } "#, ); } @@ -176,6 +180,7 @@ mod tests { fragment noArgOnOptionalArg on Dog { isHousetrained } + { __typename } "#, ); } @@ -233,6 +238,7 @@ mod tests { fragment invalidArgName on Dog { doesKnowCommand(unknown: true) } + { __typename } "#, ); } @@ -245,6 +251,7 @@ mod tests { fragment oneGoodArgOneInvalidArg on Dog { doesKnowCommand(whoknows: 1, dogCommand: SIT, unknown: true) } + { __typename } "#, ); } diff --git a/src/validation/rules/known_directives.rs b/src/validation/rules/known_directives.rs index ee83662f..8bd30a34 100644 --- a/src/validation/rules/known_directives.rs +++ b/src/validation/rules/known_directives.rs @@ -1,7 +1,7 @@ use crate::model::__DirectiveLocation; use crate::parser::types::{ - Directive, Field, FragmentDefinition, FragmentSpread, InlineFragment, OperationDefinition, - OperationType, + Directive, Field, FragmentDefinition, FragmentSpread, InlineFragment, Name, + OperationDefinition, OperationType, }; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::Positioned; @@ -15,6 +15,7 @@ impl<'a> Visitor<'a> for KnownDirectives { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: Option<&'a Name>, operation_definition: &'a Positioned, ) { self.location_stack @@ -28,6 +29,7 @@ impl<'a> Visitor<'a> for KnownDirectives { fn exit_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: Option<&'a Name>, _operation_definition: &'a Positioned, ) { self.location_stack.pop(); @@ -36,6 +38,7 @@ impl<'a> Visitor<'a> for KnownDirectives { fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: &'a Name, _fragment_definition: &'a Positioned, ) { self.location_stack @@ -45,6 +48,7 @@ impl<'a> Visitor<'a> for KnownDirectives { fn exit_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: &'a Name, _fragment_definition: &'a Positioned, ) { self.location_stack.pop(); diff --git a/src/validation/rules/known_type_names.rs b/src/validation/rules/known_type_names.rs index d47bde6e..4d4d1d53 100644 --- a/src/validation/rules/known_type_names.rs +++ b/src/validation/rules/known_type_names.rs @@ -1,4 +1,6 @@ -use crate::parser::types::{FragmentDefinition, InlineFragment, TypeCondition, VariableDefinition}; +use crate::parser::types::{ + FragmentDefinition, InlineFragment, Name, TypeCondition, VariableDefinition, +}; use crate::registry::MetaTypeName; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::{Pos, Positioned}; @@ -10,6 +12,7 @@ impl<'a> Visitor<'a> for KnownTypeNames { fn enter_fragment_definition( &mut self, ctx: &mut VisitorContext<'a>, + _name: &'a Name, fragment_definition: &'a Positioned, ) { let TypeCondition { on: name } = &fragment_definition.node.type_condition.node; diff --git a/src/validation/rules/lone_anonymous_operation.rs b/src/validation/rules/lone_anonymous_operation.rs deleted file mode 100644 index dfd95ef4..00000000 --- a/src/validation/rules/lone_anonymous_operation.rs +++ /dev/null @@ -1,127 +0,0 @@ -use crate::parser::types::{ExecutableDefinition, ExecutableDocument, OperationDefinition}; -use crate::validation::visitor::{Visitor, VisitorContext}; -use crate::Positioned; - -#[derive(Default)] -pub struct LoneAnonymousOperation { - operation_count: Option, -} - -impl<'a> Visitor<'a> for LoneAnonymousOperation { - fn enter_document(&mut self, _ctx: &mut VisitorContext<'a>, doc: &'a ExecutableDocument) { - self.operation_count = Some( - doc.definitions - .iter() - .filter(|d| matches!(&d, ExecutableDefinition::Operation(_))) - .count(), - ); - } - - fn enter_operation_definition( - &mut self, - ctx: &mut VisitorContext<'a>, - operation_definition: &'a Positioned, - ) { - if let Some(operation_count) = self.operation_count { - if operation_definition.node.name.is_none() && operation_count > 1 { - ctx.report_error( - vec![operation_definition.pos], - "This anonymous operation must be the only defined operation", - ); - } - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - pub fn factory() -> LoneAnonymousOperation { - LoneAnonymousOperation::default() - } - - #[test] - fn no_operations() { - expect_passes_rule!( - factory, - r#" - fragment fragA on Type { - field - } - "#, - ); - } - - #[test] - fn one_anon_operation() { - expect_passes_rule!( - factory, - r#" - { - field - } - "#, - ); - } - - #[test] - fn multiple_named_operations() { - expect_passes_rule!( - factory, - r#" - query Foo { - field - } - query Bar { - field - } - "#, - ); - } - - #[test] - fn anon_operation_with_fragment() { - expect_passes_rule!( - factory, - r#" - { - ...Foo - } - fragment Foo on Type { - field - } - "#, - ); - } - - #[test] - fn multiple_anon_operations() { - expect_fails_rule!( - factory, - r#" - { - fieldA - } - { - fieldB - } - "#, - ); - } - - #[test] - fn anon_operation_with_a_mutation() { - expect_fails_rule!( - factory, - r#" - { - fieldA - } - mutation Foo { - fieldB - } - "#, - ); - } -} diff --git a/src/validation/rules/mod.rs b/src/validation/rules/mod.rs index 6ff8d490..157f741d 100644 --- a/src/validation/rules/mod.rs +++ b/src/validation/rules/mod.rs @@ -6,7 +6,6 @@ mod known_argument_names; mod known_directives; mod known_fragment_names; mod known_type_names; -mod lone_anonymous_operation; mod no_fragment_cycles; mod no_undefined_variables; mod no_unused_fragments; @@ -16,8 +15,6 @@ mod possible_fragment_spreads; mod provided_non_null_arguments; mod scalar_leafs; mod unique_argument_names; -mod unique_fragment_names; -mod unique_operation_names; mod unique_variable_names; mod upload_file; mod variables_are_input_types; @@ -31,7 +28,6 @@ pub use known_argument_names::KnownArgumentNames; pub use known_directives::KnownDirectives; pub use known_fragment_names::KnownFragmentNames; pub use known_type_names::KnownTypeNames; -pub use lone_anonymous_operation::LoneAnonymousOperation; pub use no_fragment_cycles::NoFragmentCycles; pub use no_undefined_variables::NoUndefinedVariables; pub use no_unused_fragments::NoUnusedFragments; @@ -41,8 +37,6 @@ pub use possible_fragment_spreads::PossibleFragmentSpreads; pub use provided_non_null_arguments::ProvidedNonNullArguments; pub use scalar_leafs::ScalarLeafs; pub use unique_argument_names::UniqueArgumentNames; -pub use unique_fragment_names::UniqueFragmentNames; -pub use unique_operation_names::UniqueOperationNames; pub use unique_variable_names::UniqueVariableNames; pub use upload_file::UploadFile; pub use variables_are_input_types::VariablesAreInputTypes; diff --git a/src/validation/rules/no_fragment_cycles.rs b/src/validation/rules/no_fragment_cycles.rs index 9f84883f..add04625 100644 --- a/src/validation/rules/no_fragment_cycles.rs +++ b/src/validation/rules/no_fragment_cycles.rs @@ -1,5 +1,5 @@ use crate::error::RuleError; -use crate::parser::types::{ExecutableDocument, FragmentDefinition, FragmentSpread}; +use crate::parser::types::{ExecutableDocument, FragmentDefinition, FragmentSpread, Name}; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::{Pos, Positioned}; use std::collections::{HashMap, HashSet}; @@ -75,16 +75,17 @@ impl<'a> Visitor<'a> for NoFragmentCycles<'a> { fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, - fragment_definition: &'a Positioned, + name: &'a Name, + _fragment_definition: &'a Positioned, ) { - self.current_fragment = Some(&fragment_definition.node.name.node); - self.fragment_order - .push(&fragment_definition.node.name.node); + self.current_fragment = Some(name); + self.fragment_order.push(name); } fn exit_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: &'a Name, _fragment_definition: &'a Positioned, ) { self.current_fragment = None; @@ -122,6 +123,7 @@ mod tests { r#" fragment fragA on Dog { ...fragB } fragment fragB on Dog { name } + { __typename } "#, ); } @@ -133,6 +135,7 @@ mod tests { r#" fragment fragA on Dog { ...fragB, ...fragB } fragment fragB on Dog { name } + { __typename } "#, ); } @@ -145,6 +148,7 @@ mod tests { fragment fragA on Dog { ...fragB, ...fragC } fragment fragB on Dog { ...fragC } fragment fragC on Dog { name } + { __typename } "#, ); } @@ -162,6 +166,7 @@ mod tests { ... on Dog { ...nameFragment } ... on Cat { ...nameFragment } } + { __typename } "#, ); } @@ -174,6 +179,7 @@ mod tests { fragment nameFragment on Pet { ...UnknownFragment } + { __typename } "#, ); } @@ -184,6 +190,7 @@ mod tests { factory, r#" fragment fragA on Human { relatives { ...fragA } }, + { __typename } "#, ); } @@ -194,6 +201,7 @@ mod tests { factory, r#" fragment fragA on Dog { ...fragA } + { __typename } "#, ); } @@ -208,6 +216,7 @@ mod tests { ...fragA } } + { __typename } "#, ); } @@ -219,6 +228,7 @@ mod tests { r#" fragment fragA on Dog { ...fragB } fragment fragB on Dog { ...fragA } + { __typename } "#, ); } @@ -230,6 +240,7 @@ mod tests { r#" fragment fragB on Dog { ...fragA } fragment fragA on Dog { ...fragB } + { __typename } "#, ); } @@ -249,6 +260,7 @@ mod tests { ...fragA } } + { __typename } "#, ); } @@ -266,6 +278,7 @@ mod tests { fragment fragZ on Dog { ...fragO } fragment fragO on Dog { ...fragP } fragment fragP on Dog { ...fragA, ...fragX } + { __typename } "#, ); } @@ -278,6 +291,7 @@ mod tests { fragment fragA on Dog { ...fragB, ...fragC } fragment fragB on Dog { ...fragA } fragment fragC on Dog { ...fragA } + { __typename } "#, ); } @@ -290,6 +304,7 @@ mod tests { fragment fragA on Dog { ...fragC } fragment fragB on Dog { ...fragC } fragment fragC on Dog { ...fragA, ...fragB } + { __typename } "#, ); } @@ -302,6 +317,7 @@ mod tests { fragment fragA on Dog { ...fragB } fragment fragB on Dog { ...fragB, ...fragC } fragment fragC on Dog { ...fragA, ...fragB } + { __typename } "#, ); } diff --git a/src/validation/rules/no_undefined_variables.rs b/src/validation/rules/no_undefined_variables.rs index 89a74e99..8b90b64d 100644 --- a/src/validation/rules/no_undefined_variables.rs +++ b/src/validation/rules/no_undefined_variables.rs @@ -76,13 +76,10 @@ impl<'a> Visitor<'a> for NoUndefinedVariables<'a> { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, + name: Option<&'a Name>, operation_definition: &'a Positioned, ) { - let name = operation_definition - .node - .name - .as_ref() - .map(|name| &*name.node); + let name = name.map(|name| name.as_str()); self.current_scope = Some(Scope::Operation(name)); self.defined_variables .insert(name, (operation_definition.pos, HashSet::new())); @@ -91,9 +88,10 @@ impl<'a> Visitor<'a> for NoUndefinedVariables<'a> { fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, - fragment_definition: &'a Positioned, + name: &'a Name, + _fragment_definition: &'a Positioned, ) { - self.current_scope = Some(Scope::Fragment(&fragment_definition.node.name.node)); + self.current_scope = Some(Scope::Fragment(name)); } fn enter_variable_definition( diff --git a/src/validation/rules/no_unused_fragments.rs b/src/validation/rules/no_unused_fragments.rs index 45da8ffb..ee6b5553 100644 --- a/src/validation/rules/no_unused_fragments.rs +++ b/src/validation/rules/no_unused_fragments.rs @@ -1,6 +1,5 @@ use crate::parser::types::{ - ExecutableDefinition, ExecutableDocument, FragmentDefinition, FragmentSpread, - OperationDefinition, + ExecutableDocument, FragmentDefinition, FragmentSpread, Name, OperationDefinition, }; use crate::validation::utils::Scope; use crate::validation::visitor::{Visitor, VisitorContext}; @@ -36,19 +35,11 @@ impl<'a> Visitor<'a> for NoUnusedFragments<'a> { fn exit_document(&mut self, ctx: &mut VisitorContext<'a>, doc: &'a ExecutableDocument) { let mut reachable = HashSet::new(); - for def in &doc.definitions { - if let ExecutableDefinition::Operation(operation_definition) = def { - self.find_reachable_fragments( - &Scope::Operation( - operation_definition - .node - .name - .as_ref() - .map(|name| &*name.node), - ), - &mut reachable, - ); - } + for (name, _) in doc.operations.iter() { + self.find_reachable_fragments( + &Scope::Operation(name.map(|name| name.as_str())), + &mut reachable, + ); } for (fragment_name, pos) in &self.defined_fragments { @@ -64,25 +55,21 @@ impl<'a> Visitor<'a> for NoUnusedFragments<'a> { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, - operation_definition: &'a Positioned, + name: Option<&'a Name>, + _operation_definition: &'a Positioned, ) { - self.current_scope = Some(Scope::Operation( - operation_definition - .node - .name - .as_ref() - .map(|name| &*name.node), - )); + self.current_scope = Some(Scope::Operation(name.map(|name| name.as_str()))); } fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, + name: &'a Name, fragment_definition: &'a Positioned, ) { - self.current_scope = Some(Scope::Fragment(&fragment_definition.node.name.node)); + self.current_scope = Some(Scope::Fragment(name)); self.defined_fragments - .insert((&fragment_definition.node.name.node, fragment_definition.pos)); + .insert((name, fragment_definition.pos)); } fn enter_fragment_spread( diff --git a/src/validation/rules/no_unused_variables.rs b/src/validation/rules/no_unused_variables.rs index 26e06ef1..b0faa52a 100644 --- a/src/validation/rules/no_unused_variables.rs +++ b/src/validation/rules/no_unused_variables.rs @@ -76,13 +76,10 @@ impl<'a> Visitor<'a> for NoUnusedVariables<'a> { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, - operation_definition: &'a Positioned, + name: Option<&'a Name>, + _operation_definition: &'a Positioned, ) { - let op_name = operation_definition - .node - .name - .as_ref() - .map(|name| &*name.node); + let op_name = name.map(|name| name.as_str()); self.current_scope = Some(Scope::Operation(op_name)); self.defined_variables.insert(op_name, HashSet::new()); } @@ -90,9 +87,10 @@ impl<'a> Visitor<'a> for NoUnusedVariables<'a> { fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, - fragment_definition: &'a Positioned, + name: &'a Name, + _fragment_definition: &'a Positioned, ) { - self.current_scope = Some(Scope::Fragment(&fragment_definition.node.name.node)); + self.current_scope = Some(Scope::Fragment(name)); } fn enter_variable_definition( diff --git a/src/validation/rules/possible_fragment_spreads.rs b/src/validation/rules/possible_fragment_spreads.rs index cbc93d9c..27384cb4 100644 --- a/src/validation/rules/possible_fragment_spreads.rs +++ b/src/validation/rules/possible_fragment_spreads.rs @@ -1,6 +1,4 @@ -use crate::parser::types::{ - ExecutableDefinition, ExecutableDocument, FragmentSpread, InlineFragment, TypeCondition, -}; +use crate::parser::types::{ExecutableDocument, FragmentSpread, InlineFragment, TypeCondition}; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::Positioned; use std::collections::HashMap; @@ -12,12 +10,9 @@ pub struct PossibleFragmentSpreads<'a> { impl<'a> Visitor<'a> for PossibleFragmentSpreads<'a> { fn enter_document(&mut self, _ctx: &mut VisitorContext<'a>, doc: &'a ExecutableDocument) { - for d in &doc.definitions { - if let ExecutableDefinition::Fragment(fragment) = &d { - let TypeCondition { on: type_name } = &fragment.node.type_condition.node; - self.fragment_types - .insert(&fragment.node.name.node, &type_name.node); - } + for (name, fragment) in doc.fragments.iter() { + self.fragment_types + .insert(name.as_str(), &fragment.node.type_condition.node.on.node); } } @@ -91,6 +86,7 @@ mod tests { r#" fragment objectWithinObject on Dog { ...dogFragment } fragment dogFragment on Dog { barkVolume } + { __typename } "#, ); } @@ -101,6 +97,7 @@ mod tests { factory, r#" fragment objectWithinObjectAnon on Dog { ... on Dog { barkVolume } } + { __typename } "#, ); } @@ -112,6 +109,7 @@ mod tests { r#" fragment objectWithinInterface on Pet { ...dogFragment } fragment dogFragment on Dog { barkVolume } + { __typename } "#, ); } @@ -123,6 +121,7 @@ mod tests { r#" fragment objectWithinUnion on CatOrDog { ...dogFragment } fragment dogFragment on Dog { barkVolume } + { __typename } "#, ); } @@ -134,6 +133,7 @@ mod tests { r#" fragment unionWithinObject on Dog { ...catOrDogFragment } fragment catOrDogFragment on CatOrDog { __typename } + { __typename } "#, ); } @@ -145,6 +145,7 @@ mod tests { r#" fragment unionWithinInterface on Pet { ...catOrDogFragment } fragment catOrDogFragment on CatOrDog { __typename } + { __typename } "#, ); } @@ -156,6 +157,7 @@ mod tests { r#" fragment unionWithinUnion on DogOrHuman { ...catOrDogFragment } fragment catOrDogFragment on CatOrDog { __typename } + { __typename } "#, ); } @@ -167,6 +169,7 @@ mod tests { r#" fragment interfaceWithinObject on Dog { ...petFragment } fragment petFragment on Pet { name } + { __typename } "#, ); } @@ -178,6 +181,7 @@ mod tests { r#" fragment interfaceWithinInterface on Pet { ...beingFragment } fragment beingFragment on Being { name } + { __typename } "#, ); } @@ -188,6 +192,7 @@ mod tests { factory, r#" fragment interfaceWithinInterface on Pet { ... on Being { name } } + { __typename } "#, ); } @@ -199,6 +204,7 @@ mod tests { r#" fragment interfaceWithinUnion on CatOrDog { ...petFragment } fragment petFragment on Pet { name } + { __typename } "#, ); } @@ -210,6 +216,7 @@ mod tests { r#" fragment invalidObjectWithinObject on Cat { ...dogFragment } fragment dogFragment on Dog { barkVolume } + { __typename } "#, ); } @@ -222,6 +229,7 @@ mod tests { fragment invalidObjectWithinObjectAnon on Cat { ... on Dog { barkVolume } } + { __typename } "#, ); } @@ -233,6 +241,7 @@ mod tests { r#" fragment invalidObjectWithinInterface on Pet { ...humanFragment } fragment humanFragment on Human { pets { name } } + { __typename } "#, ); } @@ -244,6 +253,7 @@ mod tests { r#" fragment invalidObjectWithinUnion on CatOrDog { ...humanFragment } fragment humanFragment on Human { pets { name } } + { __typename } "#, ); } @@ -255,6 +265,7 @@ mod tests { r#" fragment invalidUnionWithinObject on Human { ...catOrDogFragment } fragment catOrDogFragment on CatOrDog { __typename } + { __typename } "#, ); } @@ -266,6 +277,7 @@ mod tests { r#" fragment invalidUnionWithinInterface on Pet { ...humanOrAlienFragment } fragment humanOrAlienFragment on HumanOrAlien { __typename } + { __typename } "#, ); } @@ -277,6 +289,7 @@ mod tests { r#" fragment invalidUnionWithinUnion on CatOrDog { ...humanOrAlienFragment } fragment humanOrAlienFragment on HumanOrAlien { __typename } + { __typename } "#, ); } @@ -288,6 +301,7 @@ mod tests { r#" fragment invalidInterfaceWithinObject on Cat { ...intelligentFragment } fragment intelligentFragment on Intelligent { iq } + { __typename } "#, ); } @@ -301,6 +315,7 @@ mod tests { ...intelligentFragment } fragment intelligentFragment on Intelligent { iq } + { __typename } "#, ); } @@ -313,6 +328,7 @@ mod tests { fragment invalidInterfaceWithinInterfaceAnon on Pet { ...on Intelligent { iq } } + { __typename } "#, ); } @@ -324,6 +340,7 @@ mod tests { r#" fragment invalidInterfaceWithinUnion on HumanOrAlien { ...petFragment } fragment petFragment on Pet { name } + { __typename } "#, ); } diff --git a/src/validation/rules/scalar_leafs.rs b/src/validation/rules/scalar_leafs.rs index 382b8991..b23b9f1e 100644 --- a/src/validation/rules/scalar_leafs.rs +++ b/src/validation/rules/scalar_leafs.rs @@ -47,6 +47,7 @@ mod tests { fragment scalarSelection on Dog { barks } + { __typename } "#, ); } @@ -83,6 +84,7 @@ mod tests { fragment scalarSelectionWithArgs on Dog { doesKnowCommand(dogCommand: SIT) } + { __typename } "#, ); } @@ -95,6 +97,7 @@ mod tests { fragment scalarSelectionsNotAllowedOnBoolean on Dog { barks { sinceWhen } } + { __typename } "#, ); } @@ -107,6 +110,7 @@ mod tests { fragment scalarSelectionsNotAllowedOnEnum on Cat { furColor { inHexdec } } + { __typename } "#, ); } @@ -119,6 +123,7 @@ mod tests { fragment scalarSelectionsNotAllowedWithArgs on Dog { doesKnowCommand(dogCommand: SIT) { sinceWhen } } + { __typename } "#, ); } @@ -131,6 +136,7 @@ mod tests { fragment scalarSelectionsNotAllowedWithDirectives on Dog { name @include(if: true) { isAlsoHumanName } } + { __typename } "#, ); } @@ -143,6 +149,7 @@ mod tests { fragment scalarSelectionsNotAllowedWithDirectivesAndArgs on Dog { doesKnowCommand(dogCommand: SIT) @include(if: true) { sinceWhen } } + { __typename } "#, ); } diff --git a/src/validation/rules/unique_fragment_names.rs b/src/validation/rules/unique_fragment_names.rs deleted file mode 100644 index f0dcdc98..00000000 --- a/src/validation/rules/unique_fragment_names.rs +++ /dev/null @@ -1,163 +0,0 @@ -use crate::parser::types::FragmentDefinition; -use crate::validation::visitor::{Visitor, VisitorContext}; -use crate::Positioned; -use std::collections::HashSet; - -#[derive(Default)] -pub struct UniqueFragmentNames<'a> { - names: HashSet<&'a str>, -} - -impl<'a> Visitor<'a> for UniqueFragmentNames<'a> { - fn enter_fragment_definition( - &mut self, - ctx: &mut VisitorContext<'a>, - fragment_definition: &'a Positioned, - ) { - if !self.names.insert(&fragment_definition.node.name.node) { - ctx.report_error( - vec![fragment_definition.pos], - format!( - "There can only be one fragment named \"{}\"", - fragment_definition.node.name - ), - ) - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - pub fn factory<'a>() -> UniqueFragmentNames<'a> { - UniqueFragmentNames::default() - } - - #[test] - fn no_fragments() { - expect_passes_rule!( - factory, - r#" - { - dog { - name - } - } - "#, - ); - } - - #[test] - fn one_fragment() { - expect_passes_rule!( - factory, - r#" - { - dog { - ...fragA - } - } - fragment fragA on Dog { - name - } - "#, - ); - } - - #[test] - fn many_fragments() { - expect_passes_rule!( - factory, - r#" - { - dog { - ...fragA - ...fragB - ...fragC - } - } - fragment fragA on Dog { - name - } - fragment fragB on Dog { - nickname - } - fragment fragC on Dog { - barkVolume - } - "#, - ); - } - - #[test] - fn inline_fragments_always_unique() { - expect_passes_rule!( - factory, - r#" - { - dorOrHuman { - ...on Dog { - name - } - ...on Dog { - barkVolume - } - } - } - "#, - ); - } - - #[test] - fn fragment_and_operation_named_the_same() { - expect_passes_rule!( - factory, - r#" - query Foo { - dog { - ...Foo - } - } - fragment Foo on Dog { - name - } - "#, - ); - } - - #[test] - fn fragments_named_the_same() { - expect_fails_rule!( - factory, - r#" - { - dog { - ...fragA - } - } - fragment fragA on Dog { - name - } - fragment fragA on Dog { - barkVolume - } - "#, - ); - } - - #[test] - fn fragments_named_the_same_no_reference() { - expect_fails_rule!( - factory, - r#" - fragment fragA on Dog { - name - } - fragment fragA on Dog { - barkVolume - } - "#, - ); - } -} diff --git a/src/validation/rules/unique_operation_names.rs b/src/validation/rules/unique_operation_names.rs deleted file mode 100644 index ec598afa..00000000 --- a/src/validation/rules/unique_operation_names.rs +++ /dev/null @@ -1,158 +0,0 @@ -use crate::parser::types::OperationDefinition; -use crate::validation::visitor::{Visitor, VisitorContext}; -use crate::Positioned; -use std::collections::HashSet; - -#[derive(Default)] -pub struct UniqueOperationNames<'a> { - names: HashSet<&'a str>, -} - -impl<'a> Visitor<'a> for UniqueOperationNames<'a> { - fn enter_operation_definition( - &mut self, - ctx: &mut VisitorContext<'a>, - operation_definition: &'a Positioned, - ) { - if let Some(name) = &operation_definition.node.name { - if !self.names.insert(&name.node) { - ctx.report_error( - vec![name.pos], - format!("There can only be one operation named \"{}\"", name), - ) - } - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - pub fn factory<'a>() -> UniqueOperationNames<'a> { - UniqueOperationNames::default() - } - - #[test] - fn no_operations() { - expect_passes_rule!( - factory, - r#" - fragment fragA on Dog { - name - } - "#, - ); - } - - #[test] - fn one_anon_operation() { - expect_passes_rule!( - factory, - r#" - { - field - } - "#, - ); - } - - #[test] - fn one_named_operation() { - expect_passes_rule!( - factory, - r#" - query Foo { - field - } - "#, - ); - } - - #[test] - fn multiple_operations() { - expect_passes_rule!( - factory, - r#" - query Foo { - dog { - name - } - } - query Bar { - dog { - name - } - } - "#, - ); - } - - #[test] - fn multiple_operations_of_different_types() { - expect_passes_rule!( - factory, - r#" - query Foo { - field - } - mutation Bar { - field - } - "#, - ); - } - - #[test] - fn fragment_and_operation_named_the_same() { - expect_passes_rule!( - factory, - r#" - query Foo { - dog { - ...Foo - } - } - fragment Foo on Dog { - name - } - "#, - ); - } - - #[test] - fn multiple_operations_of_same_name() { - expect_fails_rule!( - factory, - r#" - query Foo { - dog { - name - } - } - query Foo { - human { - name - } - } - "#, - ); - } - - #[test] - fn multiple_ops_of_same_name_of_different_types() { - expect_fails_rule!( - factory, - r#" - query Foo { - dog { - name - } - } - mutation Foo { - testInput - } - "#, - ); - } -} diff --git a/src/validation/rules/unique_variable_names.rs b/src/validation/rules/unique_variable_names.rs index 33dd5bbb..ee9a9dc2 100644 --- a/src/validation/rules/unique_variable_names.rs +++ b/src/validation/rules/unique_variable_names.rs @@ -1,4 +1,4 @@ -use crate::parser::types::{OperationDefinition, VariableDefinition}; +use crate::parser::types::{Name, OperationDefinition, VariableDefinition}; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::Positioned; use std::collections::HashSet; @@ -12,6 +12,7 @@ impl<'a> Visitor<'a> for UniqueVariableNames<'a> { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: Option<&'a Name>, _operation_definition: &'a Positioned, ) { self.names.clear(); diff --git a/src/validation/rules/upload_file.rs b/src/validation/rules/upload_file.rs index 3ec6d08d..4a273a47 100644 --- a/src/validation/rules/upload_file.rs +++ b/src/validation/rules/upload_file.rs @@ -1,4 +1,4 @@ -use crate::parser::types::{OperationDefinition, OperationType}; +use crate::parser::types::{Name, OperationDefinition, OperationType}; use crate::validation::visitor::{Visitor, VisitorContext}; use crate::Positioned; @@ -9,6 +9,7 @@ impl<'a> Visitor<'a> for UploadFile { fn enter_operation_definition( &mut self, ctx: &mut VisitorContext<'a>, + _name: Option<&'a Name>, operation_definition: &'a Positioned, ) { for var in &operation_definition.node.variable_definitions { diff --git a/src/validation/rules/variables_in_allowed_position.rs b/src/validation/rules/variables_in_allowed_position.rs index 36bd862f..6948d2fb 100644 --- a/src/validation/rules/variables_in_allowed_position.rs +++ b/src/validation/rules/variables_in_allowed_position.rs @@ -1,5 +1,5 @@ use crate::parser::types::{ - ExecutableDocument, FragmentDefinition, FragmentSpread, OperationDefinition, Value, + ExecutableDocument, FragmentDefinition, FragmentSpread, Name, OperationDefinition, Value, VariableDefinition, }; use crate::registry::MetaTypeName; @@ -72,23 +72,19 @@ impl<'a> Visitor<'a> for VariableInAllowedPosition<'a> { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, - operation_definition: &'a Positioned, + name: Option<&'a Name>, + _operation_definition: &'a Positioned, ) { - self.current_scope = Some(Scope::Operation( - operation_definition - .node - .name - .as_ref() - .map(|name| &*name.node), - )); + self.current_scope = Some(Scope::Operation(name.map(|name| name.as_str()))); } fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, - fragment_definition: &'a Positioned, + name: &'a Name, + _fragment_definition: &'a Positioned, ) { - self.current_scope = Some(Scope::Fragment(&fragment_definition.node.name.node)); + self.current_scope = Some(Scope::Fragment(name)); } fn enter_variable_definition( diff --git a/src/validation/visitor.rs b/src/validation/visitor.rs index 0a778fd6..9fa71a37 100644 --- a/src/validation/visitor.rs +++ b/src/validation/visitor.rs @@ -1,8 +1,8 @@ use crate::error::RuleError; use crate::parser::types::{ - Directive, ExecutableDefinition, ExecutableDocument, Field, FragmentDefinition, FragmentSpread, - InlineFragment, Name, OperationDefinition, OperationType, Selection, SelectionSet, - TypeCondition, Value, VariableDefinition, + Directive, ExecutableDocument, Field, FragmentDefinition, FragmentSpread, InlineFragment, Name, + OperationDefinition, OperationType, Selection, SelectionSet, TypeCondition, Value, + VariableDefinition, }; use crate::registry::{self, MetaType, MetaTypeName}; use crate::{Pos, Positioned, Variables}; @@ -14,7 +14,7 @@ pub struct VisitorContext<'a> { pub errors: Vec, type_stack: Vec>, input_type: Vec>>, - fragments: HashMap<&'a str, &'a Positioned>, + fragments: &'a HashMap>, } impl<'a> VisitorContext<'a> { @@ -29,16 +29,7 @@ impl<'a> VisitorContext<'a> { errors: Default::default(), type_stack: Default::default(), input_type: Default::default(), - fragments: doc - .definitions - .iter() - .filter_map(|d| match &d { - ExecutableDefinition::Fragment(fragment) => { - Some((&*fragment.node.name.node, fragment)) - } - _ => None, - }) - .collect(), + fragments: &doc.fragments, } } @@ -93,7 +84,7 @@ impl<'a> VisitorContext<'a> { } pub fn fragment(&self, name: &str) -> Option<&'a Positioned> { - self.fragments.get(name).copied() + self.fragments.get(name) } } @@ -104,12 +95,14 @@ pub trait Visitor<'a> { fn enter_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: Option<&'a Name>, _operation_definition: &'a Positioned, ) { } fn exit_operation_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: Option<&'a Name>, _operation_definition: &'a Positioned, ) { } @@ -117,12 +110,14 @@ pub trait Visitor<'a> { fn enter_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: &'a Name, _fragment_definition: &'a Positioned, ) { } fn exit_fragment_definition( &mut self, _ctx: &mut VisitorContext<'a>, + _name: &'a Name, _fragment_definition: &'a Positioned, ) { } @@ -277,37 +272,49 @@ where fn enter_operation_definition( &mut self, ctx: &mut VisitorContext<'a>, + name: Option<&'a Name>, operation_definition: &'a Positioned, ) { - self.0.enter_operation_definition(ctx, operation_definition); - self.1.enter_operation_definition(ctx, operation_definition); + self.0 + .enter_operation_definition(ctx, name, operation_definition); + self.1 + .enter_operation_definition(ctx, name, operation_definition); } fn exit_operation_definition( &mut self, ctx: &mut VisitorContext<'a>, + name: Option<&'a Name>, operation_definition: &'a Positioned, ) { - self.0.exit_operation_definition(ctx, operation_definition); - self.1.exit_operation_definition(ctx, operation_definition); + self.0 + .exit_operation_definition(ctx, name, operation_definition); + self.1 + .exit_operation_definition(ctx, name, operation_definition); } fn enter_fragment_definition( &mut self, ctx: &mut VisitorContext<'a>, + name: &'a Name, fragment_definition: &'a Positioned, ) { - self.0.enter_fragment_definition(ctx, fragment_definition); - self.1.enter_fragment_definition(ctx, fragment_definition); + self.0 + .enter_fragment_definition(ctx, name, fragment_definition); + self.1 + .enter_fragment_definition(ctx, name, fragment_definition); } fn exit_fragment_definition( &mut self, ctx: &mut VisitorContext<'a>, + name: &'a Name, fragment_definition: &'a Positioned, ) { - self.0.exit_fragment_definition(ctx, fragment_definition); - self.1.exit_fragment_definition(ctx, fragment_definition); + self.0 + .exit_fragment_definition(ctx, name, fragment_definition); + self.1 + .exit_fragment_definition(ctx, name, fragment_definition); } fn enter_variable_definition( @@ -455,36 +462,30 @@ pub fn visit<'a, V: Visitor<'a>>( doc: &'a ExecutableDocument, ) { v.enter_document(ctx, doc); - visit_definitions(v, ctx, doc); - v.exit_document(ctx, doc); -} -fn visit_definitions<'a, V: Visitor<'a>>( - v: &mut V, - ctx: &mut VisitorContext<'a>, - doc: &'a ExecutableDocument, -) { - for d in &doc.definitions { - match d { - ExecutableDefinition::Operation(operation) => { - visit_operation_definition(v, ctx, operation); - } - ExecutableDefinition::Fragment(fragment) => { - let TypeCondition { on: name } = &fragment.node.type_condition.node; - ctx.with_type(ctx.registry.types.get(name.node.as_str()), |ctx| { - visit_fragment_definition(v, ctx, fragment) - }); - } - } + for (name, fragment) in &doc.fragments { + ctx.with_type( + ctx.registry + .types + .get(fragment.node.type_condition.node.on.node.as_str()), + |ctx| visit_fragment_definition(v, ctx, name, fragment), + ) } + + for (name, operation) in doc.operations.iter() { + visit_operation_definition(v, ctx, name, operation); + } + + v.exit_document(ctx, doc); } fn visit_operation_definition<'a, V: Visitor<'a>>( v: &mut V, ctx: &mut VisitorContext<'a>, + name: Option<&'a Name>, operation: &'a Positioned, ) { - v.enter_operation_definition(ctx, operation); + v.enter_operation_definition(ctx, name, operation); let root_name = match &operation.node.ty { OperationType::Query => Some(&*ctx.registry.query_type), OperationType::Mutation => ctx.registry.mutation_type.as_deref(), @@ -503,7 +504,7 @@ fn visit_operation_definition<'a, V: Visitor<'a>>( format!("Schema is not configured for {}s.", operation.node.ty), ); } - v.exit_operation_definition(ctx, operation); + v.exit_operation_definition(ctx, name, operation); } fn visit_selection_set<'a, V: Visitor<'a>>( @@ -682,12 +683,13 @@ fn visit_directives<'a, V: Visitor<'a>>( fn visit_fragment_definition<'a, V: Visitor<'a>>( v: &mut V, ctx: &mut VisitorContext<'a>, + name: &'a Name, fragment: &'a Positioned, ) { - v.enter_fragment_definition(ctx, fragment); + v.enter_fragment_definition(ctx, name, fragment); visit_directives(v, ctx, &fragment.node.directives); visit_selection_set(v, ctx, &fragment.node.selection_set); - v.exit_fragment_definition(ctx, fragment); + v.exit_fragment_definition(ctx, name, fragment); } fn visit_fragment_spread<'a, V: Visitor<'a>>(