typescript-eslint · bradzacher · Nov 12, 2019 · May 27, 2019 · May 28, 2019 · May 28, 2019
diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
@@ -160,6 +160,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
 | [`@typescript-eslint/member-naming`](./docs/rules/member-naming.md)                                       | Enforces naming conventions for class members by visibility                                                                                         |                    |          |                   |
 | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md)                                   | Require a consistent member declaration order                                                                                                       |                    |          |                   |
 | [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md)                         | Disallow generic `Array` constructors                                                                                                               | :heavy_check_mark: | :wrench: |                   |
+| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md)                               | Bans usage of the delete operator with computed key expressions                                                                                     |                    | :wrench: |                   |
 | [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md)                               | Disallow empty functions                                                                                                                            | :heavy_check_mark: |          |                   |
 | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md)                             | Disallow the declaration of empty interfaces                                                                                                        | :heavy_check_mark: |          |                   |
 | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md)                                   | Disallow usage of the `any` type                                                                                                                    | :heavy_check_mark: | :wrench: |                   |

diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md
@@ -60,7 +60,7 @@
 | [`no-duplicate-super`]               | 🌟  | [`constructor-super`][constructor-super]                              |
 | [`no-duplicate-switch-case`]         | 🌟  | [`no-duplicate-case`][no-duplicate-case]                              |
 | [`no-duplicate-variable`]            | 🌟  | [`no-redeclare`][no-redeclare]                                        |
-| [`no-dynamic-delete`]                | 🛑  | N/A                                                                   |
+| [`no-dynamic-delete`]                | ✅  | [`@typescript-eslint/no-dynamic-delete`]                              |
 | [`no-empty`]                         | 🌟  | [`no-empty`][no-empty]                                                |
 | [`no-eval`]                          | 🌟  | [`no-eval`][no-eval]                                                  |
 | [`no-floating-promises`]             | ✅  | [`@typescript-eslint/no-floating-promises`]                           |
