fix error report for unique-fragment-name and `unique-operation-nam…

…e` rule (#735) * fix error report for `unique-fragment-name` and `unique-operation-name` rule
dimaMachina · Oct 31, 2021 · f712780 · f712780
1 parent a8c7b43
commit f712780
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 22 deletions.
diff --git a/.changeset/odd-islands-run.md b/.changeset/odd-islands-run.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+fix error report for `unique-fragment-name` and `unique-operation-name` rule
diff --git a/packages/plugin/src/rules/unique-fragment-name.ts b/packages/plugin/src/rules/unique-fragment-name.ts
@@ -2,7 +2,7 @@ import { relative } from 'path';
 import { FragmentDefinitionNode, Kind, OperationDefinitionNode } from 'graphql';
 import { GraphQLESLintRule, GraphQLESLintRuleContext } from '../types';
 import { GraphQLESTreeNode } from '../estree-parser';
-import { normalizePath, requireSiblingsOperations, getOnDiskFilepath } from '../utils';
+import { normalizePath, requireSiblingsOperations, getOnDiskFilepath, getLocation } from '../utils';
 import { FragmentSource, OperationSource } from '../sibling-operations';
 
 const RULE_NAME = 'unique-fragment-name';
@@ -14,11 +14,7 @@ export const checkNode = (
   ruleName: string,
   messageId: string
 ): void => {
-  const documentName = node.name?.value;
-  if (!documentName) {
-    return;
-  }
-
+  const documentName = node.name.value;
   const siblings = requireSiblingsOperations(ruleName, context);
   const siblingDocuments: (FragmentSource | OperationSource)[] =
     node.kind === Kind.FRAGMENT_DEFINITION ? siblings.getFragment(documentName) : siblings.getOperation(documentName);
@@ -31,7 +27,6 @@ export const checkNode = (
   });
 
   if (conflictingDocuments.length > 0) {
-    const { start, end } = node.name.loc;
     context.report({
       messageId,
       data: {
@@ -40,16 +35,7 @@ export const checkNode = (
           .map(f => `\t${relative(process.cwd(), getOnDiskFilepath(f.filePath))}`)
           .join('\n'),
       },
-      loc: {
-        start: {
-          line: start.line,
-          column: start.column - 1,
-        },
-        end: {
-          line: end.line,
-          column: end.column - 1,
-        },
-      },
+      loc: getLocation(node.name.loc, documentName),
     });
   }
 };

diff --git a/packages/plugin/src/rules/unique-operation-name.ts b/packages/plugin/src/rules/unique-operation-name.ts
@@ -58,7 +58,7 @@ const rule: GraphQLESLintRule = {
   },
   create(context) {
     return {
-      OperationDefinition(node) {
+      'OperationDefinition[name!=undefined]'(node) {
         checkNode(context, node, RULE_NAME, UNIQUE_OPERATION_NAME);
       },
     };

diff --git a/packages/plugin/src/utils.ts b/packages/plugin/src/utils.ts
@@ -189,3 +189,28 @@ export const convertCase = (style: CaseStyle, str: string): string => {
       return lowerCase(str).replace(/ /g, '-');
   }
 };
+
+export function getLocation(
+  loc: Partial<AST.SourceLocation>,
+  fieldName = '',
+  offset?: { offsetStart?: number; offsetEnd?: number }
+): AST.SourceLocation {
+  const { start } = loc;
+
+  /*
+   * ESLint has 0-based column number
+   * https://eslint.org/docs/developer-guide/working-with-rules#contextreport
+   */
+  const { offsetStart = 1, offsetEnd = 1 } = offset ?? {};
+
+  return {
+    start: {
+      line: start.line,
+      column: start.column - offsetStart,
+    },
+    end: {
+      line: start.line,
+      column: start.column - offsetEnd + fieldName.length,
+    },
+  };
+}
diff --git a/packages/plugin/tests/__snapshots__/unique-fragment-name.spec.ts.snap b/packages/plugin/tests/__snapshots__/unique-fragment-name.spec.ts.snap
@@ -2,13 +2,13 @@
 
 exports[` 1`] = `
 > 1 | fragment HasIdFields on U { a b c }
-    |          ^ Fragment named "HasIdFields" already defined in:
+    |          ^^^^^^^^^^^ Fragment named "HasIdFields" already defined in:
 	-1866344359.graphql
 `;
 
 exports[` 2`] = `
 > 1 | fragment HasIdFields on U { a b c }
-    |          ^ Fragment named "HasIdFields" already defined in:
+    |          ^^^^^^^^^^^ Fragment named "HasIdFields" already defined in:
 	-1559743294.graphql
 	-1866344359.graphql
 `;
diff --git a/packages/plugin/tests/__snapshots__/unique-operation-name.spec.ts.snap b/packages/plugin/tests/__snapshots__/unique-operation-name.spec.ts.snap
@@ -2,13 +2,13 @@
 
 exports[` 1`] = `
 > 1 | query test { bar }
-    |       ^ Operation named "test" already defined in:
+    |       ^^^^ Operation named "test" already defined in:
 	6844040.graphql
 `;
 
 exports[` 2`] = `
 > 1 | query test { bar }
-    |       ^ Operation named "test" already defined in:
+    |       ^^^^ Operation named "test" already defined in:
 	6844040.graphql
 	84823255.graphql
 `;