Skip to content

Commit f713823

Browse files
author
Dimitri POSTOLOV
authored
fix false positive case for no-unused-variables rule when variable used in a fragment (#653)
1 parent ed6644b commit f713823

File tree

8 files changed

+147
-11
lines changed

8 files changed

+147
-11
lines changed

‎.changeset/swift-games-walk.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-eslint/eslint-plugin': minor
3+
---
4+
5+
fix false positive case for `no-unused-variables` rule

‎packages/plugin/src/rules/graphql-js-validation.ts‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ const validationToRule = (
103103
};
104104
};
105105

106+
const importFiles = (context: GraphQLESLintRuleContext) => {
107+
const code = context.getSourceCode().text;
108+
if (!isGraphQLImportFile(code)) {
109+
return null;
110+
}
111+
// Import documents because file contains '#import' comments
112+
return processImport(context.getFilename());
113+
};
114+
106115
export const GRAPHQL_JS_VALIDATIONS = Object.assign(
107116
{},
108117
validationToRule('executable-definitions', 'ExecutableDefinitions', {
@@ -192,14 +201,7 @@ export const GRAPHQL_JS_VALIDATIONS = Object.assign(
192201
},
193202
],
194203
},
195-
context => {
196-
const code = context.getSourceCode().text;
197-
if (!isGraphQLImportFile(code)) {
198-
return null;
199-
}
200-
// Import documents because file contains '#import' comments
201-
return processImport(context.getFilename());
202-
}
204+
importFiles
203205
),
204206
validationToRule('known-type-names', 'KnownTypeNames', {
205207
description: `A GraphQL document is only valid if referenced types (specifically variable definitions and fragment conditions) are defined by the type schema.`,
@@ -257,9 +259,14 @@ export const GRAPHQL_JS_VALIDATIONS = Object.assign(
257259
return getParentNode(context.getFilename());
258260
}
259261
),
260-
validationToRule('no-unused-variables', 'NoUnusedVariables', {
261-
description: `A GraphQL operation is only valid if all variables defined by an operation are used, either directly or within a spread fragment.`,
262-
}),
262+
validationToRule(
263+
'no-unused-variables',
264+
'NoUnusedVariables',
265+
{
266+
description: `A GraphQL operation is only valid if all variables defined by an operation are used, either directly or within a spread fragment.`,
267+
},
268+
importFiles
269+
),
263270
validationToRule('overlapping-fields-can-be-merged', 'OverlappingFieldsCanBeMerged', {
264271
description: `A selection set is only valid if all fields (including spreading any fragments) either correspond to distinct response names or can be merged without ambiguity.`,
265272
}),

‎packages/plugin/tests/mocks/no-unused-variables-imported.gql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
fragment UserFields on User {
2+
firstName
3+
posts(limit: $limit, offset: $offset) {
4+
id
5+
}
6+
}

‎packages/plugin/tests/mocks/no-unused-variables.gql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#import UserFields from './no-unused-variables-imported.gql'
2+
3+
query User($limit: Int!, $offset: Int!) {
4+
user {
5+
id
6+
...UserFields
7+
}
8+
}

‎packages/plugin/tests/mocks/user-schema.graphql‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
type User {
22
id: ID!
33
firstName: String!
4+
posts(limit: Int = 25, offset: Int = 0): [Post!]!
45
}
56

67
type Post {
78
id: ID!
9+
title: String!
10+
content: String!
811
user: User!
912
}
1013

‎packages/plugin/tests/mocks/user-schema.json‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,51 @@
4242
},
4343
"isDeprecated": false,
4444
"deprecationReason": null
45+
},
46+
{
47+
"name": "posts",
48+
"description": null,
49+
"args": [
50+
{
51+
"name": "limit",
52+
"description": null,
53+
"type": {
54+
"kind": "SCALAR",
55+
"name": "Int",
56+
"ofType": null
57+
},
58+
"defaultValue": "25"
59+
},
60+
{
61+
"name": "offset",
62+
"description": null,
63+
"type": {
64+
"kind": "SCALAR",
65+
"name": "Int",
66+
"ofType": null
67+
},
68+
"defaultValue": "0"
69+
}
70+
],
71+
"type": {
72+
"kind": "NON_NULL",
73+
"name": null,
74+
"ofType": {
75+
"kind": "LIST",
76+
"name": null,
77+
"ofType": {
78+
"kind": "NON_NULL",
79+
"name": null,
80+
"ofType": {
81+
"kind": "OBJECT",
82+
"name": "Post",
83+
"ofType": null
84+
}
85+
}
86+
}
87+
},
88+
"isDeprecated": false,
89+
"deprecationReason": null
4590
}
4691
],
4792
"inputFields": null,
@@ -69,6 +114,16 @@
69114
"enumValues": null,
70115
"possibleTypes": null
71116
},
117+
{
118+
"kind": "SCALAR",
119+
"name": "Int",
120+
"description": "The `Int` scalar type represents non-fractional signed whole numeric values. Int can represent values between -(2^31) and 2^31 - 1.",
121+
"fields": null,
122+
"inputFields": null,
123+
"interfaces": null,
124+
"enumValues": null,
125+
"possibleTypes": null
126+
},
72127
{
73128
"kind": "OBJECT",
74129
"name": "Post",
@@ -90,6 +145,38 @@
90145
"isDeprecated": false,
91146
"deprecationReason": null
92147
},
148+
{
149+
"name": "title",
150+
"description": null,
151+
"args": [],
152+
"type": {
153+
"kind": "NON_NULL",
154+
"name": null,
155+
"ofType": {
156+
"kind": "SCALAR",
157+
"name": "String",
158+
"ofType": null
159+
}
160+
},
161+
"isDeprecated": false,
162+
"deprecationReason": null
163+
},
164+
{
165+
"name": "content",
166+
"description": null,
167+
"args": [],
168+
"type": {
169+
"kind": "NON_NULL",
170+
"name": null,
171+
"ofType": {
172+
"kind": "SCALAR",
173+
"name": "String",
174+
"ofType": null
175+
}
176+
},
177+
"isDeprecated": false,
178+
"deprecationReason": null
179+
},
93180
{
94181
"name": "user",
95182
"description": null,

‎packages/plugin/tests/mocks/user-schema.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ const SCHEMA = /* GraphQL */ `
22
type User {
33
id: ID!
44
firstName: String!
5+
posts(limit: Int = 25, offset: Int = 0): [Post!]!
56
}
67
78
type Post {
89
id: ID!
10+
title: String!
11+
content: String!
912
user: User!
1013
}
1114

‎packages/plugin/tests/no-unused-variables.spec.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { join } from 'path';
2+
import { GraphQLRuleTester, rules } from '../src';
3+
4+
const ruleTester = new GraphQLRuleTester();
5+
6+
ruleTester.runGraphQLTests('no-unused-variables', rules['no-unused-variables'], {
7+
valid: [
8+
{
9+
filename: join(__dirname, 'mocks/no-unused-variables.gql'),
10+
code: ruleTester.fromMockFile('no-unused-variables.gql'),
11+
parserOptions: {
12+
schema: join(__dirname, 'mocks/user-schema.graphql'),
13+
},
14+
},
15+
],
16+
invalid: [],
17+
});

0 commit comments

Comments
 (0)