@@ -613,6 +613,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
 [`@typescript-eslint/member-delimiter-style`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-delimiter-style.md
 [`@typescript-eslint/prefer-for-of`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-for-of.md
 [`@typescript-eslint/no-array-constructor`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-array-constructor.md
+[`@typescript-eslint/no-dynamic-delete`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-dynamic-delete.md
 [`@typescript-eslint/prefer-function-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-function-type.md
 [`@typescript-eslint/prefer-readonly`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-readonly.md
 [`@typescript-eslint/require-await`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/require-await.md

diff --git a/packages/eslint-plugin/docs/rules/no-dynamic-delete.md b/packages/eslint-plugin/docs/rules/no-dynamic-delete.md
@@ -0,0 +1,49 @@
+# Disallow the delete operator with computed key expressions (no-dynamic-delete)
+
+Deleting dynamically computed keys can be dangerous and in some cases not well optimized.
+
+## Rule Details
+
+Using the `delete` operator on keys that aren't runtime constants could be a sign that you're using the wrong data structures.
+Using `Object`s with added and removed keys can cause occasional edge case bugs, such as if a key is named `"hasOwnProperty"`.
+Consider using a `Map` or `Set` if you’re storing collections of objects.
+
+Examples of **correct** code wth this rule:
+
+```ts
+const container: { [i: string]: number } = {
+  /* ... */
+};
+
+// Constant runtime lookups by string index
+delete container.aaa;
+
+// Constants that must be accessed by []
+delete container[7];
+delete container['-Infinity'];
+```
+
+Examples of **incorrect** code with this rule:
+
+```ts
+// Can be replaced with the constant equivalents, such as container.aaa
+delete container['aaa'];
+delete container['Infinity'];
+
+// Dynamic, difficult-to-reason-about lookups
+const name = 'name';
+delete container[name];
+delete container[name.toUpperCase()];
+```
+
+## When Not To Use It
+
+When you know your keys are safe to delete, this rule can be unnecessary.
+Some environments such as older browsers might not support `Map` and `Set`.
+
+Do not consider this rule as performance advice before profiling your code's bottlenecks.
+Even repeated minor performance slowdowns likely do not significantly affect your application's general perceived speed.
+
+## Related to
+
+- TSLint: [no-dynamic-delete](https://palantir.github.io/tslint/rules/no-dynamic-delete)
diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json
@@ -26,6 +26,7 @@
     "@typescript-eslint/member-ordering": "error",
     "no-array-constructor": "off",
     "@typescript-eslint/no-array-constructor": "error",
+    "@typescript-eslint/no-dynamic-delete": "error",
     "no-empty-function": "off",
     "@typescript-eslint/no-empty-function": "error",
     "@typescript-eslint/no-empty-interface": "error",

diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts
@@ -18,6 +18,7 @@ import memberDelimiterStyle from './member-delimiter-style';
 import memberNaming from './member-naming';
 import memberOrdering from './member-ordering';
 import noArrayConstructor from './no-array-constructor';
+import noDynamicDelete from './no-dynamic-delete';
 import noEmptyFunction from './no-empty-function';
 import noEmptyInterface from './no-empty-interface';
 import noExplicitAny from './no-explicit-any';
@@ -85,6 +86,7 @@ export default {
   'member-naming': memberNaming,
   'member-ordering': memberOrdering,
   'no-array-constructor': noArrayConstructor,
+  'no-dynamic-delete': noDynamicDelete,
   'no-empty-function': noEmptyFunction,
   'no-empty-interface': noEmptyInterface,
   'no-explicit-any': noExplicitAny,

diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts
@@ -0,0 +1,109 @@
+import {
+  TSESTree,
+  AST_NODE_TYPES,
+  TSESLint,
+} from '@typescript-eslint/experimental-utils';
+import * as tsutils from 'tsutils';
+import * as util from '../util';
+
+export default util.createRule({
+  name: 'no-dynamic-delete',
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description:
+        'Bans usage of the delete operator with computed key expressions',
+      recommended: false,
+    },
+    fixable: 'code',
+    messages: {
+      dynamicDelete: 'Do not delete dynamically computed property keys.',
+    },
+    schema: [],
+    type: 'suggestion',
+  },
+  defaultOptions: [],
+  create(context) {
+    function createFixer(
+      member: TSESTree.MemberExpression,
+    ): TSESLint.ReportFixFunction | undefined {
+      if (
+        member.property.type === AST_NODE_TYPES.Literal &&
+        typeof member.property.value === 'string'
+      ) {
+        return createPropertyReplacement(
+          member.property,
+          member.property.value,
+        );
+      }
+
+      if (member.property.type === AST_NODE_TYPES.Identifier) {
+        return createPropertyReplacement(member.property, member.property.name);
+      }
+
+      return undefined;
+    }
+
+    return {
+      'UnaryExpression[operator=delete]'(node: TSESTree.UnaryExpression): void {
+        if (
+          node.argument.type !== AST_NODE_TYPES.MemberExpression ||
+          !node.argument.computed ||
+          isNecessaryDynamicAccess(
+            diveIntoWrapperExpressions(node.argument.property),
+          )
+        ) {
+          return;
+        }
+
+        context.report({
+          fix: createFixer(node.argument),
+          messageId: 'dynamicDelete',
+          node: node.argument.property,
+        });
+      },
+    };
+
+    function createPropertyReplacement(
+      property: TSESTree.Expression,
+      replacement: string,
+    ) {
+      return (fixer: TSESLint.RuleFixer): TSESLint.RuleFix =>
+        fixer.replaceTextRange(getTokenRange(property), `.${replacement}`);
+    }
+
+    function getTokenRange(property: TSESTree.Expression): [number, number] {
+      const sourceCode = context.getSourceCode();
+
+      return [
+        sourceCode.getTokenBefore(property)!.range[0],
+        sourceCode.getTokenAfter(property)!.range[1],
+      ];
+    }
+  },
+});
+
+function diveIntoWrapperExpressions(
+  node: TSESTree.Expression,
+): TSESTree.Expression {
+  if (node.type === AST_NODE_TYPES.UnaryExpression) {
+    return diveIntoWrapperExpressions(node.argument);
+  }
+
+  return node;
+}
+
+function isNecessaryDynamicAccess(property: TSESTree.Expression): boolean {
+  if (property.type !== AST_NODE_TYPES.Literal) {
+    return false;
+  }
+
+  if (typeof property.value === 'number') {
+    return true;
+  }
+
+  return (
+    typeof property.value === 'string' &&
+    !tsutils.isValidPropertyAccess(property.value)
+  );
+}
diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts
@@ -0,0 +1,105 @@
+import path from 'path';
+import rule from '../../src/rules/no-dynamic-delete';
+import { RuleTester } from '../RuleTester';
+
+const rootDir = path.join(process.cwd(), 'tests/fixtures');
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2015,
+    tsconfigRootDir: rootDir,
+    project: './tsconfig.json',
+  },
+  parser: '@typescript-eslint/parser',
+});
+
+ruleTester.run('no-dynamic-delete', rule, {
+  valid: [
+    `const container: { [i: string]: 0 } = {};
+      delete container.aaa;`,
+    `const container: { [i: string]: 0 } = {};
+      delete container.delete;`,
+    `const container: { [i: string]: 0 } = {};
+      delete container[7];`,
+    `const container: { [i: string]: 0 } = {};
+      delete container[-7];`,
+    `const container: { [i: string]: 0 } = {};
+      delete container[+7];`,
+    `const container: { [i: string]: 0 } = {};
+      delete container['-Infinity'];`,
+    `const container: { [i: string]: 0 } = {};
+      delete container['+Infinity'];`,
+    `const value = 1;
+      delete value;`,
+    `const value = 1;
+      delete -value;`,
+  ],
+  invalid: [
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container['aaa'];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+      output: `const container: { [i: string]: 0 } = {};
+        delete container.aaa;`,
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container [  'aaa'   ] ;`,
+      errors: [{ messageId: 'dynamicDelete' }],
+      output: `const container: { [i: string]: 0 } = {};
+        delete container .aaa ;`,
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container['aa' + 'b'];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container['delete'];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+      output: `const container: { [i: string]: 0 } = {};
+        delete container.delete;`,
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container[-Infinity];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container[+Infinity];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container[NaN];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container['NaN'];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+      output: `const container: { [i: string]: 0 } = {};
+        delete container.NaN;`,
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        delete container    [   'NaN'  ] ;`,
+      errors: [{ messageId: 'dynamicDelete' }],
+      output: `const container: { [i: string]: 0 } = {};
+        delete container    .NaN ;`,
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        const name = 'name';
+        delete container[name];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+    },
+    {
+      code: `const container: { [i: string]: 0 } = {};
+        const getName = () => 'aaa';
+        delete container[getName()];`,
+      errors: [{ messageId: 'dynamicDelete' }],
+    },
+  ],
+});