Skip to content

Commit 7702eea

Browse files
dittosdittos
committed
Merge branch 'jhgg-no-unused-variables'
Conflicts: .gitignore
2 parents 8ed7642 + 3ce5dba commit 7702eea

File tree

3 files changed

+256
-5
lines changed

3 files changed

+256
-5
lines changed

‎graphql/core/validation/__init__.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,14 @@ def enter(self, node, key, parent, path, ancestors):
6767
append(self.errors, result)
6868
result = False
6969

70-
if result is None and hasattr(self.instance, 'visit_spread_fragments') and isinstance(node, FragmentSpread):
71-
fragment = self.context.get_fragment(node.name.value)
72-
if fragment:
73-
visit(fragment, self)
70+
if result is None and getattr(self.instance, 'visit_spread_fragments', False) and isinstance(node, FragmentSpread):
71+
try:
72+
fragment = self.instance.context.get_fragment(node.name.value)
73+
except KeyError:
74+
pass
75+
else:
76+
if fragment:
77+
visit(fragment, self)
7478

7579
if result is False:
7680
self.type_info.leave(node)

‎graphql/core/validation/rules.py

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,53 @@ class NoUndefinedVariables(ValidationRule):
247247

248248

249249
class NoUnusedVariables(ValidationRule):
250-
pass
250+
visited_fragment_names = None
251+
variable_definitions = None
252+
variable_name_used = None
253+
visit_spread_fragments = True
254+
255+
def __init__(self, context):
256+
super(NoUnusedVariables, self).__init__(context)
257+
258+
def enter_OperationDefinition(self, *args):
259+
self.visited_fragment_names = set()
260+
self.variable_definitions = []
261+
self.variable_name_used = set()
262+
263+
def leave_OperationDefinition(self, *args):
264+
errors = [
265+
GraphQLError(
266+
self.unused_variable_message(variable_definition.variable.name.value),
267+
[variable_definition]
268+
)
269+
for variable_definition in self.variable_definitions
270+
if variable_definition.variable.name.value not in self.variable_name_used
271+
]
272+
273+
if errors:
274+
return errors
275+
276+
def enter_VariableDefinition(self, node, *args):
277+
if self.variable_definitions is not None:
278+
self.variable_definitions.append(node)
279+
280+
return False
281+
282+
def enter_Variable(self, node, *args):
283+
if self.variable_name_used is not None:
284+
self.variable_name_used.add(node.name.value)
285+
286+
def enter_FragmentSpread(self, node, *args):
287+
if self.visited_fragment_names is not None:
288+
spread_name = node.name.value
289+
if spread_name in self.visited_fragment_names:
290+
return False
291+
292+
self.visited_fragment_names.add(spread_name)
293+
294+
@staticmethod
295+
def unused_variable_message(variable_name):
296+
return 'Variable "${}" is never used.'.format(variable_name)
251297

252298

253299
class KnownDirectives(ValidationRule):

