feat(eslint-plugin): [no-dynamic-delete] allow all string literals as…

… index (#9280) * feat(eslint-plugin): [no-dynamic-delete] allow all string literals as index * Update tests * Bleh
typescript-eslint · Jun 8, 2024 · fb52f78 · fb52f78
1 parent f2f1546
commit fb52f78
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 66 deletions.
diff --git a/packages/eslint-plugin/docs/rules/no-dynamic-delete.mdx b/packages/eslint-plugin/docs/rules/no-dynamic-delete.mdx
@@ -25,10 +25,6 @@ Dynamically adding and removing keys from objects can cause occasional edge case
 <TabItem value="❌ Incorrect">
 
 ```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];
@@ -48,7 +44,12 @@ delete container.aaa;
 
 // Constants that must be accessed by []
 delete container[7];
-delete container['-Infinity'];
+delete container[-1];
+
+// All strings are allowed, to be compatible with the noPropertyAccessFromIndexSignature
+// TS compiler option
+delete container['aaa'];
+delete container['Infinity'];
 ```
 
 </TabItem>

diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts
@@ -1,6 +1,5 @@
 import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
-import * as tsutils from 'ts-api-utils';
 
 import { createRule, nullThrows, NullThrowsReasons } from '../util';
 
@@ -42,9 +41,7 @@ export default createRule({
         if (
           node.argument.type !== AST_NODE_TYPES.MemberExpression ||
           !node.argument.computed ||
-          isNecessaryDynamicAccess(
-            diveIntoWrapperExpressions(node.argument.property),
-          )
+          isAcceptableIndexExpression(node.argument.property)
         ) {
           return;
         }
@@ -80,27 +77,13 @@ export default createRule({
   },
 });
 
-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;
-  }
-
+function isAcceptableIndexExpression(property: TSESTree.Expression): boolean {
   return (
-    typeof property.value === 'string' &&
-    !tsutils.isValidPropertyAccess(property.value)
+    (property.type === AST_NODE_TYPES.Literal &&
+      ['string', 'number'].includes(typeof property.value)) ||
+    (property.type === AST_NODE_TYPES.UnaryExpression &&
+      property.operator === '-' &&
+      property.argument.type === AST_NODE_TYPES.Literal &&
+      typeof property.argument.value === 'number')
   );
 }
diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-dynamic-delete.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-dynamic-delete.shot
diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts
@@ -33,10 +33,6 @@ delete container[-7];
     `,
     `
 const container: { [i: string]: 0 } = {};
-delete container[+7];
-    `,
-    `
-const container: { [i: string]: 0 } = {};
 delete container['-Infinity'];
     `,
     `
@@ -51,19 +47,20 @@ 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;
-      `,
-    },
+delete container['delete'];
+    `,
+    `
+const container: { [i: string]: 0 } = {};
+delete container['NaN'];
+    `,
+  ],
+  invalid: [
     {
       code: `
 const container: { [i: string]: 0 } = {};
@@ -75,13 +72,10 @@ delete container['aa' + 'b'];
     {
       code: `
 const container: { [i: string]: 0 } = {};
-delete container['delete'];
+delete container[+7];
       `,
       errors: [{ messageId: 'dynamicDelete' }],
-      output: `
-const container: { [i: string]: 0 } = {};
-delete container.delete;
-      `,
+      output: null,
     },
     {
       code: `
@@ -110,37 +104,42 @@ delete container[NaN];
     {
       code: `
 const container: { [i: string]: 0 } = {};
-delete container['NaN'];
+const name = 'name';
+delete container[name];
       `,
       errors: [{ messageId: 'dynamicDelete' }],
-      output: `
+      output: null,
+    },
+    {
+      code: `
 const container: { [i: string]: 0 } = {};
-delete container.NaN;
+const getName = () => 'aaa';
+delete container[getName()];
       `,
+      output: null,
+      errors: [{ messageId: 'dynamicDelete' }],
     },
     {
       code: `
 const container: { [i: string]: 0 } = {};
-const name = 'name';
-delete container[name];
+const name = { foo: { bar: 'bar' } };
+delete container[name.foo.bar];
       `,
-      errors: [{ messageId: 'dynamicDelete' }],
       output: null,
+      errors: [{ messageId: 'dynamicDelete' }],
     },
     {
       code: `
 const container: { [i: string]: 0 } = {};
-const getName = () => 'aaa';
-delete container[getName()];
+delete container[+'Infinity'];
       `,
       output: null,
       errors: [{ messageId: 'dynamicDelete' }],
     },
     {
       code: `
 const container: { [i: string]: 0 } = {};
-const name = { foo: { bar: 'bar' } };
-delete container[name.foo.bar];
+delete container[typeof 1];
       `,
       output: null,
       errors: [{ messageId: 'dynamicDelete' }],