fix(no-hashtag-description): allow hashtag comments between fields and arguments (#588)

Dimitri POSTOLOV · web-flow · commit 7b12bbfa55e9 · 2021-09-06T12:06:40.000+03:00
diff --git a/.changeset/two-bees-chew.md b/.changeset/two-bees-chew.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+fix(no-hashtag-description): allow hashtag comments between fields and arguments
diff --git a/packages/plugin/src/estree-parser/converter.ts b/packages/plugin/src/estree-parser/converter.ts
@@ -7,12 +7,10 @@ export function convertToESTree<T extends ASTNode>(
   node: T,
   typeInfo?: TypeInfo
 ): { rootTree: GraphQLESTreeNode<T>; comments: Comment[] } {
-  const comments = extractCommentsFromAst(node);
   const visitor = { leave: convertNode(typeInfo) };
-
   return {
     rootTree: visit(node, typeInfo ? visitWithTypeInfo(typeInfo, visitor) : visitor),
-    comments,
+    comments: extractCommentsFromAst(node.loc),
   };
 }
 
diff --git a/packages/plugin/src/estree-parser/utils.ts b/packages/plugin/src/estree-parser/utils.ts
@@ -69,33 +69,28 @@ export function convertRange(gqlLocation: Location): [number, number] {
   return [gqlLocation.start, gqlLocation.end];
 }
 
-export function extractCommentsFromAst(node: ASTNode): Comment[] {
-  const { loc } = node;
-
+export function extractCommentsFromAst(loc: Location): Comment[] {
   if (!loc) {
     return [];
   }
-
   const comments: Comment[] = [];
   let token = loc.startToken;
 
   while (token !== null) {
-    if (token.kind === TokenKind.COMMENT) {
-      const { line, column } = token;
+    const { kind, value, line, column, start, end, next } = token;
+    if (kind === TokenKind.COMMENT) {
       comments.push({
         type: 'Block',
-        value: ` ${token.value} `,
+        value,
         loc: {
           start: { line, column },
           end: { line, column },
         },
-        range: [token.start, token.end],
+        range: [start, end],
       });
     }
-
-    token = token.next;
+    token = next;
   }
-
   return comments;
 }
 
diff --git a/packages/plugin/src/rules/no-hashtag-description.ts b/packages/plugin/src/rules/no-hashtag-description.ts
@@ -1,19 +1,19 @@
-import { GraphQLESLintRule } from '../types';
 import { TokenKind } from 'graphql';
-import { checkForEslint } from '../utils';
+import { GraphQLESLintRule } from '../types';
 
 const HASHTAG_COMMENT = 'HASHTAG_COMMENT';
 
 const rule: GraphQLESLintRule = {
   meta: {
     messages: {
-      [HASHTAG_COMMENT]: `Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.`,
+      [HASHTAG_COMMENT]:
+        'Using hashtag (#) for adding GraphQL descriptions is not allowed. Prefer using """ for multiline, or " for a single line description.',
     },
     docs: {
       description:
         'Requires to use `"""` or `"` for adding a GraphQL description instead of `#`.\nAllows to use hashtag for comments, as long as it\'s not attached to an AST definition.',
       category: 'Best Practices',
-      url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/no-hashtag-description.md`,
+      url: 'https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/no-hashtag-description.md',
       examples: [
         {
           title: 'Incorrect',
@@ -55,60 +55,27 @@ const rule: GraphQLESLintRule = {
   create(context) {
     return {
       Document(node) {
-        if (node) {
-          const rawNode = node.rawNode();
+        const rawNode = node.rawNode();
+        let token = rawNode.loc.startToken;
 
-          if (rawNode && rawNode.loc && rawNode.loc.startToken) {
-            let token = rawNode.loc.startToken;
+        while (token !== null) {
+          const { kind, prev, next, value, line, column } = token;
 
-            while (token !== null) {
-              if (token.kind === TokenKind.COMMENT && token.next && token.prev) {
-                if (
-                  token.prev.kind !== TokenKind.SOF &&
-                  token.prev.kind !== TokenKind.COMMENT &&
-                  token.next.kind !== TokenKind.COMMENT &&
-                  token.next.line - token.line > 1 &&
-                  token.prev.line !== token.line
-                ) {
-                  context.report({
-                    messageId: HASHTAG_COMMENT,
-                    loc: {
-                      start: {
-                        line: token.line,
-                        column: token.column,
-                      },
-                      end: {
-                        line: token.line,
-                        column: token.column,
-                      },
-                    },
-                  });
-                } else if (
-                  token.next.kind !== TokenKind.COMMENT &&
-                  !checkForEslint(token, rawNode) &&
-                  token.next.kind !== TokenKind.EOF &&
-                  token.next.line - token.line < 2 &&
-                  token.prev.line !== token.line
-                ) {
-                  context.report({
-                    messageId: HASHTAG_COMMENT,
-                    loc: {
-                      start: {
-                        line: token.line,
-                        column: token.column,
-                      },
-                      end: {
-                        line: token.line,
-                        column: token.column,
-                      },
-                    },
-                  });
-                }
-              }
+          if (kind === TokenKind.COMMENT && prev && next) {
+            const isEslintComment = value.trimLeft().startsWith('eslint');
+            const linesAfter = next.line - line;
 
-              token = token.next;
+            if (!isEslintComment && line !== prev.line && next.kind === TokenKind.NAME && linesAfter < 2) {
+              context.report({
+                messageId: HASHTAG_COMMENT,
+                loc: {
+                  start: { line, column },
+                  end: { line, column },
+                },
+              });
             }
           }
+          token = next;
         }
       },
     };
diff --git a/packages/plugin/src/testkit.ts b/packages/plugin/src/testkit.ts
@@ -39,7 +39,7 @@ export class GraphQLRuleTester extends require('eslint').RuleTester {
     name: string,
     rule: GraphQLESLintRule,
     tests: {
-      valid: GraphQLValidTestCase<Config>[];
+      valid: (string | GraphQLValidTestCase<Config>)[];
       invalid: GraphQLInvalidTestCase<Config>[];
     }
   ): void {
diff --git a/packages/plugin/src/utils.ts b/packages/plugin/src/utils.ts
@@ -102,17 +102,6 @@ export function extractTokens(source: string): AST.Token[] {
   return tokens;
 }
 
-export function checkForEslint(token: Token, rawNode: DocumentNode): boolean {
-  if (token.kind !== 'Comment') {
-    return false;
-  }
-  const string = rawNode.loc?.source.body.substring(token.start + 1, token.start + 8);
-  if (string.toLocaleLowerCase().includes('eslint')) {
-    return true;
-  }
-  return false;
-}
-
 export const normalizePath = (path: string): string => (path || '').replace(/\\/g, '/');
 
 /**
@@ -152,4 +141,4 @@ export const loaderCache: Record<string, LoaderSource[]> = new Proxy(Object.crea
     }
     return true;
   }
-});
+});
diff --git a/packages/plugin/tests/no-hashtag-description.spec.ts b/packages/plugin/tests/no-hashtag-description.spec.ts
@@ -1,104 +1,123 @@
-import { GraphQLRuleTester } from '../src/testkit';
+import { GraphQLRuleTester } from '../src';
 import rule from '../src/rules/no-hashtag-description';
-import { Kind } from 'graphql';
 
 const ruleTester = new GraphQLRuleTester();
 
 ruleTester.runGraphQLTests('no-hashtag-description', rule, {
   valid: [
-    {
-      code: /* GraphQL */ `
-        " test "
-        type Query {
-          foo: String
-        }
-      `,
-    },
-    {
-      code: /* GraphQL */ `
-        # Test
+    /* GraphQL */ `
+      " Good "
+      type Query {
+        foo: String
+      }
+    `,
+    /* GraphQL */ `
+      # Good
 
-        type Query {
-          foo: String
-        }
-      `,
-    },
-    {
-      code: `#import t
+      type Query {
+        foo: String
+      }
+      # Good
+    `,
+    /* GraphQL */ `
+      #import t
 
-        type Query {
-          foo: String
-        }
-      `,
-    },
-    {
-      code: /* GraphQL */ `
-        # multiline
-        # multiline
-        # multiline
-        # multiline
+      type Query {
+        foo: String
+      }
+    `,
+    /* GraphQL */ `
+      # multiline
+      # multiline
+      # multiline
 
-        type Query {
-          foo: String
-        }
-      `,
-    },
-    {
-      code: /* GraphQL */ `
-        type Query {
-          foo: String
-        }
+      type Query {
+        foo: String
+      }
+    `,
+    /* GraphQL */ `
+      type Query { # Good
+        foo: String # Good
+      } # Good
+    `,
+    /* GraphQL */ `
+      # eslint-disable-next-line
+      type Query {
+        foo: String
+      }
+    `,
+    /* GraphQL */ `
+      type Query {
+        # Good
 
-        # Test
-      `,
-    },
+        foo: ID
+      }
+    `,
+    /* GraphQL */ `
+      type Query {
+        foo: ID
+        # Good
+
+        bar: ID
+      }
+    `,
+    /* GraphQL */ `
+      type Query {
+        user(
+          # Good
+
+          id: Int
+        ): User
+      }
+    `,
+  ],
+  invalid: [
     {
       code: /* GraphQL */ `
+        # Bad
         type Query {
-          foo: String # this is also fine, comes after the definition
-        }
-      `,
-    },
-    {
-      code: /* GraphQL */ `
-        type Query { # this is also fine, comes after the definition
           foo: String
-        } # this is also fine, comes after the definition
+        }
       `,
+      errors: [{ messageId: 'HASHTAG_COMMENT' }],
     },
     {
       code: /* GraphQL */ `
+        # multiline
+        # multiline
         type Query {
           foo: String
         }
-
-        # Test
       `,
+      errors: [{ messageId: 'HASHTAG_COMMENT' }],
     },
     {
       code: /* GraphQL */ `
-        # eslint
         type Query {
+          # Bad
           foo: String
         }
       `,
+      errors: [{ messageId: 'HASHTAG_COMMENT' }],
     },
-  ],
-  invalid: [
     {
       code: /* GraphQL */ `
-        # Test
         type Query {
-          foo: String
+          bar: ID
+          # Bad
+          foo: ID
+          # Good
         }
       `,
       errors: [{ messageId: 'HASHTAG_COMMENT' }],
     },
     {
       code: /* GraphQL */ `
         type Query {
-          # Test
-          foo: String
+          user(
+            # Bad
+            id: Int!
+          ): User
         }
       `,
       errors: [{ messageId: 'HASHTAG_COMMENT' }],

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@graphql-eslint/eslint-plugin': patch
 +---
++
 +fix(no-hashtag-description): allow hashtag comments between fields and arguments
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ export class GraphQLRuleTester extends require('eslint').RuleTester {`
`39`	`39`	`name: string,`
`40`	`40`	`rule: GraphQLESLintRule,`
`41`	`41`	`tests: {`
`42`		`- valid: GraphQLValidTestCase<Config>[];`
	`42`	`+ valid: (string \| GraphQLValidTestCase<Config>)[];`
`43`	`43`	`invalid: GraphQLInvalidTestCase<Config>[];`
`44`	`44`	`}`
`45`	`45`	`): void {`