feat(eslint-plugin): [no-array-delete] add new rule (#8067)

* feat(eslint-plugin): [no-array-delete] add new rule * small refactor * add more cases * fix docs * fix message * use suggestion instead of fix * added more test cases * remove redundant condition * keep comments
typescript-eslint · Jan 9, 2024 · e0f591e · e0f591e
1 parent 9399029
commit e0f591e
Show file tree

Hide file tree

Showing 8 changed files with 770 additions and 0 deletions.
diff --git a/packages/eslint-plugin/docs/rules/no-array-delete.md 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
+
+<!--tabs-->
+
+### ❌ Incorrect
+
+```ts
+declare const arr: number[];
+
+delete arr[0];
+```
+
+### ✅ Correct
+
+```ts
+declare const arr: number[];
+
+arr.splice(0, 1);
+```
+
+<!--/tabs-->
+
+## 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
@@ -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
@@ -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
@@ -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
@@ -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';
@@ -174,6 +175,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
@@ -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);
+              },
+            },
+          ],
+        });
+      },
+    };
+  },
+});