Skip to content

Commit 7fefee5

Browse files
jhggjhgg
committed
Implement NoUnusedVariables
* Fix validation visitor visiting spread fragments. * implement unit tests #6
1 parent 1f0c725 commit 7fefee5

File tree

4 files changed

+253
-4
lines changed

4 files changed

+253
-4
lines changed

‎.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,5 @@ docs/_build/
5959
# PyBuilder
6060
target/
6161

62+
# IntelliJ
63+
.idea

‎graphql/core/validation/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ 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)
70+
if result is None and getattr(self.instance, 'visit_spread_fragments', False) and isinstance(node, FragmentSpread):
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: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,53 @@ class NoUndefinedVariables(ValidationRule):
187187

188188

189189
class NoUnusedVariables(ValidationRule):
190-
pass
190+
visited_fragment_names = None
191+
variable_definitions = None
192+
variable_name_used = None
193+
visit_spread_fragments = True
194+
195+
def __init__(self, context):
196+
super(NoUnusedVariables, self).__init__(context)
197+
198+
def enter_OperationDefinition(self, *args):
199+
self.visited_fragment_names = set()
200+
self.variable_definitions = []
201+
self.variable_name_used = set()
202+
203+
def leave_OperationDefinition(self, *args):
204+
errors = [
205+
GraphQLError(
206+
self.unused_variable_message(variable_definition.variable.name.value),
207+
[variable_definition]
208+
)
209+
for variable_definition in self.variable_definitions
210+
if variable_definition.variable.name.value not in self.variable_name_used
211+
]
212+
213+
if errors:
214+
return errors
215+
216+
def enter_VariableDefinition(self, node, *args):
217+
if self.variable_definitions is not None:
218+
self.variable_definitions.append(node)
219+
220+
return False
221+
222+
def enter_Variable(self, node, *args):
223+
if self.variable_name_used is not None:
224+
self.variable_name_used.add(node.name.value)
225+
226+
def enter_FragmentSpread(self, node, *args):
227+
if self.visited_fragment_names is not None:
228+
spread_name = node.name.value
229+
if spread_name in self.visited_fragment_names:
230+
return False
231+
232+
self.visited_fragment_names.add(spread_name)
233+
234+
@staticmethod
235+
def unused_variable_message(variable_name):
236+
return 'Variable "${}" is never used.'.format(variable_name)
191237

192238

193239
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)