fix: produce valid code when when fixing properties accessed with square brackets (#1131)

G-Rath · web-flow · commit 6cd600dbd484 · 2022-05-30T07:34:07.000+12:00
* fix(prefer-todo): support fixing indexed property access

* fix(prefer-to-be): support fixing indexed property access

* fix(no-alias-methods): support fixing indexed property access

* fix(prefer-strict-equal): support fixing indexed property access

* refactor: use helper for wrapping text in quotes

* refactor(prefer-todo): reduce code a little
diff --git a/src/rules/__tests__/no-alias-methods.test.ts b/src/rules/__tests__/no-alias-methods.test.ts
@@ -231,5 +231,20 @@ ruleTester.run('no-alias-methods', rule, {
         },
       ],
     },
+    {
+      code: 'expect(a).not["toThrowError"]()',
+      output: "expect(a).not['toThrow']()",
+      errors: [
+        {
+          messageId: 'replaceAlias',
+          data: {
+            alias: 'toThrowError',
+            canonical: 'toThrow',
+          },
+          column: 15,
+          line: 1,
+        },
+      ],
+    },
   ],
 });
diff --git a/src/rules/__tests__/prefer-strict-equal.test.ts b/src/rules/__tests__/prefer-strict-equal.test.ts
@@ -26,5 +26,21 @@ ruleTester.run('prefer-strict-equal', rule, {
         },
       ],
     },
+    {
+      code: 'expect(something)["toEqual"](somethingElse);',
+      errors: [
+        {
+          messageId: 'useToStrictEqual',
+          column: 19,
+          line: 1,
+          suggestions: [
+            {
+              messageId: 'suggestReplaceWithStrictEqual',
+              output: "expect(something)['toStrictEqual'](somethingElse);",
+            },
+          ],
+        },
+      ],
+    },
   ],
 });
