Skip to content

Commit db18317

Browse files
gabriel-laetgabriel-laet
committed
Adding NoUndefinedVariables rule
* Added tests and implementation of `NoUndefinedVariables` rule * Fixed `visit_spread_fragments` attribute on `ValidationVisitor` * Fixed `get_fragment` method on `ValidationContext`
1 parent 1f0c725 commit db18317

File tree

3 files changed

+340
-3
lines changed

3 files changed

+340
-3
lines changed

‎graphql/core/validation/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def enter(self, node, key, parent, path, ancestors):
6868
result = False
6969

7070
if result is None and hasattr(self.instance, 'visit_spread_fragments') and isinstance(node, FragmentSpread):
71-
fragment = self.context.get_fragment(node.name.value)
71+
fragment = self.instance.context.get_fragment(node.name.value)
7272
if fragment:
7373
visit(fragment, self)
7474

@@ -118,7 +118,7 @@ def get_fragment(self, name):
118118
fragments = self._fragments
119119
if fragments is None:
120120
self._fragments = fragments = {}
121-
for statement in self.get_document().definitions:
121+
for statement in self.get_ast().definitions:
122122
if isinstance(statement, FragmentDefinition):
123123
fragments[statement.name.value] = statement
124124
return fragments[name]

‎graphql/core/validation/rules.py

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from graphql.core.language.ast import FragmentDefinition
12
from ..utils import type_from_ast
23
from ..error import GraphQLError
34
from ..type.definition import is_composite_type, is_input_type, is_leaf_type
@@ -183,7 +184,50 @@ class NoFragmentCycles(ValidationRule):
183184

184185

185186
class NoUndefinedVariables(ValidationRule):
186-
pass
187+
def __init__(self, context):
188+
self.operation = None
189+
self.visited_fragment_names = {}
190+
self.defined_variable_names = {}
191+
self.visit_spread_fragments = True
192+
super(NoUndefinedVariables, self).__init__(context)
193+
194+
@staticmethod
195+
def undefined_var_message(var_name):
196+
return 'Variable "${}" is not defined.'.format(var_name)
197+
198+
@staticmethod
199+
def undefined_var_by_op_message(var_name, op_name):
200+
return 'Variable "${}" is not defined by operation "{}".'.format(
201+
var_name, op_name
202+
)
203+
204+
def enter_OperationDefinition(self, node, *args):
205+
self.operation = node
206+
self.visited_fragment_names = {}
207+
self.defined_variable_names = {}
208+
209+
def enter_VariableDefinition(self, node, *args):
210+
self.defined_variable_names[node.variable.name.value] = True
211+
212+
def enter_Variable(self, variable, key, parent, path, ancestors):
213+
var_name = variable.name.value
214+
if var_name not in self.defined_variable_names:
215+
is_fragment = lambda node: isinstance(node, FragmentDefinition)
216+
within_fragment = filter(is_fragment, ancestors)
217+
if within_fragment and self.operation and self.operation.name:
218+
return GraphQLError(
219+
self.undefined_var_by_op_message(var_name, self.operation.name.value),
220+
[variable, self.operation]
221+
)
222+
return GraphQLError(
223+
self.undefined_var_message(var_name),
224+
[variable]
225+
)
226+
227+
def enter_FragmentSpread(self, spread_ast, *args):
228+
if spread_ast.name.value in self.visited_fragment_names:
229+
return False
230+
self.visited_fragment_names[spread_ast.name.value] = True
187231

188232

189233
class NoUnusedVariables(ValidationRule):

‎tests/core_validation/test_no_undefined_variables.py