‎tests/core_validation/test_no_unused_variables.py

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
from graphql.core.language.location import SourceLocation
2+
from graphql.core.validation.rules import NoUnusedVariables
3+
from utils import expect_passes_rule, expect_fails_rule
4+
5+
6+
def unused_variable(variable_name, line, column):
7+
return {
8+
'message': NoUnusedVariables.unused_variable_message(variable_name),
9+
'locations': [SourceLocation(line, column)]
10+
}
11+
12+
13+
def test_uses_all_variables():
14+
expect_passes_rule(NoUnusedVariables, '''
15+
query Foo($a: String, $b: String, $c: String) {
16+
field(a: $a, b: $b, c: $c)
17+
}
18+
''')
19+
20+
21+
def test_uses_all_variables_deeply():
22+
expect_passes_rule(NoUnusedVariables, '''
23+
query Foo($a: String, $b: String, $c: String) {
24+
field(a: $a) {
25+
field(b: $b) {
26+
field(c: $c)
27+
}
28+
}
29+
}
30+
''')
31+
32+
33+
def test_uses_all_variables_deeply_in_inline_fragments():
34+
expect_passes_rule(NoUnusedVariables, '''
35+
query Foo($a: String, $b: String, $c: String) {
36+
... on Type {
37+
field(a: $a) {
38+
field(b: $b) {
39+
... on Type {
40+
field(c: $c)
41+
}
42+
}
43+
}
44+
}
45+
}
46+
''')
47+
48+
49+
def test_uses_all_variables_in_fragment():
50+
expect_passes_rule(NoUnusedVariables, '''
51+
query Foo($a: String, $b: String, $c: String) {
52+
...FragA
53+
}
54+
fragment FragA on Type {
55+
field(a: $a) {
56+
...FragB
57+
}
58+
}
59+
fragment FragB on Type {
60+
field(b: $b) {
61+
...FragC
62+
}
63+
}
64+
fragment FragC on Type {
65+
field(c: $c)
66+
}
67+
''')
68+
69+
70+
def test_variable_used_by_fragment_in_multiple_operations():
71+
expect_passes_rule(NoUnusedVariables, '''
72+
query Foo($a: String) {
73+
...FragA
74+
}
75+
query Bar($b: String) {
76+
...FragB
77+
}
78+
fragment FragA on Type {
79+
field(a: $a)
80+
}
81+
fragment FragB on Type {
82+
field(b: $b)
83+
}
84+
''')
85+
86+
87+
def test_variable_used_by_recursive_fragment():
88+
expect_passes_rule(NoUnusedVariables, '''
89+
query Foo($a: String) {
90+
...FragA
91+
}
92+
fragment FragA on Type {
93+
field(a: $a) {
94+
...FragA
95+
}
96+
}
97+
''')
98+
99+
100+
def test_variable_not_used():
101+
expect_fails_rule(NoUnusedVariables, '''
102+
query Foo($a: String, $b: String, $c: String) {
103+
field(a: $a, b: $b)
104+
}
105+
''', [
106+
unused_variable('c', 2, 41)
107+
])
108+
109+
110+
def test_multiple_variables_not_used():
111+
expect_fails_rule(NoUnusedVariables, '''
112+
query Foo($a: String, $b: String, $c: String) {
113+
field(b: $b)
114+
}
115+
''', [
116+
unused_variable('a', 2, 17),
117+
unused_variable('c', 2, 41)
118+
])
119+
120+
121+
def test_variable_not_used_in_fragments():
122+
expect_fails_rule(NoUnusedVariables, '''
123+
query Foo($a: String, $b: String, $c: String) {
124+
...FragA
125+
}
126+
fragment FragA on Type {
127+
field(a: $a) {
128+
...FragB
129+
}
130+
}
131+
fragment FragB on Type {
132+
field(b: $b) {
133+
...FragC
134+
}
135+
}
136+
fragment FragC on Type {
137+
field
138+
}
139+
''', [
140+
unused_variable('c', 2, 41)
141+
])
142+
143+
144+
def test_multiple_variables_not_used_in_fragments():
145+
expect_fails_rule(NoUnusedVariables, '''
146+
query Foo($a: String, $b: String, $c: String) {
147+
...FragA
148+
}
149+
fragment FragA on Type {
150+
field {
151+
...FragB
152+
}
153+
}
154+
fragment FragB on Type {
155+
field(b: $b) {
156+
...FragC
157+
}
158+
}
159+
fragment FragC on Type {
160+
field
161+
}
162+
''', [
163+
unused_variable('a', 2, 17),
164+
unused_variable('c', 2, 41)
165+
])
166+
167+
168+
def test_variable_not_used_by_unreferenced_fragment():
169+
expect_fails_rule(NoUnusedVariables, '''
170+
query Foo($b: String) {
171+
...FragA
172+
}
173+
fragment FragA on Type {
174+
field(a: $a)
175+
}
176+
fragment FragB on Type {
177+
field(b: $b)
178+
}
179+
''', [
180+
unused_variable('b', 2, 17),
181+
])
182+
183+
184+
def test_variable_not_used_by_fragment_used_by_other_operation():
185+
expect_fails_rule(NoUnusedVariables, '''
186+
query Foo($b: String) {
187+
...FragA
188+
}
189+
query Bar($a: String) {
190+
...FragB
191+
}
192+
fragment FragA on Type {
193+
field(a: $a)
194+
}
195+
fragment FragB on Type {
196+
field(b: $b)
197+
}
198+
''', [
199+
unused_variable('b', 2, 17),
200+
unused_variable('a', 5, 17),
201+
])

0 commit comments

Comments
 (0)