improve error messages for some rules (#1534)

* description-style.ts * input-name.ts * no-case-insensitive-enum-values-duplicates.ts * require-deprecation-date.ts * no-scalar-result-type-on-mutation.ts * strict-id-in-types.ts * require-type-pattern-with-oneof.ts * require-nullable-fields-with-oneof.ts * no-hashtag-description.ts * no-hashtag-description.ts * alphabetize.ts * alphabetize.ts * fix type issues * update snapshots * set chageset to minor * improve enum/input value name
dimaMachina · Mar 26, 2023 · 4e94259 · 4e94259
1 parent f42f44b
commit 4e94259
Show file tree

Hide file tree

Showing 34 changed files with 321 additions and 252 deletions.
diff --git a/.changeset/curly-eagles-attack.md b/.changeset/curly-eagles-attack.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': minor
+---
+
+improve error messages for some rules
diff --git a/.eslintrc.cjs b/.eslintrc.cjs
@@ -9,7 +9,7 @@ module.exports = {
   rules: {
     'unicorn/prefer-array-some': 'error',
     'unicorn/better-regex': 'error',
-    'prefer-destructuring': ['error', { object: true }],
+    'prefer-destructuring': ['error', { VariableDeclarator: { object: true } }],
     quotes: ['error', 'single', { avoidEscape: true }], // Matches Prettier, but also replaces backticks
   },
   overrides: [

diff --git a/packages/plugin/src/rules/alphabetize.ts b/packages/plugin/src/rules/alphabetize.ts
@@ -23,7 +23,7 @@ import lowerCase from 'lodash.lowercase';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRuleListener } from '../testkit.js';
 import { GraphQLESLintRule } from '../types.js';
-import { ARRAY_DEFAULT_OPTIONS, truthy } from '../utils.js';
+import { ARRAY_DEFAULT_OPTIONS, displayNodeName, truthy } from '../utils.js';
 
 const RULE_ID = 'alphabetize';
 
@@ -218,7 +218,7 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
       },
     },
     messages: {
-      [RULE_ID]: '`{{ currName }}` should be before {{ prevName }}.',
+      [RULE_ID]: '{{ currNode }} should be before {{ prevNode }}',
     },
     schema,
   },
