fix(no-test-callback): provide suggestion instead of autofix

jest-community · Jun 20, 2020 · 782d8fa · 782d8fa
1 parent f70612d
commit 782d8fa
Show file tree

Hide file tree

Showing 2 changed files with 207 additions and 98 deletions.
diff --git a/src/rules/__tests__/no-test-callback.test.ts b/src/rules/__tests__/no-test-callback.test.ts
@@ -1,4 +1,5 @@
 import { TSESLint } from '@typescript-eslint/experimental-utils';
+import dedent from 'dedent';
 import resolveFrom from 'resolve-from';
 import rule from '../no-test-callback';
 
@@ -30,37 +31,127 @@ ruleTester.run('no-test-callback', rule, {
     },
     {
       code: 'test("something", done => {done();})',
-      errors: [{ messageId: 'illegalTestCallback', line: 1, column: 19 }],
-      output:
-        'test("something", () => {return new Promise(done => {done();})})',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 19,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output:
+                'test("something", () => {return new Promise(done => {done();})})',
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: 'test("something", finished => {finished();})',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 19,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'finished' },
+              output:
+                'test("something", () => {return new Promise(finished => {finished();})})',
+            },
+          ],
+        },
+      ],
     },
     {
       code: 'test("something", (done) => {done();})',
-      errors: [{ messageId: 'illegalTestCallback', line: 1, column: 20 }],
-      output:
-        'test("something", () => {return new Promise((done) => {done();})})',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 20,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output:
+                'test("something", () => {return new Promise((done) => {done();})})',
+            },
+          ],
+        },
+      ],
     },
     {
       code: 'test("something", done => done())',
-      errors: [{ messageId: 'illegalTestCallback', line: 1, column: 19 }],
-      output: 'test("something", () => new Promise(done => done()))',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 19,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output: 'test("something", () => new Promise(done => done()))',
+            },
+          ],
+        },
+      ],
     },
     {
       code: 'test("something", (done) => done())',
-      errors: [{ messageId: 'illegalTestCallback', line: 1, column: 20 }],
-      output: 'test("something", () => new Promise((done) => done()))',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 20,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output: 'test("something", () => new Promise((done) => done()))',
+            },
+          ],
+        },
+      ],
     },
     {
       code: 'test("something", function(done) {done();})',
-      errors: [{ messageId: 'illegalTestCallback', line: 1, column: 28 }],
-      output:
-        'test("something", function() {return new Promise((done) => {done();})})',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 28,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output:
+                'test("something", function() {return new Promise((done) => {done();})})',
+            },
+          ],
+        },
+      ],
     },
     {
       code: 'test("something", function (done) {done();})',
-      errors: [{ messageId: 'illegalTestCallback', line: 1, column: 29 }],
-      output:
-        'test("something", function () {return new Promise((done) => {done();})})',
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 29,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output:
+                'test("something", function () {return new Promise((done) => {done();})})',
+            },
+          ],
+        },
+      ],
     },
     {
       code: 'test("something", async done => {done();})',
@@ -75,27 +166,39 @@ ruleTester.run('no-test-callback', rule, {
       errors: [{ messageId: 'useAwaitInsteadOfCallback', line: 1, column: 35 }],
     },
     {
-      code: `
-      test('my test', async (done) => {
-        await myAsyncTask();
-        expect(true).toBe(false);
-        done();
-      });
+      code: dedent`
+        test('my test', async (done) => {
+          await myAsyncTask();
+          expect(true).toBe(false);
+          done();
+        });
       `,
-      errors: [{ messageId: 'useAwaitInsteadOfCallback', line: 2, column: 30 }],
+      errors: [{ messageId: 'useAwaitInsteadOfCallback', line: 1, column: 24 }],
     },
     {
-      code: `
-      test('something', (done) => {
-        done();
-      });
-      `,
-      errors: [{ messageId: 'illegalTestCallback', line: 2, column: 26 }],
-      output: `
-      test('something', () => {return new Promise((done) => {
-        done();
-      })});
+      code: dedent`
+        test('something', (done) => {
+          done();
+        });
       `,
+      errors: [
+        {
+          messageId: 'illegalTestCallback',
+          line: 1,
+          column: 20,
+          suggestions: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: 'done' },
+              output: dedent`
+                test('something', () => {return new Promise((done) => {
+                  done();
+                })});
+              `,
+            },
+          ],
+        },
+      ],
     },
   ],
 });
