From a2b9a4aadd1dcd4e55774ea482ebc157e5372d8b Mon Sep 17 00:00:00 2001 From: StyleShit Date: Thu, 14 Dec 2023 14:57:22 +0200 Subject: [PATCH 1/9] feat(eslint-plugin): [no-array-delete] add new rule --- .../docs/rules/no-array-delete.md | 40 ++ packages/eslint-plugin/src/configs/all.ts | 1 + .../src/configs/disable-type-checked.ts | 1 + .../src/configs/strict-type-checked.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-array-delete.ts | 86 +++ .../tests/rules/no-array-delete.test.ts | 519 ++++++++++++++++++ .../schema-snapshots/no-array-delete.shot | 14 + 8 files changed, 664 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-array-delete.md create mode 100644 packages/eslint-plugin/src/rules/no-array-delete.ts create mode 100644 packages/eslint-plugin/tests/rules/no-array-delete.test.ts create mode 100644 packages/eslint-plugin/tests/schema-snapshots/no-array-delete.shot diff --git a/packages/eslint-plugin/docs/rules/no-array-delete.md b/packages/eslint-plugin/docs/rules/no-array-delete.md new file mode 100644 index 000000000000..172b3083b16b --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-array-delete.md @@ -0,0 +1,40 @@ +--- +description: 'Disallow the delete operator with array expressions.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-array-delete** for documentation. + +When using the `delete` keyword with an array expression, the array's `length` property is not affected, +but the element at the specified index is removed and leaves an empty slot in the array. +This is likely to lead to unexpected behavior. As mentioned in the +[MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete#deleting_array_elements), +the recommended way to remove an element from an array is by using the +[`Array#splice`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice) method. + +## Examples + + + +### ❌ Incorrect + +```ts +declare const arr: number[]; + +delete arr[0]; +``` + +### ✅ Correct + +```ts +declare const arr: number[]; + +arr.splice(0, 1); +``` + + + +## When Not To Use It + +When you want to allow the delete operator with array expressions. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index f6e14dfe12a5..558326fdf67f 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -60,6 +60,7 @@ export = { '@typescript-eslint/naming-convention': 'error', 'no-array-constructor': 'off', '@typescript-eslint/no-array-constructor': 'error', + '@typescript-eslint/no-array-delete': 'error', '@typescript-eslint/no-base-to-string': 'error', '@typescript-eslint/no-confusing-non-null-assertion': 'error', '@typescript-eslint/no-confusing-void-expression': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 2fe413146c7b..cda1dfff63f1 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -12,6 +12,7 @@ export = { '@typescript-eslint/consistent-type-exports': 'off', '@typescript-eslint/dot-notation': 'off', '@typescript-eslint/naming-convention': 'off', + '@typescript-eslint/no-array-delete': 'off', '@typescript-eslint/no-base-to-string': 'off', '@typescript-eslint/no-confusing-void-expression': 'off', '@typescript-eslint/no-duplicate-type-constituents': 'off', diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts index 471175b9bba7..9d16308c00a5 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked.ts @@ -13,6 +13,7 @@ export = { '@typescript-eslint/ban-types': 'error', 'no-array-constructor': 'off', '@typescript-eslint/no-array-constructor': 'error', + '@typescript-eslint/no-array-delete': 'error', '@typescript-eslint/no-base-to-string': 'error', '@typescript-eslint/no-confusing-void-expression': 'error', '@typescript-eslint/no-duplicate-enum-values': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 14c171af990e..6d6e8e6613d1 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -34,6 +34,7 @@ import memberOrdering from './member-ordering'; import methodSignatureStyle from './method-signature-style'; import namingConvention from './naming-convention'; import noArrayConstructor from './no-array-constructor'; +import noArrayDelete from './no-array-delete'; import noBaseToString from './no-base-to-string'; import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion'; import noConfusingVoidExpression from './no-confusing-void-expression'; @@ -173,6 +174,7 @@ export default { 'method-signature-style': methodSignatureStyle, 'naming-convention': namingConvention, 'no-array-constructor': noArrayConstructor, + 'no-array-delete': noArrayDelete, 'no-base-to-string': noBaseToString, 'no-confusing-non-null-assertion': confusingNonNullAssertionLikeNotEqual, 'no-confusing-void-expression': noConfusingVoidExpression, diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts new file mode 100644 index 000000000000..286200791c2a --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -0,0 +1,86 @@ +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import type * as ts from 'typescript'; + +import { createRule, getParserServices } from '../util'; + +type MessageId = 'noArrayDelete' | 'useSplice'; + +export default createRule<[], MessageId>({ + name: 'no-array-delete', + meta: { + hasSuggestions: true, + type: 'problem', + docs: { + description: 'Disallow the delete operator with array expressions', + recommended: 'strict', + requiresTypeChecking: true, + }, + messages: { + noArrayDelete: + 'Using the `delete` operator with an array expression is unsafe.', + useSplice: 'Use `{{ target }}.splice({{ key }}, 1)` instead.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const services = getParserServices(context); + const checker = services.program.getTypeChecker(); + + function isUnderlyingTypeArray(type: ts.Type): boolean { + const isArray = (t: ts.Type): boolean => checker.isArrayType(t); + + if (type.isUnion()) { + return type.types.every(isArray); + } + + if (type.isIntersection()) { + return type.types.some(isArray); + } + + return isArray(type); + } + + return { + 'UnaryExpression[operator="delete"]'( + node: TSESTree.UnaryExpression, + ): void { + if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { + return; + } + + const type = services.getTypeAtLocation(node.argument.object); + + if (!isUnderlyingTypeArray(type)) { + return; + } + + const shouldHaveParentheses = + node.argument.property.type === AST_NODE_TYPES.SequenceExpression; + + const nodeMap = services.esTreeNodeToTSNodeMap; + const target = nodeMap.get(node.argument.object).getText(); + const rawKey = nodeMap.get(node.argument.property).getText(); + const key = shouldHaveParentheses ? `(${rawKey})` : rawKey; + + context.report({ + node, + messageId: 'noArrayDelete', + suggest: [ + { + messageId: 'useSplice', + data: { + key, + target, + }, + fix(fixer): TSESLint.RuleFix | null { + return fixer.replaceText(node, `${target}.splice(${key}, 1)`); + }, + }, + ], + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts new file mode 100644 index 000000000000..aaee3af38a4b --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts @@ -0,0 +1,519 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/no-array-delete'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootPath = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('no-array-delete', rule, { + valid: [ + ` + declare const obj: { a: 1; b: 2 }; + delete obj.a; + `, + + ` + declare const obj: { a: 1; b: 2 }; + delete obj['a']; + `, + + ` + declare const arr: { a: 1; b: 2 }[][][][]; + delete arr[0][0][0][0].a; + `, + + ` + declare const maybeArray: any; + delete maybeArray[0]; + `, + + ` + declare const maybeArray: unknown; + delete maybeArray[0]; + `, + + ` + declare function getObject(): T; + delete getObject().a; + `, + + ` + declare function getObject(): { a: T; b: 2 }; + delete getObject().a; + `, + ], + + invalid: [ + { + code: ` + declare const arr: number[]; + delete arr[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'arr', + }, + output: ` + declare const arr: number[]; + arr.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: number[]; + declare const key: number; + delete arr[key]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 4, + column: 9, + endColumn: 24, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: 'key', + target: 'arr', + }, + output: ` + declare const arr: number[]; + declare const key: number; + arr.splice(key, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: number[]; + + enum Keys { + A, + B, + } + + delete arr[Keys.A]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 9, + column: 9, + endColumn: 27, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: 'Keys.A', + target: 'arr', + }, + output: ` + declare const arr: number[]; + + enum Keys { + A, + B, + } + + arr.splice(Keys.A, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: number[]; + declare function doWork(): void; + delete arr[(doWork(), 1)]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 4, + column: 9, + endColumn: 34, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '(doWork(), 1)', + target: 'arr', + }, + output: ` + declare const arr: number[]; + declare function doWork(): void; + arr.splice((doWork(), 1), 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: Array; + delete arr[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'arr', + }, + output: ` + declare const arr: Array; + arr.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: 'delete [1, 2, 3][0];', + errors: [ + { + messageId: 'noArrayDelete', + line: 1, + column: 1, + endColumn: 20, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: '[1, 2, 3]', + }, + output: '[1, 2, 3].splice(0, 1);', + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: unknown[]; + delete arr[Math.random() ? 0 : 1]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 42, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: 'Math.random() ? 0 : 1', + target: 'arr', + }, + output: ` + declare const arr: unknown[]; + arr.splice(Math.random() ? 0 : 1, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: number[] | string[] | boolean[]; + delete arr[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'arr', + }, + output: ` + declare const arr: number[] | string[] | boolean[]; + arr.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: number[] & unknown; + delete arr[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'arr', + }, + output: ` + declare const arr: number[] & unknown; + arr.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const arr: (number | string)[]; + delete arr[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'arr', + }, + output: ` + declare const arr: (number | string)[]; + arr.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const obj: { a: { b: { c: number[] } } }; + delete obj.a.b.c[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 28, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'obj.a.b.c', + }, + output: ` + declare const obj: { a: { b: { c: number[] } } }; + obj.a.b.c.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare function getArray(): T; + delete getArray()[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 29, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'getArray()', + }, + output: ` + declare function getArray(): T; + getArray().splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare function getArray(): T[]; + delete getArray()[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 29, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'getArray()', + }, + output: ` + declare function getArray(): T[]; + getArray().splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + function deleteFromArray(a: number[]) { + delete a[0]; + } + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 11, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'a', + }, + output: ` + function deleteFromArray(a: number[]) { + a.splice(0, 1); + } + `, + }, + ], + }, + ], + }, + + { + code: ` + function deleteFromArray(a: T[]) { + delete a[0]; + } + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 11, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'a', + }, + output: ` + function deleteFromArray(a: T[]) { + a.splice(0, 1); + } + `, + }, + ], + }, + ], + }, + + { + code: ` + function deleteFromArray(a: T) { + delete a[0]; + } + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 11, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + data: { + key: '0', + target: 'a', + }, + output: ` + function deleteFromArray(a: T) { + a.splice(0, 1); + } + `, + }, + ], + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-array-delete.shot b/packages/eslint-plugin/tests/schema-snapshots/no-array-delete.shot new file mode 100644 index 000000000000..c8dc106464a0 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/no-array-delete.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes no-array-delete 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; From 1a8132964da2f47171bc6001bf996a0bc20d311f Mon Sep 17 00:00:00 2001 From: StyleShit Date: Thu, 14 Dec 2023 15:02:53 +0200 Subject: [PATCH 2/9] small refactor --- packages/eslint-plugin/src/rules/no-array-delete.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts index 286200791c2a..0d46a7f459cd 100644 --- a/packages/eslint-plugin/src/rules/no-array-delete.ts +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -29,17 +29,15 @@ export default createRule<[], MessageId>({ const checker = services.program.getTypeChecker(); function isUnderlyingTypeArray(type: ts.Type): boolean { - const isArray = (t: ts.Type): boolean => checker.isArrayType(t); - if (type.isUnion()) { - return type.types.every(isArray); + return type.types.every(checker.isArrayType); } if (type.isIntersection()) { - return type.types.some(isArray); + return type.types.some(checker.isArrayType); } - return isArray(type); + return checker.isArrayType(type); } return { @@ -74,7 +72,7 @@ export default createRule<[], MessageId>({ key, target, }, - fix(fixer): TSESLint.RuleFix | null { + fix(fixer): TSESLint.RuleFix { return fixer.replaceText(node, `${target}.splice(${key}, 1)`); }, }, From 6d098b804511225957b96ffc448309418994684d Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 17 Dec 2023 19:53:39 +0200 Subject: [PATCH 3/9] add more cases --- .../docs/rules/no-array-delete.md | 2 +- .../src/rules/no-array-delete.ts | 66 ++-- .../tests/rules/no-array-delete.test.ts | 304 ++++++------------ 3 files changed, 129 insertions(+), 243 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-array-delete.md b/packages/eslint-plugin/docs/rules/no-array-delete.md index 172b3083b16b..15a04c7ece31 100644 --- a/packages/eslint-plugin/docs/rules/no-array-delete.md +++ b/packages/eslint-plugin/docs/rules/no-array-delete.md @@ -1,5 +1,5 @@ --- -description: 'Disallow the delete operator with array expressions.' +description: 'Disallow using the `delete` operator on array values.' --- > 🛑 This file is source code, not the primary documentation location! 🛑 diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts index 0d46a7f459cd..c192758b75d9 100644 --- a/packages/eslint-plugin/src/rules/no-array-delete.ts +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -2,24 +2,27 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import type * as ts from 'typescript'; -import { createRule, getParserServices } from '../util'; +import { + createRule, + getConstrainedTypeAtLocation, + getParserServices, +} from '../util'; -type MessageId = 'noArrayDelete' | 'useSplice'; +type MessageId = 'noArrayDelete'; export default createRule<[], MessageId>({ name: 'no-array-delete', meta: { - hasSuggestions: true, + fixable: 'code', type: 'problem', docs: { - description: 'Disallow the delete operator with array expressions', + description: 'Disallow using the `delete` operator on array values', recommended: 'strict', requiresTypeChecking: true, }, messages: { noArrayDelete: - 'Using the `delete` operator with an array expression is unsafe.', - useSplice: 'Use `{{ target }}.splice({{ key }}, 1)` instead.', + 'Using the `delete` operator with an array expression is unsafe. Use array.splice()` instead.', }, schema: [], }, @@ -29,15 +32,18 @@ export default createRule<[], MessageId>({ const checker = services.program.getTypeChecker(); function isUnderlyingTypeArray(type: ts.Type): boolean { + const predicate = (t: ts.Type): boolean => + checker.isArrayType(t) || checker.isTupleType(t); + if (type.isUnion()) { - return type.types.every(checker.isArrayType); + return type.types.every(predicate); } if (type.isIntersection()) { - return type.types.some(checker.isArrayType); + return type.types.some(predicate); } - return checker.isArrayType(type); + return predicate(type); } return { @@ -48,35 +54,35 @@ export default createRule<[], MessageId>({ return; } - const type = services.getTypeAtLocation(node.argument.object); + const type = getConstrainedTypeAtLocation( + services, + node.argument.object, + ); if (!isUnderlyingTypeArray(type)) { return; } - const shouldHaveParentheses = - node.argument.property.type === AST_NODE_TYPES.SequenceExpression; - - const nodeMap = services.esTreeNodeToTSNodeMap; - const target = nodeMap.get(node.argument.object).getText(); - const rawKey = nodeMap.get(node.argument.property).getText(); - const key = shouldHaveParentheses ? `(${rawKey})` : rawKey; - context.report({ node, messageId: 'noArrayDelete', - suggest: [ - { - messageId: 'useSplice', - data: { - key, - target, - }, - fix(fixer): TSESLint.RuleFix { - return fixer.replaceText(node, `${target}.splice(${key}, 1)`); - }, - }, - ], + fix(fixer): TSESLint.RuleFix | null { + if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { + return null; + } + + const { object, property } = node.argument; + + const shouldHaveParentheses = + property.type === AST_NODE_TYPES.SequenceExpression; + + const nodeMap = services.esTreeNodeToTSNodeMap; + const target = nodeMap.get(object).getText(); + const rawKey = nodeMap.get(property).getText(); + const key = shouldHaveParentheses ? `(${rawKey})` : rawKey; + + return fixer.replaceText(node, `${target}.splice(${key}, 1)`); + }, }); }, }; diff --git a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts index aaee3af38a4b..fd3e67b1f965 100644 --- a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts @@ -49,6 +49,11 @@ ruleTester.run('no-array-delete', rule, { declare function getObject(): { a: T; b: 2 }; delete getObject().a; `, + + ` + declare const test: never; + delete test[0]; + `, ], invalid: [ @@ -57,25 +62,16 @@ ruleTester.run('no-array-delete', rule, { declare const arr: number[]; delete arr[0]; `, + output: ` + declare const arr: number[]; + arr.splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'arr', - }, - output: ` - declare const arr: number[]; - arr.splice(0, 1); - `, - }, - ], }, ], }, @@ -86,26 +82,17 @@ ruleTester.run('no-array-delete', rule, { declare const key: number; delete arr[key]; `, + output: ` + declare const arr: number[]; + declare const key: number; + arr.splice(key, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 4, column: 9, endColumn: 24, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: 'key', - target: 'arr', - }, - output: ` - declare const arr: number[]; - declare const key: number; - arr.splice(key, 1); - `, - }, - ], }, ], }, @@ -121,20 +108,7 @@ ruleTester.run('no-array-delete', rule, { delete arr[Keys.A]; `, - errors: [ - { - messageId: 'noArrayDelete', - line: 9, - column: 9, - endColumn: 27, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: 'Keys.A', - target: 'arr', - }, - output: ` + output: ` declare const arr: number[]; enum Keys { @@ -144,8 +118,12 @@ ruleTester.run('no-array-delete', rule, { arr.splice(Keys.A, 1); `, - }, - ], + errors: [ + { + messageId: 'noArrayDelete', + line: 9, + column: 9, + endColumn: 27, }, ], }, @@ -156,26 +134,17 @@ ruleTester.run('no-array-delete', rule, { declare function doWork(): void; delete arr[(doWork(), 1)]; `, + output: ` + declare const arr: number[]; + declare function doWork(): void; + arr.splice((doWork(), 1), 1); + `, errors: [ { messageId: 'noArrayDelete', line: 4, column: 9, endColumn: 34, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '(doWork(), 1)', - target: 'arr', - }, - output: ` - declare const arr: number[]; - declare function doWork(): void; - arr.splice((doWork(), 1), 1); - `, - }, - ], }, ], }, @@ -185,47 +154,29 @@ ruleTester.run('no-array-delete', rule, { declare const arr: Array; delete arr[0]; `, + output: ` + declare const arr: Array; + arr.splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'arr', - }, - output: ` - declare const arr: Array; - arr.splice(0, 1); - `, - }, - ], }, ], }, { code: 'delete [1, 2, 3][0];', + output: '[1, 2, 3].splice(0, 1);', errors: [ { messageId: 'noArrayDelete', line: 1, column: 1, endColumn: 20, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: '[1, 2, 3]', - }, - output: '[1, 2, 3].splice(0, 1);', - }, - ], }, ], }, @@ -235,25 +186,16 @@ ruleTester.run('no-array-delete', rule, { declare const arr: unknown[]; delete arr[Math.random() ? 0 : 1]; `, + output: ` + declare const arr: unknown[]; + arr.splice(Math.random() ? 0 : 1, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 42, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: 'Math.random() ? 0 : 1', - target: 'arr', - }, - output: ` - declare const arr: unknown[]; - arr.splice(Math.random() ? 0 : 1, 1); - `, - }, - ], }, ], }, @@ -263,25 +205,16 @@ ruleTester.run('no-array-delete', rule, { declare const arr: number[] | string[] | boolean[]; delete arr[0]; `, + output: ` + declare const arr: number[] | string[] | boolean[]; + arr.splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'arr', - }, - output: ` - declare const arr: number[] | string[] | boolean[]; - arr.splice(0, 1); - `, - }, - ], }, ], }, @@ -291,25 +224,16 @@ ruleTester.run('no-array-delete', rule, { declare const arr: number[] & unknown; delete arr[0]; `, + output: ` + declare const arr: number[] & unknown; + arr.splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'arr', - }, - output: ` - declare const arr: number[] & unknown; - arr.splice(0, 1); - `, - }, - ], }, ], }, @@ -319,25 +243,16 @@ ruleTester.run('no-array-delete', rule, { declare const arr: (number | string)[]; delete arr[0]; `, + output: ` + declare const arr: (number | string)[]; + arr.splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'arr', - }, - output: ` - declare const arr: (number | string)[]; - arr.splice(0, 1); - `, - }, - ], }, ], }, @@ -347,25 +262,16 @@ ruleTester.run('no-array-delete', rule, { declare const obj: { a: { b: { c: number[] } } }; delete obj.a.b.c[0]; `, + output: ` + declare const obj: { a: { b: { c: number[] } } }; + obj.a.b.c.splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 28, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'obj.a.b.c', - }, - output: ` - declare const obj: { a: { b: { c: number[] } } }; - obj.a.b.c.splice(0, 1); - `, - }, - ], }, ], }, @@ -375,25 +281,16 @@ ruleTester.run('no-array-delete', rule, { declare function getArray(): T; delete getArray()[0]; `, + output: ` + declare function getArray(): T; + getArray().splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 29, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'getArray()', - }, - output: ` - declare function getArray(): T; - getArray().splice(0, 1); - `, - }, - ], }, ], }, @@ -403,25 +300,16 @@ ruleTester.run('no-array-delete', rule, { declare function getArray(): T[]; delete getArray()[0]; `, + output: ` + declare function getArray(): T[]; + getArray().splice(0, 1); + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 29, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'getArray()', - }, - output: ` - declare function getArray(): T[]; - getArray().splice(0, 1); - `, - }, - ], }, ], }, @@ -432,26 +320,17 @@ ruleTester.run('no-array-delete', rule, { delete a[0]; } `, + output: ` + function deleteFromArray(a: number[]) { + a.splice(0, 1); + } + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 11, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'a', - }, - output: ` - function deleteFromArray(a: number[]) { - a.splice(0, 1); - } - `, - }, - ], }, ], }, @@ -462,26 +341,17 @@ ruleTester.run('no-array-delete', rule, { delete a[0]; } `, + output: ` + function deleteFromArray(a: T[]) { + a.splice(0, 1); + } + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 11, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'a', - }, - output: ` - function deleteFromArray(a: T[]) { - a.splice(0, 1); - } - `, - }, - ], }, ], }, @@ -492,26 +362,36 @@ ruleTester.run('no-array-delete', rule, { delete a[0]; } `, + output: ` + function deleteFromArray(a: T) { + a.splice(0, 1); + } + `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 11, endColumn: 22, - suggestions: [ - { - messageId: 'useSplice', - data: { - key: '0', - target: 'a', - }, - output: ` - function deleteFromArray(a: T) { - a.splice(0, 1); - } + }, + ], + }, + + { + code: ` + declare const tuple: [number, string]; + delete tuple[0]; `, - }, - ], + output: ` + declare const tuple: [number, string]; + tuple.splice(0, 1); + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 24, }, ], }, From a44d9577c3d4e3d1766bda4134fa18dd498a964d Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 17 Dec 2023 19:54:32 +0200 Subject: [PATCH 4/9] fix docs --- packages/eslint-plugin/docs/rules/no-array-delete.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-array-delete.md b/packages/eslint-plugin/docs/rules/no-array-delete.md index 15a04c7ece31..3abbbc6ececc 100644 --- a/packages/eslint-plugin/docs/rules/no-array-delete.md +++ b/packages/eslint-plugin/docs/rules/no-array-delete.md @@ -6,7 +6,7 @@ description: 'Disallow using the `delete` operator on array values.' > > See **https://typescript-eslint.io/rules/no-array-delete** for documentation. -When using the `delete` keyword with an array expression, the array's `length` property is not affected, +When using the `delete` operator with an array value, the array's `length` property is not affected, but the element at the specified index is removed and leaves an empty slot in the array. This is likely to lead to unexpected behavior. As mentioned in the [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete#deleting_array_elements), From 36250a98219ea5724481e4e3cb8720829499cdb1 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 17 Dec 2023 19:55:34 +0200 Subject: [PATCH 5/9] fix message --- packages/eslint-plugin/src/rules/no-array-delete.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts index c192758b75d9..dc89493b87d9 100644 --- a/packages/eslint-plugin/src/rules/no-array-delete.ts +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -22,7 +22,7 @@ export default createRule<[], MessageId>({ }, messages: { noArrayDelete: - 'Using the `delete` operator with an array expression is unsafe. Use array.splice()` instead.', + 'Using the `delete` operator with an array expression is unsafe. Use `array.splice()` instead.', }, schema: [], }, From 80e61fc3dfc8841288ea9228645b9270c32794ad Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sat, 30 Dec 2023 20:07:02 +0200 Subject: [PATCH 6/9] use suggestion instead of fix --- .../src/rules/no-array-delete.ts | 38 +-- .../tests/rules/no-array-delete.test.ts | 232 ++++++++++++------ 2 files changed, 181 insertions(+), 89 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts index dc89493b87d9..5144026a42ae 100644 --- a/packages/eslint-plugin/src/rules/no-array-delete.ts +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -8,12 +8,12 @@ import { getParserServices, } from '../util'; -type MessageId = 'noArrayDelete'; +type MessageId = 'noArrayDelete' | 'useSplice'; export default createRule<[], MessageId>({ name: 'no-array-delete', meta: { - fixable: 'code', + hasSuggestions: true, type: 'problem', docs: { description: 'Disallow using the `delete` operator on array values', @@ -22,7 +22,8 @@ export default createRule<[], MessageId>({ }, messages: { noArrayDelete: - 'Using the `delete` operator with an array expression is unsafe. Use `array.splice()` instead.', + 'Using the `delete` operator with an array expression is unsafe.', + useSplice: 'Use `array.splice()` instead.', }, schema: [], }, @@ -66,23 +67,28 @@ export default createRule<[], MessageId>({ context.report({ node, messageId: 'noArrayDelete', - fix(fixer): TSESLint.RuleFix | null { - if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { - return null; - } + suggest: [ + { + messageId: 'useSplice', + fix(fixer): TSESLint.RuleFix | null { + if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { + return null; + } - const { object, property } = node.argument; + const { object, property } = node.argument; - const shouldHaveParentheses = - property.type === AST_NODE_TYPES.SequenceExpression; + const shouldHaveParentheses = + property.type === AST_NODE_TYPES.SequenceExpression; - const nodeMap = services.esTreeNodeToTSNodeMap; - const target = nodeMap.get(object).getText(); - const rawKey = nodeMap.get(property).getText(); - const key = shouldHaveParentheses ? `(${rawKey})` : rawKey; + const nodeMap = services.esTreeNodeToTSNodeMap; + const target = nodeMap.get(object).getText(); + const rawKey = nodeMap.get(property).getText(); + const key = shouldHaveParentheses ? `(${rawKey})` : rawKey; - return fixer.replaceText(node, `${target}.splice(${key}, 1)`); - }, + return fixer.replaceText(node, `${target}.splice(${key}, 1)`); + }, + }, + ], }); }, }; diff --git a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts index fd3e67b1f965..57e0585b5a38 100644 --- a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts @@ -62,16 +62,21 @@ ruleTester.run('no-array-delete', rule, { declare const arr: number[]; delete arr[0]; `, - output: ` - declare const arr: number[]; - arr.splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: number[]; + arr.splice(0, 1); + `, + }, + ], }, ], }, @@ -82,17 +87,22 @@ ruleTester.run('no-array-delete', rule, { declare const key: number; delete arr[key]; `, - output: ` - declare const arr: number[]; - declare const key: number; - arr.splice(key, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 4, column: 9, endColumn: 24, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: number[]; + declare const key: number; + arr.splice(key, 1); + `, + }, + ], }, ], }, @@ -108,7 +118,16 @@ ruleTester.run('no-array-delete', rule, { delete arr[Keys.A]; `, - output: ` + errors: [ + { + messageId: 'noArrayDelete', + line: 9, + column: 9, + endColumn: 27, + suggestions: [ + { + messageId: 'useSplice', + output: ` declare const arr: number[]; enum Keys { @@ -118,12 +137,8 @@ ruleTester.run('no-array-delete', rule, { arr.splice(Keys.A, 1); `, - errors: [ - { - messageId: 'noArrayDelete', - line: 9, - column: 9, - endColumn: 27, + }, + ], }, ], }, @@ -134,17 +149,22 @@ ruleTester.run('no-array-delete', rule, { declare function doWork(): void; delete arr[(doWork(), 1)]; `, - output: ` - declare const arr: number[]; - declare function doWork(): void; - arr.splice((doWork(), 1), 1); - `, errors: [ { messageId: 'noArrayDelete', line: 4, column: 9, endColumn: 34, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: number[]; + declare function doWork(): void; + arr.splice((doWork(), 1), 1); + `, + }, + ], }, ], }, @@ -154,29 +174,40 @@ ruleTester.run('no-array-delete', rule, { declare const arr: Array; delete arr[0]; `, - output: ` - declare const arr: Array; - arr.splice(0, 1); - `, + errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: Array; + arr.splice(0, 1); + `, + }, + ], }, ], }, { code: 'delete [1, 2, 3][0];', - output: '[1, 2, 3].splice(0, 1);', errors: [ { messageId: 'noArrayDelete', line: 1, column: 1, endColumn: 20, + suggestions: [ + { + messageId: 'useSplice', + output: '[1, 2, 3].splice(0, 1);', + }, + ], }, ], }, @@ -186,16 +217,21 @@ ruleTester.run('no-array-delete', rule, { declare const arr: unknown[]; delete arr[Math.random() ? 0 : 1]; `, - output: ` - declare const arr: unknown[]; - arr.splice(Math.random() ? 0 : 1, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 42, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: unknown[]; + arr.splice(Math.random() ? 0 : 1, 1); + `, + }, + ], }, ], }, @@ -205,16 +241,21 @@ ruleTester.run('no-array-delete', rule, { declare const arr: number[] | string[] | boolean[]; delete arr[0]; `, - output: ` - declare const arr: number[] | string[] | boolean[]; - arr.splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: number[] | string[] | boolean[]; + arr.splice(0, 1); + `, + }, + ], }, ], }, @@ -224,16 +265,21 @@ ruleTester.run('no-array-delete', rule, { declare const arr: number[] & unknown; delete arr[0]; `, - output: ` - declare const arr: number[] & unknown; - arr.splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: number[] & unknown; + arr.splice(0, 1); + `, + }, + ], }, ], }, @@ -243,16 +289,21 @@ ruleTester.run('no-array-delete', rule, { declare const arr: (number | string)[]; delete arr[0]; `, - output: ` - declare const arr: (number | string)[]; - arr.splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const arr: (number | string)[]; + arr.splice(0, 1); + `, + }, + ], }, ], }, @@ -262,16 +313,21 @@ ruleTester.run('no-array-delete', rule, { declare const obj: { a: { b: { c: number[] } } }; delete obj.a.b.c[0]; `, - output: ` - declare const obj: { a: { b: { c: number[] } } }; - obj.a.b.c.splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 28, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const obj: { a: { b: { c: number[] } } }; + obj.a.b.c.splice(0, 1); + `, + }, + ], }, ], }, @@ -281,16 +337,21 @@ ruleTester.run('no-array-delete', rule, { declare function getArray(): T; delete getArray()[0]; `, - output: ` - declare function getArray(): T; - getArray().splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 29, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare function getArray(): T; + getArray().splice(0, 1); + `, + }, + ], }, ], }, @@ -300,16 +361,21 @@ ruleTester.run('no-array-delete', rule, { declare function getArray(): T[]; delete getArray()[0]; `, - output: ` - declare function getArray(): T[]; - getArray().splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 29, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare function getArray(): T[]; + getArray().splice(0, 1); + `, + }, + ], }, ], }, @@ -320,17 +386,22 @@ ruleTester.run('no-array-delete', rule, { delete a[0]; } `, - output: ` - function deleteFromArray(a: number[]) { - a.splice(0, 1); - } - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 11, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + function deleteFromArray(a: number[]) { + a.splice(0, 1); + } + `, + }, + ], }, ], }, @@ -341,17 +412,22 @@ ruleTester.run('no-array-delete', rule, { delete a[0]; } `, - output: ` - function deleteFromArray(a: T[]) { - a.splice(0, 1); - } - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 11, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + function deleteFromArray(a: T[]) { + a.splice(0, 1); + } + `, + }, + ], }, ], }, @@ -362,17 +438,22 @@ ruleTester.run('no-array-delete', rule, { delete a[0]; } `, - output: ` - function deleteFromArray(a: T) { - a.splice(0, 1); - } - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 11, endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + output: ` + function deleteFromArray(a: T) { + a.splice(0, 1); + } + `, + }, + ], }, ], }, @@ -382,16 +463,21 @@ ruleTester.run('no-array-delete', rule, { declare const tuple: [number, string]; delete tuple[0]; `, - output: ` - declare const tuple: [number, string]; - tuple.splice(0, 1); - `, errors: [ { messageId: 'noArrayDelete', line: 3, column: 9, endColumn: 24, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const tuple: [number, string]; + tuple.splice(0, 1); + `, + }, + ], }, ], }, From 4b29b597f196f8229ee4a3ffdae9db8afb4e5ee8 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sat, 30 Dec 2023 20:36:54 +0200 Subject: [PATCH 7/9] added more test cases --- .../tests/rules/no-array-delete.test.ts | 106 +++++++++++++++++- 1 file changed, 105 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts index 57e0585b5a38..ea5672ed3831 100644 --- a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts @@ -1,4 +1,4 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; +import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; import rule from '../../src/rules/no-array-delete'; import { getFixturesRootDir } from '../RuleTester'; @@ -481,5 +481,109 @@ ruleTester.run('no-array-delete', rule, { }, ], }, + + { + code: ` + declare const a: number[]; + declare const b: number; + + delete [...a, ...a][b]; + `, + errors: [ + { + messageId: 'noArrayDelete', + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const a: number[]; + declare const b: number; + + [...a, ...a].splice(b, 1); + `, + }, + ], + }, + ], + }, + + { + code: noFormat` + declare const a: number[]; + declare const b: number; + + delete /* multi + line */ a[(( + // single-line + b /* inline */ /* another-inline */ ) + ) /* another-one */ ]; + `, + errors: [ + { + messageId: 'noArrayDelete', + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const a: number[]; + declare const b: number; + + a.splice(b, 1); + `, + }, + ], + }, + ], + }, + + { + code: noFormat` + declare const a: number[]; + declare const b: number; + + delete ((a[((b))])); + `, + errors: [ + { + messageId: 'noArrayDelete', + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const a: number[]; + declare const b: number; + + a.splice(b, 1); + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const a: number[]; + declare const b: number; + + delete a[(b + 1) * (b + 2)]; + `, + errors: [ + { + messageId: 'noArrayDelete', + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const a: number[]; + declare const b: number; + + a.splice((b + 1) * (b + 2), 1); + `, + }, + ], + }, + ], + }, ], }); From ac595ca6e2dc23dc0066393724de4a28f11d7863 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 7 Jan 2024 19:49:16 +0200 Subject: [PATCH 8/9] remove redundant condition --- .../eslint-plugin/src/rules/no-array-delete.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts index 5144026a42ae..2af8cc409063 100644 --- a/packages/eslint-plugin/src/rules/no-array-delete.ts +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -51,14 +51,13 @@ export default createRule<[], MessageId>({ 'UnaryExpression[operator="delete"]'( node: TSESTree.UnaryExpression, ): void { - if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { + const { argument } = node; + + if (argument.type !== AST_NODE_TYPES.MemberExpression) { return; } - const type = getConstrainedTypeAtLocation( - services, - node.argument.object, - ); + const type = getConstrainedTypeAtLocation(services, argument.object); if (!isUnderlyingTypeArray(type)) { return; @@ -71,11 +70,7 @@ export default createRule<[], MessageId>({ { messageId: 'useSplice', fix(fixer): TSESLint.RuleFix | null { - if (node.argument.type !== AST_NODE_TYPES.MemberExpression) { - return null; - } - - const { object, property } = node.argument; + const { object, property } = argument; const shouldHaveParentheses = property.type === AST_NODE_TYPES.SequenceExpression; From e12f90134ed5c64500db024e795c1315c5a7bcc8 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 7 Jan 2024 19:49:31 +0200 Subject: [PATCH 9/9] keep comments --- .../src/rules/no-array-delete.ts | 25 +++++++++++++++++-- .../tests/rules/no-array-delete.test.ts | 16 +++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-array-delete.ts b/packages/eslint-plugin/src/rules/no-array-delete.ts index 2af8cc409063..141332e06382 100644 --- a/packages/eslint-plugin/src/rules/no-array-delete.ts +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -1,5 +1,6 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import { getSourceCode } from '@typescript-eslint/utils/eslint-utils'; import type * as ts from 'typescript'; import { @@ -80,7 +81,27 @@ export default createRule<[], MessageId>({ const rawKey = nodeMap.get(property).getText(); const key = shouldHaveParentheses ? `(${rawKey})` : rawKey; - return fixer.replaceText(node, `${target}.splice(${key}, 1)`); + let suggestion = `${target}.splice(${key}, 1)`; + + const sourceCode = getSourceCode(context); + const comments = sourceCode.getCommentsInside(node); + + if (comments.length > 0) { + const indentationCount = node.loc.start.column; + const indentation = ' '.repeat(indentationCount); + + const commentsText = comments + .map(comment => { + return comment.type === AST_TOKEN_TYPES.Line + ? `//${comment.value}` + : `/*${comment.value}*/`; + }) + .join(`\n${indentation}`); + + suggestion = `${commentsText}\n${indentation}${suggestion}`; + } + + return fixer.replaceText(node, suggestion); }, }, ], diff --git a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts index ea5672ed3831..ac803cb5b2f8 100644 --- a/packages/eslint-plugin/tests/rules/no-array-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts @@ -512,11 +512,13 @@ ruleTester.run('no-array-delete', rule, { declare const a: number[]; declare const b: number; - delete /* multi + // before expression + delete /** multi line */ a[(( // single-line b /* inline */ /* another-inline */ ) - ) /* another-one */ ]; + ) /* another-one */ ] /* before semicolon */; /* after semicolon */ + // after expression `, errors: [ { @@ -528,7 +530,15 @@ ruleTester.run('no-array-delete', rule, { declare const a: number[]; declare const b: number; - a.splice(b, 1); + // before expression + /** multi + line */ + // single-line + /* inline */ + /* another-inline */ + /* another-one */ + a.splice(b, 1) /* before semicolon */; /* after semicolon */ + // after expression `, }, ],