From dbb9a35147c5185dc6727381279a93c3d41744f6 Mon Sep 17 00:00:00 2001
From: Erin Zimmer <ezimmer@atlassian.com>
Date: Fri, 30 Aug 2024 09:43:10 +1000
Subject: [PATCH] fix(prefer-importing-jest-globals): ensure imports aren't
 inserted in the middle of a statement

---
 .../prefer-importing-jest-globals.test.ts     | 595 ++++++++++--------
 src/rules/prefer-importing-jest-globals.ts    |  22 +-
 2 files changed, 346 insertions(+), 271 deletions(-)

diff --git a/src/rules/__tests__/prefer-importing-jest-globals.test.ts b/src/rules/__tests__/prefer-importing-jest-globals.test.ts
index 19bc24b8b..d1f14a110 100644
--- a/src/rules/__tests__/prefer-importing-jest-globals.test.ts
+++ b/src/rules/__tests__/prefer-importing-jest-globals.test.ts
@@ -24,76 +24,76 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        test('should pass', () => {
-            expect(true).toBeDefined();
-        });
-      `,
+            test('should pass', () => {
+                expect(true).toBeDefined();
+            });
+          `,
       options: [{ types: ['jest'] }],
       parserOptions: { sourceType: 'module' },
     },
     {
       code: dedent`
-        const { it } = require('@jest/globals');
-        it('should pass', () => {
-            expect(true).toBeDefined();
-        });
-      `,
+            const { it } = require('@jest/globals');
+            it('should pass', () => {
+                expect(true).toBeDefined();
+            });
+          `,
       options: [{ types: ['test'] }],
       parserOptions: { sourceType: 'module' },
     },
     {
       code: dedent`
-        // with require
-        const { test, expect } = require('@jest/globals');
-        test('should pass', () => {
-            expect(true).toBeDefined();
-        });
-      `,
+            // with require
+            const { test, expect } = require('@jest/globals');
+            test('should pass', () => {
+                expect(true).toBeDefined();
+            });
+          `,
     },
     {
       code: dedent`
-        const { test, expect } = require(\`@jest/globals\`);
-        test('should pass', () => {
-            expect(true).toBeDefined();
-        });
-      `,
+            const { test, expect } = require(\`@jest/globals\`);
+            test('should pass', () => {
+                expect(true).toBeDefined();
+            });
+          `,
     },
     {
       code: dedent`
-        import { it as itChecks } from '@jest/globals';
-        itChecks("foo");
-      `,
+            import { it as itChecks } from '@jest/globals';
+            itChecks("foo");
+          `,
       parserOptions: { sourceType: 'module' },
     },
     {
       code: dedent`
-        const { test } = require('@jest/globals');
-        test("foo");
-      `,
+            const { test } = require('@jest/globals');
+            test("foo");
+          `,
     },
     {
       code: dedent`
-        const { test } = require('my-test-library');
-        test("foo");
-      `,
+            const { test } = require('my-test-library');
+            test("foo");
+          `,
     },
   ],
   invalid: [
     {
       code: dedent`
-        import describe from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            import describe from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        import { describe, expect, test } from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            import { describe, expect, test } from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       parserOptions: { sourceType: 'module' },
       errors: [
         {
@@ -106,20 +106,20 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        jest.useFakeTimers();
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            jest.useFakeTimers();
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        import { jest } from '@jest/globals';
-        jest.useFakeTimers();
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            import { jest } from '@jest/globals';
+            jest.useFakeTimers();
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       options: [{ types: ['jest'] }],
       parserOptions: { sourceType: 'module' },
       errors: [
@@ -133,27 +133,27 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        import React from 'react';
-        import { yourFunction } from './yourFile';
-        import something from "something";
-        import { test } from '@jest/globals';
-        import { xit } from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            import React from 'react';
+            import { yourFunction } from './yourFile';
+            import something from "something";
+            import { test } from '@jest/globals';
+            import { xit } from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        import React from 'react';
-        import { yourFunction } from './yourFile';
-        import something from "something";
-        import { describe, expect, test } from '@jest/globals';
-        import { xit } from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            import React from 'react';
+            import { yourFunction } from './yourFile';
+            import something from "something";
+            import { describe, expect, test } from '@jest/globals';
+            import { xit } from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       parserOptions: { sourceType: 'module' },
       errors: [
         {
@@ -166,23 +166,23 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        console.log('hello');
-        import * as fs from 'fs';
-        const { test, 'describe': describe } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            import * as fs from 'fs';
+            const { test, 'describe': describe } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        console.log('hello');
-        import * as fs from 'fs';
-        import { describe, expect, test } from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            import * as fs from 'fs';
+            import { describe, expect, test } from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       parserOptions: { sourceType: 'module' },
       errors: [
         {
@@ -195,21 +195,21 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        console.log('hello');
-        import jestGlobals from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            import jestGlobals from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        console.log('hello');
-        import { describe, expect, jestGlobals, test } from '@jest/globals';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            import { describe, expect, jestGlobals, test } from '@jest/globals';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       parserOptions: { sourceType: 'module' },
       errors: [
         {
@@ -222,18 +222,18 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        import { pending } from 'actions';
-        describe('foo', () => {
-          test.each(['hello', 'world'])("%s", (a) => {});
-        });
-      `,
+            import { pending } from 'actions';
+            describe('foo', () => {
+              test.each(['hello', 'world'])("%s", (a) => {});
+            });
+          `,
       output: dedent`
-        import { pending } from 'actions';
-        import { describe, test } from '@jest/globals';
-        describe('foo', () => {
-          test.each(['hello', 'world'])("%s", (a) => {});
-        });
-      `,
+            import { describe, test } from '@jest/globals';
+            import { pending } from 'actions';
+            describe('foo', () => {
+              test.each(['hello', 'world'])("%s", (a) => {});
+            });
+          `,
       parserOptions: { sourceType: 'module' },
       errors: [
         {
@@ -246,19 +246,19 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        const {describe} = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const {describe} = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 7,
@@ -270,20 +270,20 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        const {describe} = require(\`@jest/globals\`);
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const {describe} = require(\`@jest/globals\`);
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       // todo: we should really maintain the template literals
       output: dedent`
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 7,
@@ -295,23 +295,23 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        const source = 'globals';
-        const {describe} = require(\`@jest/\${source}\`);
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const source = 'globals';
+            const {describe} = require(\`@jest/\${source}\`);
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       // todo: this shouldn't be indenting the "test"
       output: dedent`
-        const source = 'globals';
-        const {describe} = require(\`@jest/\${source}\`);
-        describe("suite", () => {
-          const { expect, test } = require('@jest/globals');
-        test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const source = 'globals';
+            const {describe} = require(\`@jest/\${source}\`);
+            describe("suite", () => {
+              const { expect, test } = require('@jest/globals');
+            test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 7,
@@ -323,19 +323,19 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        const { [() => {}]: it } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const { [() => {}]: it } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -347,23 +347,23 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        console.log('hello');
-        const fs = require('fs');
-        const { test, 'describe': describe } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            const fs = require('fs');
+            const { test, 'describe': describe } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        console.log('hello');
-        const fs = require('fs');
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            const fs = require('fs');
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -375,21 +375,21 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        console.log('hello');
-        const jestGlobals = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            const jestGlobals = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        console.log('hello');
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            console.log('hello');
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -401,18 +401,18 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        const { pending } = require('actions');
-        describe('foo', () => {
-          test.each(['hello', 'world'])("%s", (a) => {});
-        });
-      `,
+            const { pending } = require('actions');
+            describe('foo', () => {
+              test.each(['hello', 'world'])("%s", (a) => {});
+            });
+          `,
       output: dedent`
-        const { pending } = require('actions');
-        const { describe, test } = require('@jest/globals');
-        describe('foo', () => {
-          test.each(['hello', 'world'])("%s", (a) => {});
-        });
-      `,
+            const { pending } = require('actions');
+            const { describe, test } = require('@jest/globals');
+            describe('foo', () => {
+              test.each(['hello', 'world'])("%s", (a) => {});
+            });
+          `,
       errors: [
         {
           endColumn: 9,
@@ -424,18 +424,18 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -447,20 +447,20 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        #!/usr/bin/env node
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            #!/usr/bin/env node
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        #!/usr/bin/env node
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            #!/usr/bin/env node
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       parserOptions: { sourceType: 'script' },
       errors: [
         {
@@ -473,20 +473,20 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        // with comment above
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            // with comment above
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        // with comment above
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            // with comment above
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -498,20 +498,20 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        'use strict';
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            'use strict';
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        'use strict';
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            'use strict';
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -523,20 +523,20 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
     },
     {
       code: dedent`
-        \`use strict\`;
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            \`use strict\`;
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       output: dedent`
-        \`use strict\`;
-        const { describe, expect, test } = require('@jest/globals');
-        describe("suite", () => {
-          test("foo");
-          expect(true).toBeDefined();
-        })
-      `,
+            \`use strict\`;
+            const { describe, expect, test } = require('@jest/globals');
+            describe("suite", () => {
+              test("foo");
+              expect(true).toBeDefined();
+            })
+          `,
       errors: [
         {
           endColumn: 9,
@@ -546,5 +546,60 @@ ruleTester.run('prefer-importing-jest-globals', rule, {
         },
       ],
     },
+    {
+      code: dedent`
+          console.log('hello');
+          const onClick = jest.fn();
+          describe("suite", () => {
+            test("foo");
+            expect(onClick).toHaveBeenCalled();
+          })
+          `,
+      output: dedent`
+          console.log('hello');
+          const { describe, expect, jest, test } = require('@jest/globals');
+          const onClick = jest.fn();
+          describe("suite", () => {
+            test("foo");
+            expect(onClick).toHaveBeenCalled();
+          })
+          `,
+      errors: [
+        {
+          endColumn: 21,
+          column: 17,
+          line: 2,
+          messageId: 'preferImportingJestGlobal',
+        },
+      ],
+    },
+    {
+      code: dedent`
+      console.log('hello');
+      const onClick = jest.fn();
+      describe("suite", () => {
+        test("foo");
+        expect(onClick).toHaveBeenCalled();
+      })
+      `,
+      output: dedent`
+      import { describe, expect, jest, test } from '@jest/globals';
+      console.log('hello');
+      const onClick = jest.fn();
+      describe("suite", () => {
+        test("foo");
+        expect(onClick).toHaveBeenCalled();
+      })
+      `,
+      parserOptions: { sourceType: 'module' },
+      errors: [
+        {
+          endColumn: 21,
+          column: 17,
+          line: 2,
+          messageId: 'preferImportingJestGlobal',
+        },
+      ],
+    },
   ],
 });
diff --git a/src/rules/prefer-importing-jest-globals.ts b/src/rules/prefer-importing-jest-globals.ts
index 9cce2d213..532a4bfaf 100644
--- a/src/rules/prefer-importing-jest-globals.ts
+++ b/src/rules/prefer-importing-jest-globals.ts
@@ -21,6 +21,22 @@ const createFixerImports = (
     : `const { ${allImportsFormatted} } = require('@jest/globals');`;
 };
 
+const findInsertionPoint = (reportingNode: TSESTree.Node) => {
+  let currentNode = reportingNode;
+
+  while (
+    currentNode.parent &&
+    currentNode.parent.type !== AST_NODE_TYPES.Program &&
+    currentNode.parent.type !== AST_NODE_TYPES.VariableDeclaration
+  ) {
+    currentNode = currentNode.parent;
+  }
+
+  return currentNode.parent?.type === AST_NODE_TYPES.VariableDeclaration
+    ? currentNode.parent
+    : reportingNode;
+};
+
 const allJestFnTypes: JestFnType[] = [
   'hook',
   'describe',
@@ -158,8 +174,12 @@ export default createRule({
             );
 
             if (requireNode?.type !== AST_NODE_TYPES.VariableDeclaration) {
+              const insertBeforeNode = isModule
+                ? firstNode
+                : findInsertionPoint(reportingNode);
+
               return fixer.insertTextBefore(
-                reportingNode,
+                insertBeforeNode,
                 `${createFixerImports(isModule, functionsToImport)}\n`,
               );
             }