feat: show a warning if vi.mock or vi.hoisted are declared outside of top level of the module (#9387)

Author: sheremet-va

packages/mocker/src/node/esmWalker.ts

@@ -5,6 +5,7 @@ import type {
   Identifier,
   ImportExpression,
   Literal,
+  MetaProperty,
   Pattern,
   Property,
   VariableDeclaration,
@@ -43,7 +44,7 @@ interface Visitors {
     info: IdentifierInfo,
     parentStack: Node[],
   ) => void
-  onImportMeta?: (node: Node) => void
+  onImportMeta?: (node: Positioned<MetaProperty>) => void
   onDynamicImport?: (node: Positioned<ImportExpression>) => void
   onCallExpression?: (node: Positioned<CallExpression>) => void
 }
@@ -142,7 +143,7 @@ export function esmWalker(
       }
 
       if (node.type === 'MetaProperty' && node.meta.name === 'import') {
-        onImportMeta?.(node as Node)
+        onImportMeta?.(node as Positioned<MetaProperty>)
       }
       else if (node.type === 'ImportExpression') {
         onDynamicImport?.(node as Positioned<ImportExpression>)

packages/mocker/src/node/hoistMocksPlugin.ts

@@ -237,9 +237,9 @@ export function hoistMocks(
   }
 
   const declaredConst = new Set<string>()
-  const hoistedNodes: Positioned<
+  const hoistedNodes: Set<Positioned<
   CallExpression | VariableDeclaration | AwaitExpression
-  >[] = []
+  >> = new Set()
 
   function createSyntaxError(node: Positioned<Node>, message: string) {
     const _error = new SyntaxError(message)
@@ -304,8 +304,15 @@ export function hoistMocks(
   }
 
   const usedUtilityExports = new Set<string>()
+  let hasImportMetaVitest = false
 
   esmWalker(ast, {
+    onImportMeta(node) {
+      const property = code.slice(node.end, node.end + 7) // '.vitest'.length
+      if (property === '.vitest') {
+        hasImportMetaVitest = true
+      }
+    },
     onIdentifier(id, info, parentStack) {
       const binding = idToImportMap.get(id.name)
       if (!binding) {
@@ -382,7 +389,7 @@ export function hoistMocks(
               )
             }
           }
-          hoistedNodes.push(node)
+          hoistedNodes.add(node)
         }
         // vi.doMock(import('./path')) -> vi.doMock('./path')
         // vi.doMock(await import('./path')) -> vi.doMock('./path')
@@ -420,7 +427,7 @@ export function hoistMocks(
               'Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first.',
             )
             // hoist "const variable = vi.hoisted(() => {})"
-            hoistedNodes.push(declarationNode)
+            hoistedNodes.add(declarationNode)
           }
           else {
             const awaitedExpression = findNodeAround(
@@ -430,7 +437,7 @@ export function hoistMocks(
             )?.node as Positioned<AwaitExpression> | undefined
             // hoist "await vi.hoisted(async () => {})" or "vi.hoisted(() => {})"
             const moveNode = awaitedExpression?.argument === node ? awaitedExpression : node
-            hoistedNodes.push(moveNode)
+            hoistedNodes.add(moveNode)
           }
         }
       }
@@ -444,7 +451,11 @@ export function hoistMocks(
       && isIdentifier(callee.property)
       && isIdentifier(callee.object)
     ) {
-      return `${callee.object.name}.${callee.property.name}()`
+      const argument = node.arguments[0] as Positioned<Expression>
+      const argStr = argument.type === 'Literal' || argument.type === 'ImportExpression'
+        ? code.slice(argument.start, argument.end)
+        : ''
+      return `${callee.object.name}.${callee.property.name}(${argStr})`
     }
     return '"hoisted method"'
   }
@@ -481,10 +492,11 @@ export function hoistMocks(
   }
 
   // validate hoistedNodes doesn't have nodes inside other nodes
-  for (let i = 0; i < hoistedNodes.length; i++) {
-    const node = hoistedNodes[i]
-    for (let j = i + 1; j < hoistedNodes.length; j++) {
-      const otherNode = hoistedNodes[j]
+  const arrayNodes = Array.from(hoistedNodes)
+  for (let i = 0; i < arrayNodes.length; i++) {
+    const node = arrayNodes[i]
+    for (let j = i + 1; j < arrayNodes.length; j++) {
+      const otherNode = arrayNodes[j]
 
       if (node.start >= otherNode.start && node.end <= otherNode.end) {
         throw createError(otherNode, node)
@@ -495,8 +507,29 @@ export function hoistMocks(
     }
   }
 
+  // validate that hoisted nodes are defined on the top level
+  // ignore `import.meta.vitest` because it needs to be inside an IfStatement
+  // and it can be used anywhere in the code (inside methods too)
+  if (!hasImportMetaVitest) {
+    for (const node of ast.body as Node[]) {
+      hoistedNodes.delete(node as any)
+      if (node.type === 'ExpressionStatement') {
+        hoistedNodes.delete(node.expression as any)
+      }
+    }
+
+    for (const invalidNode of hoistedNodes) {
+      console.warn(
+        `Warning: A ${getNodeName(getNodeCall(invalidNode))} call in "${id}" is not at the top level of the module. `
+        + `Although it appears nested, it will be hoisted and executed before any tests run. `
+        + `Move it to the top level to reflect its actual execution order. This will become an error in a future version.\n`
+        + `See: https://vitest.dev/guide/mocking/modules#how-it-works`,
+      )
+    }
+  }
+
   // hoist vi.mock/vi.hoisted
-  for (const node of hoistedNodes) {
+  for (const node of arrayNodes) {
     const end = getNodeTail(code, node)
     // don't hoist into itself if it's already at the top
     if (hoistIndex === end || hoistIndex === node.start) {
@@ -530,7 +563,7 @@ export function hoistMocks(
     }
   }
 
-  if (!hoistedModuleImported && hoistedNodes.length) {
+  if (!hoistedModuleImported && arrayNodes.length > 0) {
     const utilityImports = [...usedUtilityExports]
     // "vi" or "vitest" is imported from a module other than "vitest"
     if (utilityImports.some(name => idToImportMap.has(name))) {

test/core/test/__snapshots__/injector-mock.test.ts.snap

@@ -1,6 +1,6 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if awaited assigned vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if awaited assigned vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock('./mocked'): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if awaited assigned vi.hoisted is called inside vi.mock 2`] = `
 "      2| 
@@ -11,7 +11,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error
       6| "
 `;
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock('./mocked'): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 2`] = `
 "      2| 
@@ -44,7 +44,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error
       5| })"
 `;
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock('./mocked'): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is called inside vi.mock 2`] = `
 "      2| 
@@ -77,7 +77,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error
       5| })"
 `;
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 2`] = `
 "      2| 
@@ -88,7 +88,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error
       6| "
 `;
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 2`] = `
 "      2| 
@@ -99,7 +99,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error
       6| "
 `;
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 2`] = `
 "      2| 
@@ -110,7 +110,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error
       6| "
 `;
 
-exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
+exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`;
 
 exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 2`] = `
 "      2| 

test/core/test/injector-mock.test.ts

@@ -1,7 +1,7 @@
 import type { HoistMocksPluginOptions } from '../../../packages/mocker/src/node/hoistMocksPlugin'
 import { stripVTControlCharacters } from 'node:util'
 import { parseAst } from 'vite'
-import { describe, expect, it, test } from 'vitest'
+import { describe, expect, it, test, vi } from 'vitest'
 import { hoistMocks } from '../../../packages/mocker/src/node/hoistMocksPlugin'
 import { generateCodeFrame } from '../../../packages/vitest/src/node/printError.js'
 
@@ -1554,4 +1554,147 @@ export const mocked = vi.unmock('./mocked')
     expect(error.message).toMatchSnapshot()
     expect(stripVTControlCharacters(error.frame)).toMatchSnapshot()
   })
+
+  it('shows an error when hoisted methods are used outside the top level scope', ({ onTestFinished }) => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    onTestFinished(() => warn.mockRestore())
+    const result = hoistSimpleCode(`
+// correct
+vi.mock('./hello-world-1')
+
+if (condition) {
+  vi.mock('./hello-world-2')
+}
+
+test('some test', () => {
+  vi.mock('./hello-world-3')
+})
+
+test('some test', () => {
+  if (condition) {
+    vi.mock('./hello-world-4')
+    vi.hoisted(() => {})
+    vi.mock(import('./hello-world-5'))
+    const variable = vi.hoisted(() => {})
+  }
+})
+
+describe('some suite', () => {
+  if (condition) {
+    vi.mock('./hello-world-6')
+  }
+})
+      `)
+    expect(result).toMatchInlineSnapshot(`
+      "import { vi } from "vitest"
+      vi.mock('./hello-world-1')
+      vi.mock('./hello-world-2')
+      vi.mock('./hello-world-3')
+      vi.mock('./hello-world-4')
+      vi.hoisted(() => {})
+      vi.mock('./hello-world-5')
+      const variable = vi.hoisted(() => {})
+      vi.mock('./hello-world-6')
+
+      // correct
+
+      if (condition) {
+        }
+
+      test('some test', () => {
+        })
+
+      test('some test', () => {
+        if (condition) {
+                        }
+      })
+
+      describe('some suite', () => {
+        if (condition) {
+            }
+      })"
+    `)
+    expect(warn).toMatchInlineSnapshot(`
+      [MockFunction warn] {
+        "calls": [
+          [
+            "Warning: A vi.mock('./hello-world-2') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+          [
+            "Warning: A vi.mock('./hello-world-3') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+          [
+            "Warning: A vi.mock('./hello-world-4') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+          [
+            "Warning: A vi.hoisted() call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+          [
+            "Warning: A vi.mock(import('./hello-world-5')) call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+          [
+            "Warning: A vi.hoisted() call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+          [
+            "Warning: A vi.mock('./hello-world-6') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version.
+      See: https://vitest.dev/guide/mocking/modules#how-it-works",
+          ],
+        ],
+        "results": [
+          {
+            "type": "return",
+            "value": undefined,
+          },
+          {
+            "type": "return",
+            "value": undefined,
+          },
+          {
+            "type": "return",
+            "value": undefined,
+          },
+          {
+            "type": "return",
+            "value": undefined,
+          },
+          {
+            "type": "return",
+            "value": undefined,
+          },
+          {
+            "type": "return",
+            "value": undefined,
+          },
+          {
+            "type": "return",
+            "value": undefined,
+          },
+        ],
+      }
+    `)
+  })
+
+  it('ignores vi.mock position if import.meta.vitest is present', ({ onTestFinished }) => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    onTestFinished(() => warn.mockRestore())
+    const result = hoistSimpleCode(`
+if (import.meta.vitest) {
+  vi.mock('./hello-world-1')
+}
+      `)
+    expect(result).toMatchInlineSnapshot(`
+      "import { vi } from "vitest"
+      vi.mock('./hello-world-1')
+
+      if (import.meta.vitest) {
+        }"
+    `)
+    expect(warn).not.toHaveBeenCalled()
+  })
 })