fix error report for require-deprecation-reason and `require-field-…

…of-type-query-in-mutation-result` rule (#746)
dimaMachina · Oct 31, 2021 · cf3cc4f · cf3cc4f
1 parent a285de3
commit cf3cc4f
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 92 deletions.
diff --git a/.changeset/curly-rocks-brush.md b/.changeset/curly-rocks-brush.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+fix error report for `require-deprecation-reason` and `require-field-of-type-query-in-mutation-result` rule
diff --git a/docs/rules/avoid-duplicate-fields.md b/docs/rules/avoid-duplicate-fields.md
@@ -14,11 +14,11 @@ Checks for duplicate fields in selection set, variables in operation definition,
 ```graphql
 # eslint @graphql-eslint/avoid-duplicate-fields: 'error'
 
-query getUserDetails {
+query {
   user {
-    name # first
+    name
     email
-    name # second
+    name # duplicate field
   }
 }
 ```
@@ -28,7 +28,7 @@ query getUserDetails {
 ```graphql
 # eslint @graphql-eslint/avoid-duplicate-fields: 'error'
 
-query getUsers {
+query {
   users(
     first: 100
     skip: 50
@@ -45,9 +45,11 @@ query getUsers {
 ```graphql
 # eslint @graphql-eslint/avoid-duplicate-fields: 'error'
 
-query getUsers($first: Int!, $first: Int!) {
-  # Duplicate variable
-  users(first: 100, skip: 50, after: "cji629tngfgou0b73kt7vi5jo") {
+query (
+  $first: Int!
+  $first: Int! # duplicate variable
+) {
+  users(first: $first, skip: 50) {
     id
   }
 }

diff --git a/packages/plugin/src/rules/avoid-duplicate-fields.ts b/packages/plugin/src/rules/avoid-duplicate-fields.ts
@@ -1,47 +1,34 @@
-import { Kind } from 'graphql';
+import { VariableDefinitionNode, ArgumentNode, FieldNode, Kind } from 'graphql';
 import { GraphQLESLintRule } from '../types';
+import { GraphQLESTreeNode } from '../estree-parser';
+import { getLocation } from '../utils';
 
 const AVOID_DUPLICATE_FIELDS = 'AVOID_DUPLICATE_FIELDS';
 
-const ensureUnique = () => {
-  const set = new Set<string>();
-
-  return {
-    add: (item: string, onError: () => void) => {
-      if (set.has(item)) {
-        onError();
-      } else {
-        set.add(item);
-      }
-    },
-  };
-};
-
-const rule: GraphQLESLintRule<[], false> = {
+const rule: GraphQLESLintRule = {
   meta: {
     type: 'suggestion',
     docs: {
-      description:
-        'Checks for duplicate fields in selection set, variables in operation definition, or in arguments set of a field.',
+      description: `Checks for duplicate fields in selection set, variables in operation definition, or in arguments set of a field.`,
       category: 'Stylistic Issues',
       url: 'https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/avoid-duplicate-fields.md',
       examples: [
         {
           title: 'Incorrect',
           code: /* GraphQL */ `
-            query getUserDetails {
+            query {
               user {
-                name # first
+                name
                 email
-                name # second
+                name # duplicate field
               }
             }
           `,
         },
         {
           title: 'Incorrect',
           code: /* GraphQL */ `
-            query getUsers {
+            query {
               users(
                 first: 100
                 skip: 50
@@ -56,9 +43,11 @@ const rule: GraphQLESLintRule<[], false> = {
         {
           title: 'Incorrect',
           code: /* GraphQL */ `
-            query getUsers($first: Int!, $first: Int!) {
-              # Duplicate variable
-              users(first: 100, skip: 50, after: "cji629tngfgou0b73kt7vi5jo") {
+            query (
+              $first: Int!
+              $first: Int! # duplicate variable
+            ) {
+              users(first: $first, skip: 50) {
                 id
               }
             }
@@ -67,61 +56,51 @@ const rule: GraphQLESLintRule<[], false> = {
       ],
     },
     messages: {
-      [AVOID_DUPLICATE_FIELDS]: `{{ type }} "{{ fieldName }}" defined multiple times.`,
+      [AVOID_DUPLICATE_FIELDS]: `{{ type }} "{{ fieldName }}" defined multiple times`,
     },
     schema: [],
   },
   create(context) {
+    function checkNode(
+      usedFields: Set<string>,
+      fieldName: string,
+      type: string,
+      node: GraphQLESTreeNode<VariableDefinitionNode | ArgumentNode | FieldNode>
+    ): void {
+      if (usedFields.has(fieldName)) {
+        context.report({
+          loc: getLocation((node.kind === Kind.FIELD && node.alias ? node.alias : node).loc, fieldName, {
+            offsetEnd: node.kind === Kind.VARIABLE_DEFINITION ? 0 : 1,
+          }),
+          messageId: AVOID_DUPLICATE_FIELDS,
+          data: {
+            type,
+            fieldName,
+          },
+        });
+      } else {
+        usedFields.add(fieldName);
+      }
+    }
+
     return {
       OperationDefinition(node) {
-        const uniqueCheck = ensureUnique();
-
-        for (const arg of node.variableDefinitions || []) {
-          uniqueCheck.add(arg.variable.name.value, () => {
-            context.report({
-              messageId: AVOID_DUPLICATE_FIELDS,
-              data: {
-                type: 'Operation variable',
-                fieldName: arg.variable.name.value,
-              },
-              node: arg,
-            });
-          });
+        const set = new Set<string>();
+        for (const varDef of node.variableDefinitions) {
+          checkNode(set, varDef.variable.name.value, 'Operation variable', varDef);
         }
       },
       Field(node) {
-        const uniqueCheck = ensureUnique();
-
-        for (const arg of node.arguments || []) {
-          uniqueCheck.add(arg.name.value, () => {
-            context.report({
-              messageId: AVOID_DUPLICATE_FIELDS,
-              data: {
-                type: 'Field argument',
-                fieldName: arg.name.value,
-              },
-              node: arg,
-            });
-          });
+        const set = new Set<string>();
+        for (const arg of node.arguments) {
+          checkNode(set, arg.name.value, 'Field argument', arg);
         }
       },
       SelectionSet(node) {
-        const uniqueCheck = ensureUnique();
-
-        for (const selection of node.selections || []) {
+        const set = new Set<string>();
+        for (const selection of node.selections) {
           if (selection.kind === Kind.FIELD) {
-            const nameToCheck = selection.alias?.value || selection.name.value;
-
-            uniqueCheck.add(nameToCheck, () => {
-              context.report({
-                messageId: AVOID_DUPLICATE_FIELDS,
-                data: {
-                  type: 'Field',
-                  fieldName: nameToCheck,
-                },
-                node: selection,
-              });
-            });
+            checkNode(set, selection.alias?.value || selection.name.value, 'Field', selection);
           }
         }
       },

diff --git a/packages/plugin/src/rules/no-hashtag-description.ts b/packages/plugin/src/rules/no-hashtag-description.ts
@@ -1,5 +1,6 @@
 import { TokenKind } from 'graphql';
 import { GraphQLESLintRule } from '../types';
+import { getLocation } from '../utils';
 
 const HASHTAG_COMMENT = 'HASHTAG_COMMENT';
 
@@ -69,10 +70,7 @@ const rule: GraphQLESLintRule = {
             if (!isEslintComment && line !== prev.line && next.kind === TokenKind.NAME && linesAfter < 2) {
               context.report({
                 messageId: HASHTAG_COMMENT,
-                loc: {
-                  start: { line, column },
-                  end: { line, column },
-                },
+                loc: getLocation({ start: { line, column } }),
               });
             }
           }

diff --git a/packages/plugin/tests/__snapshots__/avoid-duplicate-fields.spec.ts.snap b/packages/plugin/tests/__snapshots__/avoid-duplicate-fields.spec.ts.snap
@@ -3,7 +3,7 @@
 exports[` 1`] = `
   1 |
 > 2 |         query test($v: String, $t: String, $v: String) {
-    |                                             ^^^^ Operation variable "v" defined multiple times.
+    |                                            ^^ Operation variable "v" defined multiple times
   3 |           id
   4 |         }
   5 |       
@@ -13,7 +13,7 @@ exports[` 2`] = `
   1 |
   2 |         query test {
 > 3 |           users(first: 100, after: 10, filter: "test", first: 50) {
-    |                                                         ^^^^^^^ Field argument "first" defined multiple times.
+    |                                                        ^^^^^ Field argument "first" defined multiple times
   4 |             id
   5 |           }
   6 |         }
@@ -28,7 +28,7 @@ exports[` 3`] = `
    5 |             name
    6 |             email
 >  7 |             name
-     |              ^ Field "name" defined multiple times.
+     |             ^^^^ Field "name" defined multiple times
    8 |           }
    9 |         }
   10 |       
@@ -42,7 +42,7 @@ exports[` 4`] = `
    5 |             name
    6 |             email
 >  7 |             email: somethingElse
-     |              ^^^^^^^ Field "email" defined multiple times.
+     |             ^^^^^ Field "email" defined multiple times
    8 |           }
    9 |         }
   10 |       

diff --git a/packages/plugin/tests/__snapshots__/examples.spec.ts.snap b/packages/plugin/tests/__snapshots__/examples.spec.ts.snap
@@ -191,7 +191,7 @@ Array [
     filePath: examples/graphql-config/operations/user.fragment.graphql,
     messages: Array [
       Object {
-        message: Field "name" defined multiple times.,
+        message: Field "name" defined multiple times,
         ruleId: @graphql-eslint/avoid-duplicate-fields,
       },
     ],

diff --git a/packages/plugin/tests/__snapshots__/no-hashtag-description.spec.ts.snap b/packages/plugin/tests/__snapshots__/no-hashtag-description.spec.ts.snap
@@ -3,7 +3,7 @@
 exports[` 1`] = `
   1 |
 > 2 |         # Bad
-    |          ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
+    |         ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
   3 |         type Query {
   4 |           foo: String
   5 |         }
@@ -14,7 +14,7 @@ exports[` 2`] = `
   1 |
   2 |         # multiline
 > 3 |         # multiline
-    |          ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
+    |         ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
   4 |         type Query {
   5 |           foo: String
   6 |         }
@@ -25,7 +25,7 @@ exports[` 3`] = `
   1 |
   2 |         type Query {
 > 3 |           # Bad
-    |            ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
+    |           ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
   4 |           foo: String
   5 |         }
   6 |       
@@ -36,7 +36,7 @@ exports[` 4`] = `
   2 |         type Query {
   3 |           bar: ID
 > 4 |           # Bad
-    |            ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
+    |           ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
   5 |           foo: ID
   6 |           # Good
   7 |         }
@@ -48,7 +48,7 @@ exports[` 5`] = `
   2 |         type Query {
   3 |           user(
 > 4 |             # Bad
-    |              ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
+    |             ^ Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.
   5 |             id: Int!
   6 |           ): User
   7 |         }

diff --git a/packages/plugin/tests/avoid-duplicate-fields.spec.ts b/packages/plugin/tests/avoid-duplicate-fields.spec.ts
@@ -12,7 +12,7 @@ ruleTester.runGraphQLTests('avoid-duplicate-fields', rule, {
           id
         }
       `,
-      errors: [{ message: 'Operation variable "v" defined multiple times.' }],
+      errors: [{ message: 'Operation variable "v" defined multiple times' }],
     },
     {
       code: /* GraphQL */ `
@@ -22,7 +22,7 @@ ruleTester.runGraphQLTests('avoid-duplicate-fields', rule, {
           }
         }
       `,
-      errors: [{ message: 'Field argument "first" defined multiple times.' }],
+      errors: [{ message: 'Field argument "first" defined multiple times' }],
     },
     {
       code: /* GraphQL */ `
@@ -35,7 +35,7 @@ ruleTester.runGraphQLTests('avoid-duplicate-fields', rule, {
           }
         }
       `,
-      errors: [{ message: 'Field "name" defined multiple times.' }],
+      errors: [{ message: 'Field "name" defined multiple times' }],
     },
     {
       code: /* GraphQL */ `
@@ -48,7 +48,7 @@ ruleTester.runGraphQLTests('avoid-duplicate-fields', rule, {
           }
         }
       `,
-      errors: [{ message: 'Field "email" defined multiple times.' }],
+      errors: [{ message: 'Field "email" defined multiple times' }],
     },
   ],
 });