fix(vitest): show correct error when vi.hoisted is used inside vi.mock and the other way around (#4916)

Author: sheremet-va

packages/vitest/src/node/error.ts

@@ -80,7 +80,7 @@ export async function printError(error: unknown, project: WorkspaceProject | und
     printStack(project, stacks, nearest, errorProperties, (s) => {
       if (showCodeFrame && s === nearest && nearest) {
         const sourceCode = readFileSync(nearest.file, 'utf-8')
-        logger.error(generateCodeFrame(sourceCode, 4, s.line, s.column))
+        logger.error(generateCodeFrame(sourceCode, 4, s))
       }
     })
   }
@@ -248,11 +248,10 @@ function printStack(
 export function generateCodeFrame(
   source: string,
   indent = 0,
-  lineNumber: number,
-  columnNumber: number,
+  loc: { line: number; column: number } | number,
   range = 2,
 ): string {
-  const start = positionToOffset(source, lineNumber, columnNumber)
+  const start = typeof loc === 'object' ? positionToOffset(source, loc.line, loc.column) : loc
   const end = start
   const lines = source.split(lineSplitRE)
   const nl = /\r\n/.test(source) ? 2 : 1

packages/vitest/src/node/hoistMocks.ts

@@ -5,6 +5,7 @@ import type { AwaitExpression, CallExpression, Identifier, ImportDeclaration, Va
 import { findNodeAround } from 'acorn-walk'
 import type { PluginContext } from 'rollup'
 import { esmWalker } from '@vitest/utils/ast'
+import { generateCodeFrame } from './error'
 
 export type Positioned<T> = T & {
   start: number
@@ -62,9 +63,9 @@ const regexpAssignedHoisted = /=[ \t]*(\bawait|)[ \t]*\b(vi|vitest)\s*\.\s*hoist
 const hashbangRE = /^#!.*\n/
 
 export function hoistMocks(code: string, id: string, parse: PluginContext['parse']) {
-  const hasMocks = regexpHoistable.test(code) || regexpAssignedHoisted.test(code)
+  const needHoisting = regexpHoistable.test(code) || regexpAssignedHoisted.test(code)
 
-  if (!hasMocks)
+  if (!needHoisting)
     return
 
   const s = new MagicString(code)
@@ -149,7 +150,7 @@ export function hoistMocks(code: string, id: string, parse: PluginContext['parse
   }
 
   const declaredConst = new Set<string>()
-  const hoistedNodes: Node[] = []
+  const hoistedNodes: Positioned<CallExpression | VariableDeclaration | AwaitExpression>[] = []
 
   esmWalker(ast, {
     onIdentifier(id, info, parentStack) {
@@ -222,6 +223,57 @@ export function hoistMocks(code: string, id: string, parse: PluginContext['parse
     },
   })
 
+  function getNodeName(node: CallExpression) {
+    const callee = node.callee || {}
+    if (callee.type === 'MemberExpression' && isIdentifier(callee.property) && isIdentifier(callee.object))
+      return `${callee.object.name}.${callee.property.name}()`
+    return '"hoisted method"'
+  }
+
+  function getNodeCall(node: Node): Positioned<CallExpression> {
+    if (node.type === 'CallExpression')
+      return node
+    if (node.type === 'VariableDeclaration') {
+      const { declarations } = node
+      const init = declarations[0].init
+      if (init)
+        return getNodeCall(init as Node)
+    }
+    if (node.type === 'AwaitExpression') {
+      const { argument } = node
+      if (argument.type === 'CallExpression')
+        return getNodeCall(argument as Node)
+    }
+    return node as Positioned<CallExpression>
+  }
+
+  function createError(outsideNode: Node, insideNode: Node) {
+    const outsideCall = getNodeCall(outsideNode)
+    const insideCall = getNodeCall(insideNode)
+    const _error = new SyntaxError(`Cannot call ${getNodeName(insideCall)} inside ${getNodeName(outsideCall)}: both methods are hoisted to the top of the file and not actually called inside each other.`)
+    // throw an object instead of an error so it can be serialized for RPC, TODO: improve error handling in rpc serializer
+    const error = {
+      name: 'SyntaxError',
+      message: _error.message,
+      stack: _error.stack,
+      frame: generateCodeFrame(code, 4, insideCall.start + 1),
+    }
+    throw error
+  }
+
+  // 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]
+
+      if (node.start >= otherNode.start && node.end <= otherNode.end)
+        throw createError(otherNode, node)
+      if (otherNode.start >= node.start && otherNode.end <= node.end)
+        throw createError(node, otherNode)
+    }
+  }
+
   // Wait for imports to be hoisted and then hoist the mocks
   const hoistedCode = hoistedNodes.map((node) => {
     const end = getBetterEnd(code, node)

pnpm-lock.yaml

@@ -11,6 +11,7 @@
     "@vitest/expect": "workspace:*",
     "@vitest/runner": "workspace:*",
     "@vitest/utils": "workspace:*",
+    "strip-ansi": "^7.1.0",
     "tinyspy": "^1.0.2",
     "url": "^0.11.0",
     "vitest": "workspace:*"

test/core/package.json

@@ -0,0 +1,78 @@
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 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 2`] = `
+"      3|     
+      4|     vi.mock('./mocked', () => {
+      5|       const variable = vi.hoisted(() => 1)
+       |                        ^
+      6|       console.log(variable)
+      7|     })"
+`;
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 3`] = `"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 4`] = `
+"      3| 
+      4| vi.mock('./mocked', async () => {
+      5|   await vi.hoisted(() => 1)
+       |         ^
+      6| })
+      7|     "
+`;
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 5`] = `"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 6`] = `
+"      3| 
+      4| vi.mock('./mocked', async () => {
+      5|   const variable = await vi.hoisted(() => 1)
+       |                          ^
+      6| })
+      7|     "
+`;
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 7`] = `"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 8`] = `
+"      3| 
+      4| vi.hoisted(() => {
+      5|   vi.mock('./mocked')
+       |   ^
+      6| })
+      7|     "
+`;
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 9`] = `"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 10`] = `
+"      3| 
+      4| const values = vi.hoisted(() => {
+      5|   vi.mock('./mocked')
+       |   ^
+      6| })
+      7|     "
+`;
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 11`] = `"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 12`] = `
+"      3| 
+      4| await vi.hoisted(async () => {
+      5|   vi.mock('./mocked')
+       |   ^
+      6| })
+      7|     "
+`;
+
+exports[`throws an error when nodes are incompatible > correctly throws an error 13`] = `"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 14`] = `
+"      3| 
+      4| const values = await vi.hoisted(async () => {
+      5|   vi.mock('./mocked')
+       |   ^
+      6| })
+      7|     "
+`;

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

@@ -1,6 +1,6 @@
 import { parseAst } from 'rollup/parseAst'
-import { expect, test } from 'vitest'
-import { describe } from 'node:test'
+import { describe, expect, it, test } from 'vitest'
+import stripAnsi from 'strip-ansi'
 import { hoistMocks } from '../../../packages/vitest/src/node/hoistMocks'
 
 function parse(code: string, options: any) {
@@ -1183,3 +1183,71 @@ console.log(foo + 2)
     `)
   })
 })
+
+describe('throws an error when nodes are incompatible', () => {
+  const getErrorWhileHoisting = (code: string) => {
+    try {
+      hoistSimpleCode(code)
+    }
+    catch (err: any) {
+      return err
+    }
+  }
+
+  it.each([
+    `
+    import { vi } from 'vitest'
+    
+    vi.mock('./mocked', () => {
+      const variable = vi.hoisted(() => 1)
+      console.log(variable)
+    })
+        `,
+        `
+import { vi } from 'vitest'
+
+vi.mock('./mocked', async () => {
+  await vi.hoisted(() => 1)
+})
+    `,
+    `
+import { vi } from 'vitest'
+
+vi.mock('./mocked', async () => {
+  const variable = await vi.hoisted(() => 1)
+})
+    `,
+    `
+import { vi } from 'vitest'
+
+vi.hoisted(() => {
+  vi.mock('./mocked')
+})
+    `,
+    `
+import { vi } from 'vitest'
+
+const values = vi.hoisted(() => {
+  vi.mock('./mocked')
+})
+    `,
+    `
+import { vi } from 'vitest'
+
+await vi.hoisted(async () => {
+  vi.mock('./mocked')
+})
+    `,
+    `
+import { vi } from 'vitest'
+
+const values = await vi.hoisted(async () => {
+  vi.mock('./mocked')
+})
+    `,
+  ])('correctly throws an error', (code) => {
+    const error = getErrorWhileHoisting(code)
+    expect(error.message).toMatchSnapshot()
+    expect(stripAnsi(error.frame)).toMatchSnapshot()
+  })
+})