diff --git a/src/rules/no-test-callback.ts b/src/rules/no-test-callback.ts
@@ -11,6 +11,7 @@ export default createRule({
     },
     messages: {
       illegalTestCallback: 'Illegal usage of test callback',
+      suggestWrappingInPromise: 'Wrap in `new Promise({{ callback }} => ...`',
       useAwaitInsteadOfCallback:
         'Use await instead of callback in async functions',
     },
@@ -55,71 +56,76 @@ export default createRule({
         context.report({
           node: argument,
           messageId: 'illegalTestCallback',
-          fix(fixer) {
-            const { body } = callback;
-
-            /* istanbul ignore if https://github.com/typescript-eslint/typescript-eslint/issues/734 */
-            if (!body) {
-              throw new Error(
-                `Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
-              );
-            }
-
-            const sourceCode = context.getSourceCode();
-            const firstBodyToken = sourceCode.getFirstToken(body);
-            const lastBodyToken = sourceCode.getLastToken(body);
-            const tokenBeforeArgument = sourceCode.getTokenBefore(argument);
-            const tokenAfterArgument = sourceCode.getTokenAfter(argument);
-
-            /* istanbul ignore if */
-            if (
-              !('name' in argument) ||
-              !firstBodyToken ||
-              !lastBodyToken ||
-              !tokenBeforeArgument ||
-              !tokenAfterArgument
-            ) {
-              throw new Error(
-                `Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
-              );
-            }
-
-            const argumentInParens =
-              tokenBeforeArgument.value === '(' &&
-              tokenAfterArgument.value === ')';
-
-            let argumentFix = fixer.replaceText(argument, '()');
-
-            if (argumentInParens) {
-              argumentFix = fixer.remove(argument);
-            }
-
-            let newCallback = argument.name;
-
-            if (argumentInParens) {
-              newCallback = `(${newCallback})`;
-            }
-
-            let beforeReplacement = `new Promise(${newCallback} => `;
-            let afterReplacement = ')';
-            let replaceBefore = true;
-
-            if (body.type === AST_NODE_TYPES.BlockStatement) {
-              const keyword = 'return';
-
-              beforeReplacement = `${keyword} ${beforeReplacement}{`;
-              afterReplacement += '}';
-              replaceBefore = false;
-            }
-
-            return [
-              argumentFix,
-              replaceBefore
-                ? fixer.insertTextBefore(firstBodyToken, beforeReplacement)
-                : fixer.insertTextAfter(firstBodyToken, beforeReplacement),
-              fixer.insertTextAfter(lastBodyToken, afterReplacement),
-            ];
-          },
+          suggest: [
+            {
+              messageId: 'suggestWrappingInPromise',
+              data: { callback: argument.name },
+              fix(fixer) {
+                const { body } = callback;
+
+                /* istanbul ignore if https://github.com/typescript-eslint/typescript-eslint/issues/734 */
+                if (!body) {
+                  throw new Error(
+                    `Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
+                  );
+                }
+
+                const sourceCode = context.getSourceCode();
+                const firstBodyToken = sourceCode.getFirstToken(body);
+                const lastBodyToken = sourceCode.getLastToken(body);
+                const tokenBeforeArgument = sourceCode.getTokenBefore(argument);
+                const tokenAfterArgument = sourceCode.getTokenAfter(argument);
+
+                /* istanbul ignore if */
+                if (
+                  !firstBodyToken ||
+                  !lastBodyToken ||
+                  !tokenBeforeArgument ||
+                  !tokenAfterArgument
+                ) {
+                  throw new Error(
+                    `Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
+                  );
+                }
+
+                const argumentInParens =
+                  tokenBeforeArgument.value === '(' &&
+                  tokenAfterArgument.value === ')';
+
+                let argumentFix = fixer.replaceText(argument, '()');
+
+                if (argumentInParens) {
+                  argumentFix = fixer.remove(argument);
+                }
+
+                let newCallback = argument.name;
+
+                if (argumentInParens) {
+                  newCallback = `(${newCallback})`;
+                }
+
+                let beforeReplacement = `new Promise(${newCallback} => `;
+                let afterReplacement = ')';
+                let replaceBefore = true;
+
+                if (body.type === AST_NODE_TYPES.BlockStatement) {
+                  const keyword = 'return';
+
+                  beforeReplacement = `${keyword} ${beforeReplacement}{`;
+                  afterReplacement += '}';
+                  replaceBefore = false;
+                }
+
+                return [
+                  argumentFix,
+                  replaceBefore
+                    ? fixer.insertTextBefore(firstBodyToken, beforeReplacement)
+                    : fixer.insertTextAfter(firstBodyToken, beforeReplacement),
+                  fixer.insertTextAfter(lastBodyToken, afterReplacement),
+                ];
+              },
+            },
+          ],
         });
       },
     };