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

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

Hide file tree

Showing 5 changed files with 33 additions and 25 deletions.
diff --git a/.changeset/yellow-phones-drive.md b/.changeset/yellow-phones-drive.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/packages/plugin/src/rules/require-deprecation-reason.ts b/packages/plugin/src/rules/require-deprecation-reason.ts
@@ -1,5 +1,6 @@
 import { GraphQLESLintRule } from '../types';
 import { valueFromNode } from '../estree-parser/utils';
+import { getLocation } from '../utils';
 
 const rule: GraphQLESLintRule = {
   meta: {
@@ -40,18 +41,16 @@ const rule: GraphQLESLintRule = {
   },
   create(context) {
     return {
-      Directive(node) {
-        if (node && node.name && node.name.value === 'deprecated') {
-          const args = node.arguments || [];
-          const reasonArg = args.find(arg => arg.name && arg.name.value === 'reason');
-          const value = reasonArg ? String(valueFromNode(reasonArg.value) || '').trim() : null;
+      'Directive[name.value=deprecated]'(node) {
+        const args = node.arguments || [];
+        const reasonArg = args.find(arg => arg.name && arg.name.value === 'reason');
+        const value = reasonArg ? String(valueFromNode(reasonArg.value) || '').trim() : null;
 
-          if (!value) {
-            context.report({
-              node: node.name,
-              message: 'Directive "@deprecated" must have a reason!',
-            });
-          }
+        if (!value) {
+          context.report({
+            loc: getLocation(node.loc, node.name.value, { offsetEnd: 0 }),
+            message: 'Directive "@deprecated" must have a reason!',
+          });
         }
       },
     };

diff --git a/packages/plugin/src/rules/require-field-of-type-query-in-mutation-result.ts b/packages/plugin/src/rules/require-field-of-type-query-in-mutation-result.ts
@@ -1,5 +1,5 @@
 import { Kind, FieldDefinitionNode, isObjectType } from 'graphql';
-import { requireGraphQLSchemaFromContext, getTypeName } from '../utils';
+import { requireGraphQLSchemaFromContext, getTypeName, getLocation } from '../utils';
 import { GraphQLESLintRule } from '../types';
 import { GraphQLESTreeNode } from '../estree-parser';
 
@@ -54,20 +54,24 @@ const rule: GraphQLESLintRule = {
     if (!mutationType || !queryType) {
       return {};
     }
-    const selector = `:matches(${Kind.OBJECT_TYPE_DEFINITION}, ${Kind.OBJECT_TYPE_EXTENSION})[name.value=${mutationType.name}] > ${Kind.FIELD_DEFINITION}`;
+    const selector = [
+      `:matches(${Kind.OBJECT_TYPE_DEFINITION}, ${Kind.OBJECT_TYPE_EXTENSION})[name.value=${mutationType.name}]`,
+      '>',
+      Kind.FIELD_DEFINITION,
+      Kind.NAMED_TYPE,
+    ].join(' ');
 
     return {
       [selector](node: GraphQLESTreeNode<FieldDefinitionNode>) {
-        const rawNode = node.rawNode();
-        const typeName = getTypeName(rawNode);
+        const typeName = node.name.value;
         const graphQLType = schema.getType(typeName);
 
         if (isObjectType(graphQLType)) {
           const { fields } = graphQLType.astNode;
           const hasQueryType = fields.some(field => getTypeName(field) === queryType.name);
           if (!hasQueryType) {
             context.report({
-              node,
+              loc: getLocation(node.loc, typeName),
               message: `Mutation result type "${graphQLType.name}" must contain field of type "${queryType.name}".`,
             });
           }

diff --git a/packages/plugin/tests/__snapshots__/require-deprecation-reason.spec.ts.snap b/packages/plugin/tests/__snapshots__/require-deprecation-reason.spec.ts.snap
@@ -4,7 +4,7 @@ exports[` 1`] = `
    1 |
    2 |         type A {
 >  3 |           deprecatedWithoutReason: String @deprecated
-     |                                             ^ Directive "@deprecated" must have a reason!
+     |                                           ^^^^^^^^^^^ Directive "@deprecated" must have a reason!
    4 |           deprecatedWithReason: String @deprecated(reason: "Reason")
    5 |           notDeprecated: String
    6 |         }
@@ -34,7 +34,7 @@ exports[` 2`] = `
    7 |
    8 |         enum testEnum {
 >  9 |           item1 @deprecated
-     |                   ^ Directive "@deprecated" must have a reason!
+     |                 ^^^^^^^^^^^ Directive "@deprecated" must have a reason!
   10 |           item2 @deprecated(reason: "Reason")
   11 |         }
   12 |
@@ -63,7 +63,7 @@ exports[` 3`] = `
   12 |
   13 |         interface testInterface {
 > 14 |           item1: String @deprecated
-     |                           ^ Directive "@deprecated" must have a reason!
+     |                         ^^^^^^^^^^^ Directive "@deprecated" must have a reason!
   15 |           item2: Number @deprecated(reason: "Reason")
   16 |           item3: String
   17 |           item4: String @deprecated(reason: "")
@@ -90,7 +90,7 @@ exports[` 4`] = `
   15 |           item2: Number @deprecated(reason: "Reason")
   16 |           item3: String
 > 17 |           item4: String @deprecated(reason: "")
-     |                           ^ Directive "@deprecated" must have a reason!
+     |                         ^^^^^^^^^^^ Directive "@deprecated" must have a reason!
   18 |           item5: String @deprecated(reason: "  ")
   19 |         }
   20 |       
@@ -115,7 +115,7 @@ exports[` 5`] = `
   16 |           item3: String
   17 |           item4: String @deprecated(reason: "")
 > 18 |           item5: String @deprecated(reason: "  ")
-     |                           ^ Directive "@deprecated" must have a reason!
+     |                         ^^^^^^^^^^^ Directive "@deprecated" must have a reason!
   19 |         }
   20 |       
 `;
diff --git a/...es/plugin/tests/__snapshots__/require-field-of-type-query-in-mutation-result.spec.ts.snap b/...es/plugin/tests/__snapshots__/require-field-of-type-query-in-mutation-result.spec.ts.snap
@@ -5,7 +5,7 @@ exports[` 1`] = `
   2 |         type Query
   3 |         type Mutation {
 > 4 |           createUser: User!
-    |            ^^^^^^^^^^^^^^^^ Mutation result type "User" must contain field of type "Query".
+    |                       ^^^^ Mutation result type "User" must contain field of type "Query".
   5 |         }
   6 |       
 `;
@@ -17,7 +17,7 @@ exports[` 2`] = `
   4 |
   5 |         extend type Mutation {
 > 6 |           createUser: User!
-    |            ^^^^^^^^^^^^^^^^ Mutation result type "User" must contain field of type "Query".
+    |                       ^^^^ Mutation result type "User" must contain field of type "Query".
   7 |         }
   8 |       
 `;
@@ -27,7 +27,7 @@ exports[` 3`] = `
    2 |         type RootQuery
    3 |         type RootMutation {
 >  4 |           createUser: User!
-     |            ^^^^^^^^^^^^^^^^ Mutation result type "User" must contain field of type "RootQuery".
+     |                       ^^^^ Mutation result type "User" must contain field of type "RootQuery".
    5 |         }
    6 |
    7 |         schema {
@@ -43,7 +43,7 @@ exports[` 4`] = `
    3 |         type RootMutation
    4 |         extend type RootMutation {
 >  5 |           createUser: User!
-     |            ^^^^^^^^^^^^^^^^ Mutation result type "User" must contain field of type "RootQuery".
+     |                       ^^^^ Mutation result type "User" must contain field of type "RootQuery".
    6 |         }
    7 |
    8 |         schema {