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..3abbbc6ececc --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-array-delete.md @@ -0,0 +1,40 @@ +--- +description: 'Disallow using the `delete` operator on array values.' +--- + +> 🛑 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` 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), +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 9881e3397de2..5676aa745aaa 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -39,6 +39,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..141332e06382 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-array-delete.ts @@ -0,0 +1,112 @@ +import type { TSESLint, TSESTree } 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 { + createRule, + getConstrainedTypeAtLocation, + getParserServices, +} from '../util'; + +type MessageId = 'noArrayDelete' | 'useSplice'; + +export default createRule<[], MessageId>({ + name: 'no-array-delete', + meta: { + hasSuggestions: true, + type: 'problem', + docs: { + 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 `array.splice()` instead.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const services = getParserServices(context); + 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(predicate); + } + + if (type.isIntersection()) { + return type.types.some(predicate); + } + + return predicate(type); + } + + return { + 'UnaryExpression[operator="delete"]'( + node: TSESTree.UnaryExpression, + ): void { + const { argument } = node; + + if (argument.type !== AST_NODE_TYPES.MemberExpression) { + return; + } + + const type = getConstrainedTypeAtLocation(services, argument.object); + + if (!isUnderlyingTypeArray(type)) { + return; + } + + context.report({ + node, + messageId: 'noArrayDelete', + suggest: [ + { + messageId: 'useSplice', + fix(fixer): TSESLint.RuleFix | null { + const { object, property } = 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; + + 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 new file mode 100644 index 000000000000..ac803cb5b2f8 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-array-delete.test.ts @@ -0,0 +1,599 @@ +import { noFormat, 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; + `, + + ` + declare const test: never; + delete test[0]; + `, + ], + + invalid: [ + { + code: ` + declare const arr: number[]; + delete arr[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 22, + suggestions: [ + { + messageId: 'useSplice', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + 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', + output: ` + function deleteFromArray(a: T) { + a.splice(0, 1); + } + `, + }, + ], + }, + ], + }, + + { + code: ` + declare const tuple: [number, string]; + delete tuple[0]; + `, + errors: [ + { + messageId: 'noArrayDelete', + line: 3, + column: 9, + endColumn: 24, + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const tuple: [number, string]; + tuple.splice(0, 1); + `, + }, + ], + }, + ], + }, + + { + 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; + + // before expression + delete /** multi + line */ a[(( + // single-line + b /* inline */ /* another-inline */ ) + ) /* another-one */ ] /* before semicolon */; /* after semicolon */ + // after expression + `, + errors: [ + { + messageId: 'noArrayDelete', + suggestions: [ + { + messageId: 'useSplice', + output: ` + declare const a: number[]; + declare const b: number; + + // before expression + /** multi + line */ + // single-line + /* inline */ + /* another-inline */ + /* another-one */ + a.splice(b, 1) /* before semicolon */; /* after semicolon */ + // after expression + `, + }, + ], + }, + ], + }, + + { + 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); + `, + }, + ], + }, + ], + }, + ], +}); 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 = [];" +`;