Skip to content

Commit ffb8d85

Browse files
dotansimhadotansimha
committed
fixes for directive variable visitor
fixes no_unused_variables issues with directives
1 parent 3f6093b commit ffb8d85

File tree

4 files changed

+91
-32
lines changed

4 files changed

+91
-32
lines changed

‎src/ast/ext.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ impl AstWithVariables for OperationDefinition {
8383
&self,
8484
_name: &String,
8585
_parent_arg: (&String, &query::Value),
86-
_parent_field: &query::Field,
8786
_visitor_context: &mut GetVariablesInUseHelper,
8887
) {
8988
_visitor_context.variables_in_use.insert(_name.clone());

‎src/ast/query_visitor.rs

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,22 @@ pub trait QueryVisitor<T = DefaultVisitorContext> {
100100
self.leave_document(node, visitor_context);
101101
}
102102

103+
fn __visit_directive_use(&self, directive: &Directive, visitor_context: &mut T) {
104+
self.enter_directive(&directive, visitor_context);
105+
106+
for (arg_name, arg_value) in &directive.arguments {
107+
match arg_value {
108+
Value::Variable(variable) => {
109+
self.enter_variable(variable, (arg_name, arg_value), visitor_context);
110+
self.leave_variable(variable, (arg_name, arg_value), visitor_context);
111+
}
112+
_ => {}
113+
}
114+
}
115+
116+
self.leave_directive(&directive, visitor_context);
117+
}
118+
103119
fn __visit_selection_set(&self, _node: &SelectionSet, visitor_context: &mut T) {
104120
self.enter_selection_set(_node, visitor_context);
105121

@@ -111,8 +127,7 @@ pub trait QueryVisitor<T = DefaultVisitorContext> {
111127
self.enter_field(field, visitor_context);
112128

113129
for directive in &field.directives {
114-
self.enter_directive(&directive, visitor_context);
115-
self.leave_directive(&directive, visitor_context);
130+
self.__visit_directive_use(directive, visitor_context)
116131
}
117132

118133
for (name, argument) in &field.arguments {
@@ -121,18 +136,8 @@ pub trait QueryVisitor<T = DefaultVisitorContext> {
121136

122137
match argument {
123138
Value::Variable(variable) => {
124-
self.enter_variable(
125-
variable,
126-
(name, argument),
127-
&field,
128-
visitor_context,
129-
);
130-
self.leave_variable(
131-
variable,
132-
(name, argument),
133-
&field,
134-
visitor_context,
135-
);
139+
self.enter_variable(variable, (name, argument), visitor_context);
140+
self.leave_variable(variable, (name, argument), visitor_context);
136141
}
137142
_ => {}
138143
}
@@ -147,8 +152,7 @@ pub trait QueryVisitor<T = DefaultVisitorContext> {
147152
self.enter_fragment_spread(fragment_spread, visitor_context);
148153

149154
for directive in &fragment_spread.directives {
150-
self.enter_directive(&directive, visitor_context);
151-
self.leave_directive(&directive, visitor_context);
155+
self.__visit_directive_use(directive, visitor_context)
152156
}
153157

154158
self.leave_fragment_spread(fragment_spread, visitor_context);
@@ -157,8 +161,7 @@ pub trait QueryVisitor<T = DefaultVisitorContext> {
157161
self.enter_inline_fragment(inline_fragment, visitor_context);
158162

159163
for directive in &inline_fragment.directives {
160-
self.enter_directive(&directive, visitor_context);
161-
self.leave_directive(&directive, visitor_context);
164+
self.__visit_directive_use(directive, visitor_context)
162165
}
163166

164167
self.__visit_selection_set(&inline_fragment.selection_set, visitor_context);
@@ -256,15 +259,13 @@ pub trait QueryVisitor<T = DefaultVisitorContext> {
256259
&self,
257260
_name: &String,
258261
_parent_arg: (&String, &Value),
259-
_parent_field: &Field,
260262
_visitor_context: &mut T,
261263
) {
262264
}
263265
fn leave_variable(
264266
&self,
265267
_name: &String,
266268
_parent_arg: (&String, &Value),
267-
_parent_field: &Field,
268269
_visitor_context: &mut T,
269270
) {
270271
}

‎src/ast/type_info_query_visitor.rs

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use super::{
33
TypeInfoRegistry,
44
};
55
use crate::static_graphql::{
6-
query::{self, Directive, Field, Type, Value},
6+
query::{self, Directive, Type, Value},
77
schema::{self},
88
};
99

@@ -189,6 +189,38 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
189189
self.leave_document(node, visitor_context, &type_info);
190190
}
191191

192+
fn __visit_directive_use(
193+
&self,
194+
directive: &query::Directive,
195+
visitor_context: &mut T,
196+
_type_info_registry: &TypeInfoRegistry,
197+
type_info: &mut TypeInfo,
198+
) {
199+
self.enter_directive(&directive, visitor_context, type_info);
200+
201+
for (arg_name, arg_value) in &directive.arguments {
202+
match arg_value {
203+
Value::Variable(variable) => {
204+
self.enter_variable(
205+
variable,
206+
(arg_name, arg_value),
207+
visitor_context,
208+
type_info,
209+
);
210+
self.leave_variable(
211+
variable,
212+
(arg_name, arg_value),
213+
visitor_context,
214+
type_info,
215+
);
216+
}
217+
_ => {}
218+
}
219+
}
220+
221+
self.leave_directive(&directive, visitor_context, type_info);
222+
}
223+
192224
fn __visit_selection_set(
193225
&self,
194226
_node: &query::SelectionSet,
@@ -235,8 +267,12 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
235267
self.enter_field(field, visitor_context, type_info);
236268

237269
for directive in &field.directives {
238-
self.enter_directive(&directive, visitor_context, type_info);
239-
self.leave_directive(&directive, visitor_context, type_info);
270+
self.__visit_directive_use(
271+
directive,
272+
visitor_context,
273+
type_info_registry,
274+
type_info,
275+
);
240276
}
241277

242278
for (argument_name, argument_type) in &field.arguments {
@@ -281,14 +317,12 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
281317
self.enter_variable(
282318
variable,
283319
(argument_name, argument_type),
284-
&field,
285320
visitor_context,
286321
type_info,
287322
);
288323
self.leave_variable(
289324
variable,
290325
(argument_name, argument_type),
291-
&field,
292326
visitor_context,
293327
type_info,
294328
);
@@ -320,10 +354,16 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
320354
}
321355
query::Selection::FragmentSpread(fragment_spread) => {
322356
self.enter_fragment_spread(fragment_spread, visitor_context, type_info);
357+
323358
for directive in &fragment_spread.directives {
324-
self.enter_directive(&directive, visitor_context, type_info);
325-
self.leave_directive(&directive, visitor_context, type_info);
359+
self.__visit_directive_use(
360+
directive,
361+
visitor_context,
362+
type_info_registry,
363+
type_info,
364+
);
326365
}
366+
327367
self.leave_fragment_spread(fragment_spread, visitor_context, type_info);
328368
}
329369
query::Selection::InlineFragment(inline_fragment) => {
@@ -339,8 +379,12 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
339379
self.enter_inline_fragment(inline_fragment, visitor_context, type_info);
340380

341381
for directive in &inline_fragment.directives {
342-
self.enter_directive(&directive, visitor_context, type_info);
343-
self.leave_directive(&directive, visitor_context, type_info);
382+
self.__visit_directive_use(
383+
directive,
384+
visitor_context,
385+
type_info_registry,
386+
type_info,
387+
);
344388
}
345389

346390
self.__visit_selection_set(
@@ -542,7 +586,6 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
542586
&self,
543587
_name: &String,
544588
_parent_arg: (&String, &Value),
545-
_parent_field: &Field,
546589
_visitor_context: &mut T,
547590
_type_info: &TypeInfo,
548591
) {
@@ -551,7 +594,6 @@ pub trait TypeInfoQueryVisitor<T = DefaultVisitorContext> {
551594
&self,
552595
_name: &String,
553596
_parent_arg: (&String, &Value),
554-
_parent_field: &Field,
555597
_visitor_context: &mut T,
556598
_type_info: &TypeInfo,
557599
) {

‎src/validation/rules/no_unused_variables.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,20 @@ fn variables_not_used_by_fragment_used_by_other_operation() {
347347
assert!(messages.contains(&&"Variable \"$b\" is never used in operation \"Foo\".".to_owned()));
348348
assert!(messages.contains(&&"Variable \"$a\" is never used in operation \"Bar\".".to_owned()));
349349
}
350+
351+
#[test]
352+
fn should_also_check_directives_usage() {
353+
use crate::validation::test_utils::*;
354+
355+
let mut plan = create_plan_from_rule(Box::new(NoUnusedVariables {}));
356+
let errors = test_operation_without_schema(
357+
"query foo($skip: Boolean!) {
358+
field @skip(if: $skip)
359+
}
360+
",
361+
&mut plan,
362+
);
363+
364+
let messages = get_messages(&errors);
365+
assert_eq!(messages.len(), 0);
366+
}

0 commit comments

Comments
 (0)