diff --git a/benches/parser.rs b/benches/parser.rs index 2f8db70..4d39b52 100644 --- a/benches/parser.rs +++ b/benches/parser.rs @@ -41,19 +41,15 @@ fn graphql_ast_print_graphql_query_rs(bench: &mut Bencher) { bench.iter(|| ast.print()); } - - fn graphql_ast_print_gql_parser(bench: &mut Bencher) { use graphql_parser::query::parse_query; let ast = parse_query::<&str>(QUERY).ok().unwrap(); - bench.iter(|| { - ast.to_string() - }); + bench.iter(|| ast.to_string()); } fn graphql_ast_print_apollo_parser(bench: &mut Bencher) { - use apollo_parser::Parser; use apollo_parser::cst::CstNode; + use apollo_parser::Parser; let parser = Parser::new(QUERY); let cst = parser.parse(); let doc = cst.document(); diff --git a/src/ast/ast.rs b/src/ast/ast.rs index f3afee4..badf8a5 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -1,7 +1,7 @@ pub use super::ast_conversion::*; use crate::error::{Error, ErrorType, Result}; use bumpalo::collections::CollectIn; -use hashbrown::{HashMap, hash_map::DefaultHashBuilder}; +use hashbrown::{hash_map::DefaultHashBuilder, HashMap}; /// A context for a GraphQL document which holds an arena allocator. /// diff --git a/src/ast/ast_conversion.rs b/src/ast/ast_conversion.rs index a92b847..c97e2c0 100644 --- a/src/ast/ast_conversion.rs +++ b/src/ast/ast_conversion.rs @@ -1,6 +1,6 @@ // #![allow(clippy::needless_update)] -use bumpalo::collections::{vec::IntoIter, Vec}; use super::ast::*; +use bumpalo::collections::{vec::IntoIter, Vec}; // TODO: from_iter_in could be a good helper here as well @@ -75,7 +75,7 @@ impl<'a> DefaultIn<'a> for Document<'a> { fn default_in(arena: &'a bumpalo::Bump) -> Self { Document { definitions: Vec::new_in(&arena), - size_hint: 0 + size_hint: 0, } } } diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 1a31781..61e1f81 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -37,7 +37,7 @@ mod parser; mod printer; pub use ast::*; -pub use ast_kind::ASTKind; pub use ast_conversion::DefaultIn; +pub use ast_kind::ASTKind; pub use parser::ParseNode; pub use printer::PrintNode; diff --git a/src/json/values.rs b/src/json/values.rs index 88d2d10..748ecb0 100644 --- a/src/json/values.rs +++ b/src/json/values.rs @@ -287,7 +287,7 @@ mod tests { name: "Int", }))), default_value: Value::Null, - directives: Directives::default_in(&ctx.arena) + directives: Directives::default_in(&ctx.arena), }]; let var_defs = VariableDefinitions { @@ -313,7 +313,7 @@ mod tests { name: "orderByInput", })), default_value: Value::Null, - directives: Directives::default_in(&ctx.arena) + directives: Directives::default_in(&ctx.arena), }]; let var_defs = VariableDefinitions { diff --git a/src/validate/rules/mod.rs b/src/validate/rules/mod.rs index e6a2090..3d9b9c3 100644 --- a/src/validate/rules/mod.rs +++ b/src/validate/rules/mod.rs @@ -1,10 +1,14 @@ use crate::visit::ComposedVisitor; +// Missing schemaless rules +// https://github.com/graphql/graphql-js/blob/main/src/validation/rules/DeferStreamDirectiveLabelRule.ts +// https://github.com/graphql/graphql-js/blob/main/src/validation/rules/DeferStreamDirectiveOnValidOperationsRule.ts mod known_fragment_names; mod lone_anonymous_operation; mod no_fragment_cycles; mod no_undefined_variables; mod no_unused_fragments; +mod no_unused_variables; mod unique_argument_names; mod unique_fragment_names; mod unique_operation_names; @@ -16,6 +20,7 @@ pub use lone_anonymous_operation::*; pub use no_fragment_cycles::*; pub use no_undefined_variables::*; pub use no_unused_fragments::*; +pub use no_unused_variables::*; pub use unique_argument_names::*; pub use unique_fragment_names::*; pub use unique_operation_names::*; @@ -51,7 +56,12 @@ pub type DefaultRules<'a> = ComposedVisitor< 'a, ValidationContext<'a>, NoFragmentCycles<'a>, - NoUndefinedVariables<'a>, + ComposedVisitor< + 'a, + ValidationContext<'a>, + NoUndefinedVariables<'a>, + NoUnusedVariables<'a>, + >, >, >, >, diff --git a/src/validate/rules/no_unused_variables.rs b/src/validate/rules/no_unused_variables.rs new file mode 100644 index 0000000..0405f9f --- /dev/null +++ b/src/validate/rules/no_unused_variables.rs @@ -0,0 +1,94 @@ +use bumpalo::collections::Vec; + +use super::super::{ValidationContext, ValidationRule}; +use crate::{ast::*, visit::*}; + +/// Validate that a document uses all the variables it defines at least once. +/// +/// See [`ValidationRule`] +/// [Reference](https://spec.graphql.org/draft/#sec-All-Variables-Used) +pub struct NoUnusedVariables<'a> { + variables: Vec<'a, &'a str>, + used_variables: Vec<'a, &'a str>, +} + +impl<'a> DefaultIn<'a> for NoUnusedVariables<'a> { + fn default_in(arena: &'a bumpalo::Bump) -> Self { + Self { + variables: Vec::new_in(arena), + used_variables: Vec::new_in(arena), + } + } +} + +impl<'a> ValidationRule<'a> for NoUnusedVariables<'a> {} + +impl<'a> Visitor<'a, ValidationContext<'a>> for NoUnusedVariables<'a> { + fn enter_operation( + &mut self, + _ctx: &mut ValidationContext<'a>, + operation: &'a OperationDefinition<'a>, + _info: &VisitInfo, + ) -> VisitFlow { + operation + .variable_definitions + .children + .iter() + .for_each(|def| { + self.variables.push(def.variable.name); + }); + VisitFlow::Next + } + + fn enter_field( + &mut self, + _ctx: &mut ValidationContext<'a>, + field: &'a Field<'a>, + _info: &VisitInfo, + ) -> VisitFlow { + field.arguments.children.iter().for_each(|arg| { + if let Value::Variable(var) = arg.value { + self.used_variables.push(var.name); + } + }); + VisitFlow::Next + } + + fn leave_document( + &mut self, + ctx: &mut ValidationContext<'a>, + _document: &'a Document<'a>, + _info: &VisitInfo, + ) -> VisitFlow { + self.variables.iter().for_each(|defined_variable| { + if !self.used_variables.contains(defined_variable) { + ctx.add_error("All defined variables must be at least used once"); + } + }); + VisitFlow::Next + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn used_all_variables() { + let ctx = ASTContext::new(); + let document = + Document::parse(&ctx, "query ($x: Int!) { todos(from: $x) { id } }").unwrap(); + NoUnusedVariables::validate(&ctx, document).unwrap(); + } + + #[test] + fn has_unused_variable() { + let ctx = ASTContext::new(); + let document = Document::parse( + &ctx, + "query ($x: Int!, $unused: String!) { todos(from: $x) { id } }", + ) + .unwrap(); + NoUnusedVariables::validate(&ctx, document).unwrap_err(); + } +} diff --git a/src/validate/rules/validate.rs b/src/validate/rules/validate.rs index e69de29..8b13789 100644 --- a/src/validate/rules/validate.rs +++ b/src/validate/rules/validate.rs @@ -0,0 +1 @@ + diff --git a/src/validate/validate.rs b/src/validate/validate.rs index 5affb40..47e8aea 100644 --- a/src/validate/validate.rs +++ b/src/validate/validate.rs @@ -33,7 +33,6 @@ where #[inline] fn default_in(arena: &'a bumpalo::Bump) -> Self { ComposedVisitor::new(A::default_in(arena), B::default_in(arena)) - } } diff --git a/src/visit/folder.rs b/src/visit/folder.rs index 68ba5f7..78d1a09 100644 --- a/src/visit/folder.rs +++ b/src/visit/folder.rs @@ -1030,14 +1030,14 @@ mod tests { if selection_set.is_empty() { return Ok(selection_set); } - + let has_typename = selection_set.selections.iter().any(|selection| { selection .field() .map(|field| field.name == "__typename" && field.alias.is_none()) .unwrap_or(false) }); - + if !has_typename { let typename_field = Selection::Field(Field { alias: None, @@ -1046,13 +1046,16 @@ mod tests { directives: Directives::default_in(&ctx.arena), selection_set: SelectionSet::default_in(&ctx.arena), }); - - let new_selections = selection_set - .into_iter() - .chain(iter::once(typename_field)) - .collect_in::>(&ctx.arena); - - Ok(SelectionSet { selections: new_selections }) + + let new_selections = + selection_set + .into_iter() + .chain(iter::once(typename_field)) + .collect_in::>(&ctx.arena); + + Ok(SelectionSet { + selections: new_selections, + }) } else { Ok(selection_set) } @@ -1066,8 +1069,7 @@ mod tests { let ast = Document::parse(&ctx, query).unwrap(); let mut folder = AddTypenames {}; - let new_ast = ast - .fold_operation(&ctx, None, &mut folder); + let new_ast = ast.fold_operation(&ctx, None, &mut folder); assert_eq!( new_ast.unwrap().print(), "{\n todo {\n id\n author {\n id\n __typename\n }\n __typename\n }\n __typename\n}"