@@ -320,8 +320,8 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
           node: ('alias' in currNode && currNode.alias) || currNode.name,
           messageId: RULE_ID,
           data: {
-            currName,
-            prevName: prevName ? `\`${prevName}\`` : lowerCase(prevNode.kind),
+            currNode: displayNodeName(currNode),
+            prevNode: prevName ? displayNodeName(prevNode) : lowerCase(prevNode.kind),
           },
           *fix(fixer) {
             const prevRange = getRangeWithComments(prevNode);

diff --git a/packages/plugin/src/rules/description-style.ts b/packages/plugin/src/rules/description-style.ts
@@ -2,6 +2,7 @@ import { StringValueNode } from 'graphql';
 import { FromSchema } from 'json-schema-to-ts';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
+import { getNodeName } from '../utils.js';
 
 const schema = {
   type: 'array',
@@ -64,7 +65,9 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
       ) {
         context.report({
           loc: isBlock ? node.loc : node.loc.start,
-          message: `Unexpected ${isBlock ? 'inline' : 'block'} description.`,
+          message: `Unexpected ${isBlock ? 'inline' : 'block'} description for ${getNodeName(
+            (node as any).parent,
+          )}`,
           suggest: [
             {
               desc: `Change to ${isBlock ? 'block' : 'inline'} style description`,

diff --git a/packages/plugin/src/rules/input-name.ts b/packages/plugin/src/rules/input-name.ts
@@ -114,13 +114,12 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
       'FieldDefinition > InputValueDefinition[name.value!=input] > Name'(
         node: GraphQLESTreeNode<NameNode>,
       ) {
-        if (
-          shouldCheckType((node.parent.parent as GraphQLESTreeNode<FieldDefinitionNode>).parent)
-        ) {
+        const fieldDef = node.parent.parent as GraphQLESTreeNode<FieldDefinitionNode>;
+        if (shouldCheckType(fieldDef.parent)) {
           const inputName = node.value;
           context.report({
             node,
-            message: `Input \`${inputName}\` should be called \`input\`.`,
+            message: `Input "${inputName}" should be named "input" for "${fieldDef.parent.name.value}.${fieldDef.name.value}"`,
             suggest: [
               {
                 desc: 'Rename to `input`',

diff --git a/packages/plugin/src/rules/no-case-insensitive-enum-values-duplicates.ts b/packages/plugin/src/rules/no-case-insensitive-enum-values-duplicates.ts
@@ -1,6 +1,7 @@
 import { EnumTypeDefinitionNode, EnumTypeExtensionNode, Kind } from 'graphql';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
+import { getNodeName } from '../utils.js';
 
 export const rule: GraphQLESLintRule = {
   meta: {
@@ -49,7 +50,9 @@ export const rule: GraphQLESLintRule = {
           const enumName = duplicate.name.value;
           context.report({
             node: duplicate.name,
-            message: `Case-insensitive enum values duplicates are not allowed! Found: \`${enumName}\`.`,
+            message: `Unexpected case-insensitive enum values duplicates for ${getNodeName(
+              duplicate,
+            )}`,
             suggest: [
               {
                 desc: `Remove \`${enumName}\` enum value`,

diff --git a/packages/plugin/src/rules/no-hashtag-description.ts b/packages/plugin/src/rules/no-hashtag-description.ts
@@ -1,17 +1,18 @@
 import { DocumentNode, Token, TokenKind } from 'graphql';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
+import { getNodeName } from '../utils.js';
 
-const HASHTAG_COMMENT = 'HASHTAG_COMMENT';
+export const RULE_ID = 'HASHTAG_COMMENT';
 
 export const rule: GraphQLESLintRule = {
   meta: {
     type: 'suggestion',
     hasSuggestions: true,
     schema: [],
     messages: {
-      [HASHTAG_COMMENT]:
-        'Using hashtag `#` for adding GraphQL descriptions is not allowed. Prefer using `"""` for multiline, or `"` for a single line description.',
+      [RULE_ID]:
+        'Unexpected GraphQL descriptions as hashtag `#` for {{ nodeName }}.\nPrefer using `"""` for multiline, or `"` for a single line description.',
     },
     docs: {
       description:
@@ -69,15 +70,28 @@ export const rule: GraphQLESLintRule = {
             // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- TODO: remove `!` when drop support of graphql@15
             const isEslintComment = value!.trimStart().startsWith('eslint');
             const linesAfter = next.line - line;
-
             if (
               !isEslintComment &&
               line !== prev.line &&
               next.kind === TokenKind.NAME &&
               linesAfter < 2
             ) {
+              const sourceCode = context.getSourceCode();
+              const { tokens } = sourceCode.ast;
+
+              const t = tokens.find(
+                token =>
+                  token.loc.start.line === next.line && token.loc.start.column === next.column - 1,
+              )!;
+              const nextNode = sourceCode.getNodeByRangeIndex(t.range[1] + 1)!;
+
               context.report({
-                messageId: HASHTAG_COMMENT,
+                messageId: RULE_ID,
+                data: {
+                  nodeName: getNodeName(
+                    'name' in nextNode ? (nextNode as any) : (nextNode as any).parent,
+                  ),
+                },
                 loc: {
                   line,
                   column: column - 1,

diff --git a/packages/plugin/src/rules/no-scalar-result-type-on-mutation.ts b/packages/plugin/src/rules/no-scalar-result-type-on-mutation.ts
@@ -1,7 +1,7 @@
-import { isScalarType, NameNode } from 'graphql';
+import { isScalarType, Kind, NameNode } from 'graphql';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
-import { requireGraphQLSchemaFromContext } from '../utils.js';
+import { getNodeName, requireGraphQLSchemaFromContext } from '../utils.js';
 
 const RULE_ID = 'no-scalar-result-type-on-mutation';
 
@@ -52,9 +52,14 @@ export const rule: GraphQLESLintRule = {
         const typeName = node.value;
         const graphQLType = schema.getType(typeName);
         if (isScalarType(graphQLType)) {
+          let fieldDef = node.parent as any;
+          while (fieldDef.kind !== Kind.FIELD_DEFINITION) {
+            fieldDef = fieldDef.parent;
+          }
+
           context.report({
             node,
-            message: `Unexpected scalar result type \`${typeName}\`.`,
+            message: `Unexpected scalar result type \`${typeName}\` for ${getNodeName(fieldDef)}`,
             suggest: [
               {
                 desc: `Remove \`${typeName}\``,

diff --git a/packages/plugin/src/rules/require-deprecation-date.ts b/packages/plugin/src/rules/require-deprecation-date.ts
@@ -2,6 +2,7 @@ import { DirectiveNode } from 'graphql';
 import { FromSchema } from 'json-schema-to-ts';
 import { GraphQLESTreeNode, valueFromNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
+import { getNodeName } from '../utils.js';
 
 // eslint-disable-next-line unicorn/better-regex
 const DATE_REGEX = /^\d{2}\/\d{2}\/\d{4}$/;
@@ -68,10 +69,11 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
       ],
     },
     messages: {
-      [MESSAGE_REQUIRE_DATE]: 'Directive "@deprecated" must have a deletion date',
-      [MESSAGE_INVALID_FORMAT]: 'Deletion date must be in format "DD/MM/YYYY"',
-      [MESSAGE_INVALID_DATE]: 'Invalid "{{ deletionDate }}" deletion date',
-      [MESSAGE_CAN_BE_REMOVED]: '"{{ nodeName }}" сan be removed',
+      [MESSAGE_REQUIRE_DATE]:
+        'Directive "@deprecated" must have a deletion date for {{ nodeName }}',
+      [MESSAGE_INVALID_FORMAT]: 'Deletion date must be in format "DD/MM/YYYY" for {{ nodeName }}',
+      [MESSAGE_INVALID_DATE]: 'Invalid "{{ deletionDate }}" deletion date for {{ nodeName }}',
+      [MESSAGE_CAN_BE_REMOVED]: '{{ nodeName }} сan be removed',
     },
     schema,
   },
@@ -85,14 +87,21 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
           context.report({
             node: node.name,
             messageId: MESSAGE_REQUIRE_DATE,
+            data: {
+              nodeName: getNodeName(node.parent),
+            },
           });
           return;
         }
         const deletionDate = valueFromNode(deletionDateNode.value as any);
         const isValidDate = DATE_REGEX.test(deletionDate);
 
         if (!isValidDate) {
-          context.report({ node: deletionDateNode.value, messageId: MESSAGE_INVALID_FORMAT });
+          context.report({
+            node: deletionDateNode.value,
+            messageId: MESSAGE_INVALID_FORMAT,
+            data: { nodeName: getNodeName(node.parent) },
+          });
           return;
         }
         let [day, month, year] = deletionDate.split('/');
@@ -106,6 +115,7 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
             messageId: MESSAGE_INVALID_DATE,
             data: {
               deletionDate,
+              nodeName: getNodeName(node.parent),
             },
           });
           return;
@@ -119,7 +129,7 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
           context.report({
             node: parent.name,
             messageId: MESSAGE_CAN_BE_REMOVED,
-            data: { nodeName },
+            data: { nodeName: getNodeName(parent) },
             suggest: [
               {
                 desc: `Remove \`${nodeName}\``,

diff --git a/packages/plugin/src/rules/require-nullable-fields-with-oneof.ts b/packages/plugin/src/rules/require-nullable-fields-with-oneof.ts
@@ -1,6 +1,7 @@
 import { DirectiveNode, Kind } from 'graphql';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
+import { getNodeName } from '../utils.js';
 
 const RULE_ID = 'require-nullable-fields-with-oneof';
 
@@ -33,7 +34,7 @@ export const rule: GraphQLESLintRule = {
       ],
     },
     messages: {
-      [RULE_ID]: 'Field `{{fieldName}}` must be nullable.',
+      [RULE_ID]: '{{ nodeName }} must be nullable when "@oneOf" is in use',
     },
     schema: [],
   },
@@ -53,7 +54,7 @@ export const rule: GraphQLESLintRule = {
             context.report({
               node: field.name,
               messageId: RULE_ID,
-              data: { fieldName: field.name.value },
+              data: { nodeName: getNodeName(field) },
             });
           }
         }

diff --git a/packages/plugin/src/rules/require-type-pattern-with-oneof.ts b/packages/plugin/src/rules/require-type-pattern-with-oneof.ts
@@ -1,6 +1,7 @@
 import { ObjectTypeDefinitionNode } from 'graphql';
 import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
+import { displayNodeName } from '../utils.js';
 
 const RULE_ID = 'require-type-pattern-with-oneof';
 
@@ -36,7 +37,8 @@ export const rule: GraphQLESLintRule = {
       ],
     },
     messages: {
-      [RULE_ID]: 'Type `{{typeName}}` should have `{{fieldName}}` field.',
+      [RULE_ID]:
+        '{{ nodeName }} is defined as output with "@oneOf" and must be defined with "{{ fieldName }}" field',
     },
     schema: [],
   },
@@ -54,7 +56,7 @@ export const rule: GraphQLESLintRule = {
               node: parent.name,
               messageId: RULE_ID,
               data: {
-                typeName: parent.name.value,
+                nodeName: displayNodeName(parent),
                 fieldName,
               },
             });

diff --git a/packages/plugin/src/rules/strict-id-in-types.ts b/packages/plugin/src/rules/strict-id-in-types.ts
@@ -4,6 +4,7 @@ import { GraphQLESTreeNode } from '../estree-converter/index.js';
 import { GraphQLESLintRule } from '../types.js';
 import {
   ARRAY_DEFAULT_OPTIONS,
+  displayNodeName,
   englishJoinWords,
   requireGraphQLSchemaFromContext,
   truthy,
@@ -176,9 +177,9 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
           const pluralTypesSuffix = options.acceptedIdTypes.length > 1 ? 's' : '';
           context.report({
             node: node.name,
-            message: `${typeName} must have exactly one non-nullable unique identifier. Accepted name${pluralNamesSuffix}: ${englishJoinWords(
-              options.acceptedIdNames,
-            )}. Accepted type${pluralTypesSuffix}: ${englishJoinWords(options.acceptedIdTypes)}.`,
+            message: `${displayNodeName(node)} must have exactly one non-nullable unique identifier.
+Accepted name${pluralNamesSuffix}: ${englishJoinWords(options.acceptedIdNames)}.
+Accepted type${pluralTypesSuffix}: ${englishJoinWords(options.acceptedIdTypes)}.`,
           });
         }
       },

diff --git a/packages/plugin/src/utils.ts b/packages/plugin/src/utils.ts
@@ -48,8 +48,6 @@ export const VIRTUAL_DOCUMENT_REGEX = /\/\d+_document.graphql$/;
 
 export const CWD = process.cwd();
 
-export const IS_BROWSER = typeof window !== 'undefined';
-
 export const getTypeName = (node: ASTNode): string =>
   'type' in node ? getTypeName(node.type) : 'name' in node && node.name ? node.name.value : '';
 
@@ -124,20 +122,36 @@ export function truthy<T>(value: T): value is Truthy<T> {
   return !!value;
 }
 
-export function getNodeName(node: GraphQLESTreeNode<ASTNode>): string {
-  const DisplayNodeNameMap: Record<string, string> = {
-    [Kind.OBJECT_TYPE_DEFINITION]: 'type',
-    [Kind.INTERFACE_TYPE_DEFINITION]: 'interface',
-    [Kind.ENUM_TYPE_DEFINITION]: 'enum',
-    [Kind.SCALAR_TYPE_DEFINITION]: 'scalar',
-    [Kind.INPUT_OBJECT_TYPE_DEFINITION]: 'input',
-    [Kind.UNION_TYPE_DEFINITION]: 'union',
-    [Kind.DIRECTIVE_DEFINITION]: 'directive',
-    [Kind.FIELD_DEFINITION]: 'field',
-    [Kind.ENUM_VALUE_DEFINITION]: 'value',
-    [Kind.INPUT_VALUE_DEFINITION]: 'value',
-  } as const;
+const DisplayNodeNameMap: Record<string, string> = {
+  [Kind.OBJECT_TYPE_DEFINITION]: 'type',
+  [Kind.OBJECT_TYPE_EXTENSION]: 'type',
+  [Kind.INTERFACE_TYPE_DEFINITION]: 'interface',
+  [Kind.INTERFACE_TYPE_EXTENSION]: 'interface',
+  [Kind.ENUM_TYPE_DEFINITION]: 'enum',
+  [Kind.ENUM_TYPE_EXTENSION]: 'enum',
+  [Kind.SCALAR_TYPE_DEFINITION]: 'scalar',
+  [Kind.INPUT_OBJECT_TYPE_DEFINITION]: 'input',
+  [Kind.INPUT_OBJECT_TYPE_EXTENSION]: 'input',
+  [Kind.UNION_TYPE_DEFINITION]: 'union',
+  [Kind.UNION_TYPE_EXTENSION]: 'union',
+  [Kind.DIRECTIVE_DEFINITION]: 'directive',
+  [Kind.FIELD_DEFINITION]: 'field',
+  [Kind.ENUM_VALUE_DEFINITION]: 'enum value',
+  [Kind.INPUT_VALUE_DEFINITION]: 'input value',
+  [Kind.ARGUMENT]: 'argument',
+  [Kind.VARIABLE]: 'variable',
+  [Kind.FRAGMENT_DEFINITION]: 'fragment',
+  [Kind.OPERATION_DEFINITION]: 'operation',
+  [Kind.FIELD]: 'field',
+} as const;
 
+export function displayNodeName(node: GraphQLESTreeNode<ASTNode>): string {
+  return `${
+    node.kind === Kind.OPERATION_DEFINITION ? node.operation : DisplayNodeNameMap[node.kind]
+  } "${('alias' in node && node.alias?.value) || ('name' in node && node.name?.value)}"`;
+}
+
+export function getNodeName(node: GraphQLESTreeNode<ASTNode>): string {
   switch (node.kind) {
     case Kind.OBJECT_TYPE_DEFINITION:
     case Kind.INTERFACE_TYPE_DEFINITION:
@@ -146,15 +160,13 @@ export function getNodeName(node: GraphQLESTreeNode<ASTNode>): string {
     case Kind.INPUT_OBJECT_TYPE_DEFINITION:
     case Kind.UNION_TYPE_DEFINITION:
     case Kind.DIRECTIVE_DEFINITION:
-      return `${DisplayNodeNameMap[node.kind]} "${node.name.value}"`;
+      return displayNodeName(node);
     case Kind.FIELD_DEFINITION:
     case Kind.INPUT_VALUE_DEFINITION:
     case Kind.ENUM_VALUE_DEFINITION:
-      return `${DisplayNodeNameMap[node.kind]} "${node.name.value}" in ${
-        DisplayNodeNameMap[node.parent.kind]
-      } "${node.parent.name.value}"`;
+      return `${displayNodeName(node)} in ${displayNodeName(node.parent)}`;
     case Kind.OPERATION_DEFINITION:
-      return node.name ? `${node.operation} "${node.name.value}"` : node.operation;
+      return node.name ? displayNodeName(node) : node.operation;
   }
   return '';
 }