Lines changed: 293 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,293 @@
1+
from graphql.core.language.location import SourceLocation
2+
from graphql.core.validation.rules import NoUndefinedVariables
3+
from utils import expect_passes_rule, expect_fails_rule
4+
5+
6+
def undefined_var(var_name, line, column):
7+
return {
8+
'message': NoUndefinedVariables.undefined_var_message(var_name),
9+
'locations': [SourceLocation(line, column)]
10+
}
11+
12+
13+
def undefined_var_by_op(var_name, l1, c1, op_name, l2, c2):
14+
return {
15+
'message': NoUndefinedVariables.undefined_var_by_op_message(
16+
var_name, op_name),
17+
'locations': [
18+
SourceLocation(l1, c1),
19+
SourceLocation(l2, c2),
20+
]
21+
}
22+
23+
24+
def test_all_varriables_defined():
25+
expect_passes_rule(NoUndefinedVariables, '''
26+
query Foo($a: String, $b: String, $c: String) {
27+
field(a: $a, b: $b, c: $c)
28+
}
29+
''')
30+
31+
32+
def test_all_variables_deeply_defined():
33+
expect_passes_rule(NoUndefinedVariables, '''
34+
query Foo($a: String, $b: String, $c: String) {
35+
field(a: $a) {
36+
field(b: $b) {
37+
field(c: $c)
38+
}
39+
}
40+
}
41+
''')
42+
43+
44+
def test_all_variables_deeply_in_inline_fragments_defined():
45+
expect_passes_rule(NoUndefinedVariables, '''
46+
query Foo($a: String, $b: String, $c: String) {
47+
... on Type {
48+
field(a: $a) {
49+
field(b: $b) {
50+
... on Type {
51+
field(c: $c)
52+
}
53+
}
54+
}
55+
}
56+
}
57+
''')
58+
59+
60+
def test_all_variables_in_fragments_deeply_defined():
61+
expect_passes_rule(NoUndefinedVariables, '''
62+
query Foo($a: String, $b: String, $c: String) {
63+
...FragA
64+
}
65+
fragment FragA on Type {
66+
field(a: $a) {
67+
...FragB
68+
}
69+
}
70+
fragment FragB on Type {
71+
field(b: $b) {
72+
...FragC
73+
}
74+
}
75+
fragment FragC on Type {
76+
field(c: $c)
77+
}
78+
''')
79+
80+
81+
def test_variable_within_single_fragment_defined_in_multiple_operations():
82+
expect_passes_rule(NoUndefinedVariables, '''
83+
query Foo($a: String) {
84+
...FragA
85+
}
86+
query Bar($a: String) {
87+
...FragA
88+
}
89+
fragment FragA on Type {
90+
field(a: $a)
91+
}
92+
''')
93+
94+
95+
def test_variable_within_fragments_defined_in_operations():
96+
expect_passes_rule(NoUndefinedVariables, '''
97+
query Foo($a: String) {
98+
...FragA
99+
}
100+
query Bar($b: String) {
101+
...FragB
102+
}
103+
fragment FragA on Type {
104+
field(a: $a)
105+
}
106+
fragment FragB on Type {
107+
field(b: $b)
108+
}
109+
''')
110+
111+
112+
def test_variable_within_recursive_fragment_defined():
113+
expect_passes_rule(NoUndefinedVariables, '''
114+
query Foo($a: String) {
115+
...FragA
116+
}
117+
fragment FragA on Type {
118+
field(a: $a) {
119+
...FragA
120+
}
121+
}
122+
''')
123+
124+
125+
def test_variable_not_defined():
126+
expect_fails_rule(NoUndefinedVariables, '''
127+
query Foo($a: String, $b: String, $c: String) {
128+
field(a: $a, b: $b, c: $c, d: $d)
129+
}
130+
''', [
131+
undefined_var('d', 3, 39)
132+
])
133+
134+
135+
def variable_not_defined_by_unnamed_query():
136+
expect_fails_rule(NoUndefinedVariables, '''
137+
{
138+
field(a: $a)
139+
}
140+
''', [
141+
undefined_var('a', 3, 18)
142+
])
143+
144+
145+
def test_multiple_variables_not_defined():
146+
expect_fails_rule(NoUndefinedVariables, '''
147+
query Foo($b: String) {
148+
field(a: $a, b: $b, c: $c)
149+
}
150+
''', [
151+
undefined_var('a', 3, 18),
152+
undefined_var('c', 3, 32)
153+
])
154+
155+
156+
def test_variable_in_fragment_not_defined_by_unnamed_query():
157+
expect_fails_rule(NoUndefinedVariables, '''
158+
{
159+
...FragA
160+
}
161+
fragment FragA on Type {
162+
field(a: $a)
163+
}
164+
''', [
165+
undefined_var('a', 6, 18)
166+
])
167+
168+
169+
def test_variable_in_fragment_not_defined_by_operation():
170+
expect_fails_rule(NoUndefinedVariables, '''
171+
query Foo($a: String, $b: String) {
172+
...FragA
173+
}
174+
fragment FragA on Type {
175+
field(a: $a) {
176+
...FragB
177+
}
178+
}
179+
fragment FragB on Type {
180+
field(b: $b) {
181+
...FragC
182+
}
183+
}
184+
fragment FragC on Type {
185+
field(c: $c)
186+
}
187+
''', [
188+
undefined_var_by_op('c', 16, 18, 'Foo', 2, 7)
189+
])
190+
191+
192+
def test_multiple_variables_in_fragments_not_defined():
193+
expect_fails_rule(NoUndefinedVariables, '''
194+
query Foo($b: String) {
195+
...FragA
196+
}
197+
fragment FragA on Type {
198+
field(a: $a) {
199+
...FragB
200+
}
201+
}
202+
fragment FragB on Type {
203+
field(b: $b) {
204+
...FragC
205+
}
206+
}
207+
fragment FragC on Type {
208+
field(c: $c)
209+
}
210+
''', [
211+
undefined_var_by_op('a', 6, 18, 'Foo', 2, 7),
212+
undefined_var_by_op('c', 16, 18, 'Foo', 2, 7)
213+
])
214+
215+
216+
def test_single_variable_in_fragment_not_defined_by_multiple_operations():
217+
expect_fails_rule(NoUndefinedVariables, '''
218+
query Foo($a: String) {
219+
...FragAB
220+
}
221+
query Bar($a: String) {
222+
...FragAB
223+
}
224+
fragment FragAB on Type {
225+
field(a: $a, b: $b)
226+
}
227+
''', [
228+
undefined_var_by_op('b', 9, 25, 'Foo', 2, 7),
229+
undefined_var_by_op('b', 9, 25, 'Bar', 5, 7)
230+
])
231+
232+
233+
def test_variables_in_fragment_not_defined_by_multiple_operations():
234+
expect_fails_rule(NoUndefinedVariables, '''
235+
query Foo($b: String) {
236+
...FragAB
237+
}
238+
query Bar($a: String) {
239+
...FragAB
240+
}
241+
fragment FragAB on Type {
242+
field(a: $a, b: $b)
243+
}
244+
''', [
245+
undefined_var_by_op('a', 9, 18, 'Foo', 2, 7),
246+
undefined_var_by_op('b', 9, 25, 'Bar', 5, 7)
247+
])
248+
249+
250+
def test_variable_in_fragment_used_by_other_operation():
251+
expect_fails_rule(NoUndefinedVariables, '''
252+
query Foo($b: String) {
253+
...FragA
254+
}
255+
query Bar($a: String) {
256+
...FragB
257+
}
258+
fragment FragA on Type {
259+
field(a: $a)
260+
}
261+
fragment FragB on Type {
262+
field(b: $b)
263+
}
264+
''', [
265+
undefined_var_by_op('a', 9, 18, 'Foo', 2, 7),
266+
undefined_var_by_op('b', 12, 18, 'Bar', 5, 7)
267+
])
268+
269+
270+
def test_multiple_undefined_variables_produce_multiple_errors():
271+
expect_fails_rule(NoUndefinedVariables, '''
272+
query Foo($b: String) {
273+
...FragAB
274+
}
275+
query Bar($a: String) {
276+
...FragAB
277+
}
278+
fragment FragAB on Type {
279+
field1(a: $a, b: $b)
280+
...FragC
281+
field3(a: $a, b: $b)
282+
}
283+
fragment FragC on Type {
284+
field2(c: $c)
285+
}
286+
''', [
287+
undefined_var_by_op('a', 9, 19, 'Foo', 2, 7),
288+
undefined_var_by_op('c', 14, 19, 'Foo', 2, 7),
289+
undefined_var_by_op('a', 11, 19, 'Foo', 2, 7),
290+
undefined_var_by_op('b', 9, 26, 'Bar', 5, 7),
291+
undefined_var_by_op('c', 14, 19, 'Bar', 5, 7),
292+
undefined_var_by_op('b', 11, 26, 'Bar', 5, 7),
293+
])

0 commit comments

Comments
 (0)