diff --git a/src/rules/__tests__/prefer-to-be.test.ts b/src/rules/__tests__/prefer-to-be.test.ts
@@ -45,6 +45,11 @@ ruleTester.run('prefer-to-be', rule, {
       output: 'expect(value).toBe(`my string`);',
       errors: [{ messageId: 'useToBe', column: 15, line: 1 }],
     },
+    {
+      code: 'expect(value)["toEqual"](`my string`);',
+      output: "expect(value)['toBe'](`my string`);",
+      errors: [{ messageId: 'useToBe', column: 15, line: 1 }],
+    },
     {
       code: 'expect(value).toStrictEqual(`my ${string}`);',
       output: 'expect(value).toBe(`my ${string}`);',
@@ -55,6 +60,16 @@ ruleTester.run('prefer-to-be', rule, {
       output: 'expect(loadMessage()).resolves.toBe("hello world");',
       errors: [{ messageId: 'useToBe', column: 32, line: 1 }],
     },
+    {
+      code: 'expect(loadMessage()).resolves["toStrictEqual"]("hello world");',
+      output: 'expect(loadMessage()).resolves[\'toBe\']("hello world");',
+      errors: [{ messageId: 'useToBe', column: 32, line: 1 }],
+    },
+    {
+      code: 'expect(loadMessage())["resolves"].toStrictEqual("hello world");',
+      output: 'expect(loadMessage())["resolves"].toBe("hello world");',
+      errors: [{ messageId: 'useToBe', column: 35, line: 1 }],
+    },
     {
       code: 'expect(loadMessage()).resolves.toStrictEqual(false);',
       output: 'expect(loadMessage()).resolves.toBe(false);',
@@ -103,6 +118,16 @@ ruleTester.run('prefer-to-be: null', rule, {
       output: 'expect("a string").not.toBeNull();',
       errors: [{ messageId: 'useToBeNull', column: 24, line: 1 }],
     },
+    {
+      code: 'expect("a string").not["toBe"](null);',
+      output: 'expect("a string").not[\'toBeNull\']();',
+      errors: [{ messageId: 'useToBeNull', column: 24, line: 1 }],
+    },
+    {
+      code: 'expect("a string")["not"]["toBe"](null);',
+      output: 'expect("a string")["not"][\'toBeNull\']();',
+      errors: [{ messageId: 'useToBeNull', column: 27, line: 1 }],
+    },
     {
       code: 'expect("a string").not.toEqual(null);',
       output: 'expect("a string").not.toBeNull();',
@@ -156,6 +181,11 @@ ruleTester.run('prefer-to-be: undefined', rule, {
       output: 'expect("a string").rejects.toBeDefined();',
       errors: [{ messageId: 'useToBeDefined', column: 32, line: 1 }],
     },
+    {
+      code: 'expect("a string").rejects.not["toBe"](undefined);',
+      output: 'expect("a string").rejects[\'toBeDefined\']();',
+      errors: [{ messageId: 'useToBeDefined', column: 32, line: 1 }],
+    },
     {
       code: 'expect("a string").not.toEqual(undefined);',
       output: 'expect("a string").toBeDefined();',
@@ -208,6 +238,11 @@ ruleTester.run('prefer-to-be: NaN', rule, {
       output: 'expect("a string").rejects.not.toBeNaN();',
       errors: [{ messageId: 'useToBeNaN', column: 32, line: 1 }],
     },
+    {
+      code: 'expect("a string")["rejects"].not.toBe(NaN);',
+      output: 'expect("a string")["rejects"].not.toBeNaN();',
+      errors: [{ messageId: 'useToBeNaN', column: 35, line: 1 }],
+    },
     {
       code: 'expect("a string").not.toEqual(NaN);',
       output: 'expect("a string").not.toBeNaN();',
diff --git a/src/rules/__tests__/prefer-todo.test.ts b/src/rules/__tests__/prefer-todo.test.ts
@@ -58,5 +58,15 @@ ruleTester.run('prefer-todo', rule, {
       output: 'test.todo("i need to write this test");',
       errors: [{ messageId: 'emptyTest' }],
     },
+    {
+      code: `test["skip"]("i need to write this test", function() {});`,
+      output: 'test[\'todo\']("i need to write this test");',
+      errors: [{ messageId: 'emptyTest' }],
+    },
+    {
+      code: `test[\`skip\`]("i need to write this test", function() {});`,
+      output: 'test[\'todo\']("i need to write this test");',
+      errors: [{ messageId: 'emptyTest' }],
+    },
   ],
 });
diff --git a/src/rules/no-alias-methods.ts b/src/rules/no-alias-methods.ts
@@ -1,4 +1,9 @@
-import { createRule, isExpectCall, parseExpectCall } from './utils';
+import {
+  createRule,
+  isExpectCall,
+  parseExpectCall,
+  replaceAccessorFixer,
+} from './utils';
 
 export default createRule({
   name: __filename,
@@ -56,7 +61,9 @@ export default createRule({
               canonical,
             },
             node: matcher.node.property,
-            fix: fixer => [fixer.replaceText(matcher.node.property, canonical)],
+            fix: fixer => [
+              replaceAccessorFixer(fixer, matcher.node.property, canonical),
+            ],
           });
         }
       },
diff --git a/src/rules/prefer-strict-equal.ts b/src/rules/prefer-strict-equal.ts
@@ -4,6 +4,7 @@ import {
   isExpectCall,
   isParsedEqualityMatcherCall,
   parseExpectCall,
+  replaceAccessorFixer,
 } from './utils';
 
 export default createRule({
@@ -44,7 +45,8 @@ export default createRule({
               {
                 messageId: 'suggestReplaceWithStrictEqual',
                 fix: fixer => [
-                  fixer.replaceText(
+                  replaceAccessorFixer(
+                    fixer,
                     matcher.node.property,
                     EqualityMatcher.toStrictEqual,
                   ),
diff --git a/src/rules/prefer-to-be.ts b/src/rules/prefer-to-be.ts
@@ -12,6 +12,7 @@ import {
   isIdentifier,
   isParsedEqualityMatcherCall,
   parseExpectCall,
+  replaceAccessorFixer,
 } from './utils';
 
 const isNullLiteral = (node: TSESTree.Node): node is TSESTree.NullLiteral =>
@@ -70,7 +71,7 @@ const reportPreferToBe = (
     messageId: `useToBe${whatToBe}`,
     fix(fixer) {
       const fixes = [
-        fixer.replaceText(matcher.node.property, `toBe${whatToBe}`),
+        replaceAccessorFixer(fixer, matcher.node.property, `toBe${whatToBe}`),
       ];
 
       if (matcher.arguments?.length && whatToBe !== '') {
diff --git a/src/rules/prefer-todo.ts b/src/rules/prefer-todo.ts
@@ -7,6 +7,7 @@ import {
   isFunction,
   isStringNode,
   parseJestFnCall,
+  replaceAccessorFixer,
 } from './utils';
 
 function isEmptyFunction(node: TSESTree.CallExpressionArgument) {
@@ -23,20 +24,14 @@ function createTodoFixer(
   jestFnCall: ParsedJestFnCall,
   fixer: TSESLint.RuleFixer,
 ) {
-  const fixes = [
-    fixer.replaceText(jestFnCall.head.node, `${jestFnCall.head.local}.todo`),
-  ];
-
   if (jestFnCall.members.length) {
-    fixes.unshift(
-      fixer.removeRange([
-        jestFnCall.head.node.range[1],
-        jestFnCall.members[0].range[1],
-      ]),
-    );
+    return replaceAccessorFixer(fixer, jestFnCall.members[0], 'todo');
   }
 
-  return fixes;
+  return fixer.replaceText(
+    jestFnCall.head.node,
+    `${jestFnCall.head.local}.todo`,
+  );
 }
 
 const isTargetedTestCase = (jestFnCall: ParsedJestFnCall): boolean => {
@@ -91,7 +86,7 @@ export default createRule({
             node,
             fix: fixer => [
               fixer.removeRange([title.range[1], callback.range[1]]),
-              ...createTodoFixer(jestFnCall, fixer),
+              createTodoFixer(jestFnCall, fixer),
             ],
           });
         }
diff --git a/src/rules/utils/misc.ts b/src/rules/utils/misc.ts
@@ -137,3 +137,20 @@ export const getTestCallExpressionsFromDeclaredVariables = (
     [],
   );
 };
+
+/**
+ * Replaces an accessor node with the given `text`, surrounding it in quotes if required.
+ *
+ * This ensures that fixes produce valid code when replacing both dot-based and
+ * bracket-based property accessors.
+ */
+export const replaceAccessorFixer = (
+  fixer: TSESLint.RuleFixer,
+  node: AccessorNode,
+  text: string,
+) => {
+  return fixer.replaceText(
+    node,
+    node.type === AST_NODE_TYPES.Identifier ? text : `'${text}'`,
+  );
+};
