fix false positive case for no-unused-variables rule when variable …

…used in a fragment (#653)
dimaMachina · Oct 20, 2021 · f713823 · f713823
1 parent ed6644b
commit f713823
Show file tree

Hide file tree

Showing 8 changed files with 147 additions and 11 deletions.
diff --git a/.changeset/swift-games-walk.md b/.changeset/swift-games-walk.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': minor
+---
+
+fix false positive case for `no-unused-variables` rule
diff --git a/packages/plugin/src/rules/graphql-js-validation.ts b/packages/plugin/src/rules/graphql-js-validation.ts
@@ -103,6 +103,15 @@ const validationToRule = (
   };
 };
 
+const importFiles = (context: GraphQLESLintRuleContext) => {
+  const code = context.getSourceCode().text;
+  if (!isGraphQLImportFile(code)) {
+    return null;
+  }
+  // Import documents because file contains '#import' comments
+  return processImport(context.getFilename());
+};
+
 export const GRAPHQL_JS_VALIDATIONS = Object.assign(
   {},
   validationToRule('executable-definitions', 'ExecutableDefinitions', {
@@ -192,14 +201,7 @@ export const GRAPHQL_JS_VALIDATIONS = Object.assign(
         },
       ],
     },
-    context => {
-      const code = context.getSourceCode().text;
-      if (!isGraphQLImportFile(code)) {
-        return null;
-      }
-      // Import documents because file contains '#import' comments
-      return processImport(context.getFilename());
-    }
+    importFiles
   ),
   validationToRule('known-type-names', 'KnownTypeNames', {
     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(
       return getParentNode(context.getFilename());
     }
   ),
-  validationToRule('no-unused-variables', 'NoUnusedVariables', {
-    description: `A GraphQL operation is only valid if all variables defined by an operation are used, either directly or within a spread fragment.`,
-  }),
+  validationToRule(
+    'no-unused-variables',
+    'NoUnusedVariables',
+    {
+      description: `A GraphQL operation is only valid if all variables defined by an operation are used, either directly or within a spread fragment.`,
+    },
+    importFiles
+  ),
   validationToRule('overlapping-fields-can-be-merged', 'OverlappingFieldsCanBeMerged', {
     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.`,
   }),

diff --git a/packages/plugin/tests/mocks/no-unused-variables-imported.gql b/packages/plugin/tests/mocks/no-unused-variables-imported.gql
@@ -0,0 +1,6 @@
+fragment UserFields on User {
+  firstName
+  posts(limit: $limit, offset: $offset) {
+    id
+  }
+}
diff --git a/packages/plugin/tests/mocks/no-unused-variables.gql b/packages/plugin/tests/mocks/no-unused-variables.gql
@@ -0,0 +1,8 @@
+#import UserFields from './no-unused-variables-imported.gql'
+
+query User($limit: Int!, $offset: Int!) {
+  user {
+    id
+    ...UserFields
+  }
+}
diff --git a/packages/plugin/tests/mocks/user-schema.graphql b/packages/plugin/tests/mocks/user-schema.graphql
@@ -1,10 +1,13 @@
 type User {
   id: ID!
   firstName: String!
+  posts(limit: Int = 25, offset: Int = 0): [Post!]!
 }
 
 type Post {
   id: ID!
+  title: String!
+  content: String!
   user: User!
 }
 

diff --git a/packages/plugin/tests/mocks/user-schema.json b/packages/plugin/tests/mocks/user-schema.json
@@ -42,6 +42,51 @@
             },
             "isDeprecated": false,
             "deprecationReason": null
+          },
+          {
+            "name": "posts",
+            "description": null,
+            "args": [
+              {
+                "name": "limit",
+                "description": null,
+                "type": {
+                  "kind": "SCALAR",
+                  "name": "Int",
+                  "ofType": null
+                },
+                "defaultValue": "25"
+              },
+              {
+                "name": "offset",
+                "description": null,
+                "type": {
+                  "kind": "SCALAR",
+                  "name": "Int",
+                  "ofType": null
+                },
+                "defaultValue": "0"
+              }
+            ],
+            "type": {
+              "kind": "NON_NULL",
+              "name": null,
+              "ofType": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "NON_NULL",
+                  "name": null,
+                  "ofType": {
+                    "kind": "OBJECT",
+                    "name": "Post",
+                    "ofType": null
+                  }
+                }
+              }
+            },
+            "isDeprecated": false,
+            "deprecationReason": null
           }
         ],
         "inputFields": null,
@@ -69,6 +114,16 @@
         "enumValues": null,
         "possibleTypes": null
       },
+      {
+        "kind": "SCALAR",
+        "name": "Int",
+        "description": "The `Int` scalar type represents non-fractional signed whole numeric values. Int can represent values between -(2^31) and 2^31 - 1.",
+        "fields": null,
+        "inputFields": null,
+        "interfaces": null,
+        "enumValues": null,
+        "possibleTypes": null
+      },
       {
         "kind": "OBJECT",
         "name": "Post",
@@ -90,6 +145,38 @@
             "isDeprecated": false,
             "deprecationReason": null
           },
+          {
+            "name": "title",
+            "description": null,
+            "args": [],
+            "type": {
+              "kind": "NON_NULL",
+              "name": null,
+              "ofType": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              }
+            },
+            "isDeprecated": false,
+            "deprecationReason": null
+          },
+          {
+            "name": "content",
+            "description": null,
+            "args": [],
+            "type": {
+              "kind": "NON_NULL",
+              "name": null,
+              "ofType": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              }
+            },
+            "isDeprecated": false,
+            "deprecationReason": null
+          },
           {
             "name": "user",
             "description": null,

diff --git a/packages/plugin/tests/mocks/user-schema.ts b/packages/plugin/tests/mocks/user-schema.ts
@@ -2,10 +2,13 @@ const SCHEMA = /* GraphQL */ `
   type User {
     id: ID!
     firstName: String!
+    posts(limit: Int = 25, offset: Int = 0): [Post!]!
   }
 
   type Post {
     id: ID!
+    title: String!
+    content: String!
     user: User!
   }
 

diff --git a/packages/plugin/tests/no-unused-variables.spec.ts b/packages/plugin/tests/no-unused-variables.spec.ts
@@ -0,0 +1,17 @@
+import { join } from 'path';
+import { GraphQLRuleTester, rules } from '../src';
+
+const ruleTester = new GraphQLRuleTester();
+
+ruleTester.runGraphQLTests('no-unused-variables', rules['no-unused-variables'], {
+  valid: [
+    {
+      filename: join(__dirname, 'mocks/no-unused-variables.gql'),
+      code: ruleTester.fromMockFile('no-unused-variables.gql'),
+      parserOptions: {
+        schema: join(__dirname, 'mocks/user-schema.graphql'),
+      },
+    },
+  ],
+  invalid: [],
+});