From 4c65fcc3977d8e37beb5459d6a1457a7fc0171c2 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 7 Apr 2024 10:58:01 -0500 Subject: [PATCH 1/4] [no-for-in-array] refine report location --- .../src/rules/no-for-in-array.ts | 20 ++++- .../tests/rules/no-for-in-array.test.ts | 89 +++++++++++++++++-- 2 files changed, 102 insertions(+), 7 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 index ad70180f3570..8940f2953d64 100644 --- a/packages/eslint-plugin/src/rules/no-for-in-array.ts +++ b/packages/eslint-plugin/src/rules/no-for-in-array.ts @@ -1,3 +1,4 @@ +import type { TSESTree } from '@typescript-eslint/utils'; import * as ts from 'typescript'; import { @@ -5,6 +6,7 @@ import { getConstrainedTypeAtLocation, getParserServices, isTypeArrayTypeOrUnionOfArrayTypes, + nullThrows, } from '../util'; export default createRule({ @@ -24,6 +26,22 @@ export default createRule({ }, defaultOptions: [], create(context) { + function getReportLoc( + forInNode: TSESTree.ForInStatement, + ): TSESTree.SourceLocation { + const closingParens = nullThrows( + context.sourceCode.getTokenBefore( + forInNode.body, + token => token.value === ')', + ), + 'for-in must have a closing parenthesis.', + ); + return { + start: structuredClone(forInNode.loc.start), + end: structuredClone(closingParens.loc.end), + }; + } + return { ForInStatement(node): void { const services = getParserServices(context); @@ -36,7 +54,7 @@ export default createRule({ (type.flags & ts.TypeFlags.StringLike) !== 0 ) { context.report({ - node, + loc: getReportLoc(node), messageId: 'forInViolation', }); } 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 index cb0fff64e361..c41d0c548dda 100644 --- 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,4 +1,4 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; +import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import rule from '../../src/rules/no-for-in-array'; @@ -38,7 +38,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 +55,10 @@ for (const x in z) { errors: [ { messageId: 'forInViolation', - type: AST_NODE_TYPES.ForInStatement, + line: 3, + column: 1, + endLine: 3, + endColumn: 19, }, ], }, @@ -67,7 +73,10 @@ const fn = (arr: number[]) => { errors: [ { messageId: 'forInViolation', - type: AST_NODE_TYPES.ForInStatement, + line: 3, + column: 3, + endLine: 3, + endColumn: 23, }, ], }, @@ -82,7 +91,10 @@ const fn = (arr: number[] | string[]) => { errors: [ { messageId: 'forInViolation', - type: AST_NODE_TYPES.ForInStatement, + line: 3, + column: 3, + endLine: 3, + endColumn: 23, }, ], }, @@ -97,7 +109,72 @@ const fn = (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, }, ], }, From 0e407fea759c292aad110e010f78f42ae74ef6bf Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 7 Apr 2024 11:23:31 -0500 Subject: [PATCH 2/4] move code to util --- .../src/rules/no-for-in-array.ts | 21 ++---------- .../src/util/getForStatementHeadLoc.ts | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 packages/eslint-plugin/src/util/getForStatementHeadLoc.ts diff --git a/packages/eslint-plugin/src/rules/no-for-in-array.ts b/packages/eslint-plugin/src/rules/no-for-in-array.ts index 8940f2953d64..6d104b639ebe 100644 --- a/packages/eslint-plugin/src/rules/no-for-in-array.ts +++ b/packages/eslint-plugin/src/rules/no-for-in-array.ts @@ -1,4 +1,3 @@ -import type { TSESTree } from '@typescript-eslint/utils'; import * as ts from 'typescript'; import { @@ -6,8 +5,8 @@ import { getConstrainedTypeAtLocation, getParserServices, isTypeArrayTypeOrUnionOfArrayTypes, - nullThrows, } from '../util'; +import { getForStatementHeadLoc } from '../util/getForStatementHeadLoc'; export default createRule({ name: 'no-for-in-array', @@ -26,22 +25,6 @@ export default createRule({ }, defaultOptions: [], create(context) { - function getReportLoc( - forInNode: TSESTree.ForInStatement, - ): TSESTree.SourceLocation { - const closingParens = nullThrows( - context.sourceCode.getTokenBefore( - forInNode.body, - token => token.value === ')', - ), - 'for-in must have a closing parenthesis.', - ); - return { - start: structuredClone(forInNode.loc.start), - end: structuredClone(closingParens.loc.end), - }; - } - return { ForInStatement(node): void { const services = getParserServices(context); @@ -54,7 +37,7 @@ export default createRule({ (type.flags & ts.TypeFlags.StringLike) !== 0 ) { context.report({ - loc: getReportLoc(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 new file mode 100644 index 000000000000..8a7711fbe0b6 --- /dev/null +++ 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), + }; +} From 72e8c2dfea12964149ae99da50526eb1be97b60d Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 7 Apr 2024 11:36:43 -0500 Subject: [PATCH 3/4] lint check --- packages/eslint-plugin/tests/rules/no-for-in-array.test.ts | 1 - 1 file changed, 1 deletion(-) 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 index c41d0c548dda..35b1e1ae23a9 100644 --- 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 { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import rule from '../../src/rules/no-for-in-array'; import { getFixturesRootDir } from '../RuleTester'; From 4de9b2a9dd63cd71d83593207068cc5536138c71 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 9 Apr 2024 12:31:27 -0500 Subject: [PATCH 4/4] update snapshot --- .../docs-eslint-output-snapshots/no-for-in-array.shot | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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 index e5f4312e8cf0..dd18cfc5434d 100644 --- 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 @@ -6,18 +6,14 @@ exports[`Validating rule docs no-for-in-array.mdx code examples ESLint output 1` declare const array: string[]; for (const i in array) { -~~~~~~~~~~~~~~~~~~~~~~~~ For-in loops over arrays skips holes, returns indices as strings, and may visit the prototype chain or other enumerable properties. Use a more robust iteration method such as for-of or array.forEach instead. +~~~~~~~~~~~~~~~~~~~~~~ For-in loops over arrays skips holes, returns indices as strings, and may visit the prototype chain or other enumerable properties. Use a more robust iteration method such as for-of or array.forEach instead. console.log(array[i]); -~~~~~~~~~~~~~~~~~~~~~~~~ } -~ for (const i in array) { -~~~~~~~~~~~~~~~~~~~~~~~~ For-in loops over arrays skips holes, returns indices as strings, and may visit the prototype chain or other enumerable properties. Use a more robust iteration method such as for-of or array.forEach instead. +~~~~~~~~~~~~~~~~~~~~~~ For-in loops over arrays skips holes, returns indices as strings, and may visit the prototype chain or other enumerable properties. Use a more robust iteration method such as for-of or array.forEach instead. console.log(i, array[i]); -~~~~~~~~~~~~~~~~~~~~~~~~~~~ } -~ " `;