fix(eslint-plugin): [no-for-in-array] refine report location (#8874)

* [no-for-in-array] refine report location * move code to util * lint check * update snapshot
typescript-eslint · Apr 22, 2024 · fdeba42 · fdeba42
1 parent eef257b
commit fdeba42
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 14 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-for-in-array.ts b/packages/eslint-plugin/src/rules/no-for-in-array.ts
@@ -6,6 +6,7 @@ import {
   getParserServices,
   isTypeArrayTypeOrUnionOfArrayTypes,
 } from '../util';
+import { getForStatementHeadLoc } from '../util/getForStatementHeadLoc';
 
 export default createRule({
   name: 'no-for-in-array',
@@ -36,7 +37,7 @@ export default createRule({
           (type.flags & ts.TypeFlags.StringLike) !== 0
         ) {
           context.report({
-            node,
+            loc: getForStatementHeadLoc(context.sourceCode, node),
             messageId: 'forInViolation',
           });
         }

diff --git a/packages/eslint-plugin/src/util/getForStatementHeadLoc.ts b/packages/eslint-plugin/src/util/getForStatementHeadLoc.ts
@@ -0,0 +1,34 @@
+import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
+import { nullThrows } from '@typescript-eslint/utils/eslint-utils';
+
+/**
+ * Gets the location of the head of the given for statement variant for reporting.
+ *
+ * - `for (const foo in bar) expressionOrBlock`
+ *    ^^^^^^^^^^^^^^^^^^^^^^
+ *
+ * - `for (const foo of bar) expressionOrBlock`
+ *    ^^^^^^^^^^^^^^^^^^^^^^
+ *
+ * - `for await (const foo of bar) expressionOrBlock`
+ *    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ *
+ * - `for (let i = 0; i < 10; i++) expressionOrBlock`
+ *    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ */
+export function getForStatementHeadLoc(
+  sourceCode: TSESLint.SourceCode,
+  node:
+    | TSESTree.ForInStatement
+    | TSESTree.ForOfStatement
+    | TSESTree.ForStatement,
+): TSESTree.SourceLocation {
+  const closingParens = nullThrows(
+    sourceCode.getTokenBefore(node.body, token => token.value === ')'),
+    'for statement must have a closing parenthesis.',
+  );
+  return {
+    start: structuredClone(node.loc.start),
+    end: structuredClone(closingParens.loc.end),
+  };
+}
diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-for-in-array.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-for-in-array.shot
diff --git a/packages/eslint-plugin/tests/rules/no-for-in-array.test.ts b/packages/eslint-plugin/tests/rules/no-for-in-array.test.ts
@@ -1,5 +1,4 @@
-import { RuleTester } from '@typescript-eslint/rule-tester';
-import { AST_NODE_TYPES } from '@typescript-eslint/utils';
+import { noFormat, RuleTester } from '@typescript-eslint/rule-tester';
 
 import rule from '../../src/rules/no-for-in-array';
 import { getFixturesRootDir } from '../RuleTester';
@@ -38,7 +37,10 @@ for (const x in [3, 4, 5]) {
       errors: [
         {
           messageId: 'forInViolation',
-          type: AST_NODE_TYPES.ForInStatement,
+          line: 2,
+          column: 1,
+          endLine: 2,
+          endColumn: 27,
         },
       ],
     },
@@ -52,7 +54,10 @@ for (const x in z) {
       errors: [
         {
           messageId: 'forInViolation',
-          type: AST_NODE_TYPES.ForInStatement,
+          line: 3,
+          column: 1,
+          endLine: 3,
+          endColumn: 19,
         },
       ],
     },
@@ -67,7 +72,10 @@ const fn = (arr: number[]) => {
       errors: [
         {
           messageId: 'forInViolation',
-          type: AST_NODE_TYPES.ForInStatement,
+          line: 3,
+          column: 3,
+          endLine: 3,
+          endColumn: 23,
         },
       ],
     },
@@ -82,7 +90,10 @@ const fn = (arr: number[] | string[]) => {
       errors: [
         {
           messageId: 'forInViolation',
-          type: AST_NODE_TYPES.ForInStatement,
+          line: 3,
+          column: 3,
+          endLine: 3,
+          endColumn: 23,
         },
       ],
     },
@@ -97,7 +108,72 @@ const fn = <T extends any[]>(arr: T) => {
       errors: [
         {
           messageId: 'forInViolation',
-          type: AST_NODE_TYPES.ForInStatement,
+          line: 3,
+          column: 3,
+          endLine: 3,
+          endColumn: 23,
+        },
+      ],
+    },
+    {
+      code: noFormat`
+for (const x
+  in
+    (
+      (
+        (
+          [3, 4, 5]
+        )
+      )
+    )
+  )
+  // weird
+  /* spot for a */
+  // comment
+  /* ) */
+  /* ( */
+  {
+  console.log(x);
+}
+      `,
+      errors: [
+        {
+          messageId: 'forInViolation',
+          line: 2,
+          column: 1,
+          endLine: 11,
+          endColumn: 4,
+        },
+      ],
+    },
+    {
+      code: noFormat`
+for (const x
+  in
+    (
+      (
+        (
+          [3, 4, 5]
+        )
+      )
+    )
+  )
+  // weird
+  /* spot for a */
+  // comment
+  /* ) */
+  /* ( */
+
+  ((((console.log('body without braces ')))));
+
+      `,
+      errors: [
+        {
+          messageId: 'forInViolation',
+          line: 2,
+          column: 1,
+          endLine: 11,
+          endColumn: 4,
         },
       ],
     },