From f8bff9efb8bba79e6f6207decc35442d510acf2d Mon Sep 17 00:00:00 2001 From: Vladimir Date: Wed, 17 Jan 2024 15:43:09 +0100 Subject: [PATCH] fix(vitest): throw a syntax error if vi.hoisted is directly exported (#4969) --- packages/vitest/package.json | 2 +- packages/vitest/src/node/hoistMocks.ts | 37 +++-- pnpm-lock.yaml | 10 +- .../__snapshots__/injector-mock.test.ts.snap | 148 ++++++++++++------ test/core/test/injector-mock.test.ts | 103 +++++++++--- 5 files changed, 209 insertions(+), 91 deletions(-) diff --git a/packages/vitest/package.json b/packages/vitest/package.json index 701238333f0f..c4792f1f5faa 100644 --- a/packages/vitest/package.json +++ b/packages/vitest/package.json @@ -146,7 +146,7 @@ "@vitest/snapshot": "workspace:*", "@vitest/spy": "workspace:*", "@vitest/utils": "workspace:*", - "acorn-walk": "^8.3.1", + "acorn-walk": "^8.3.2", "cac": "^6.7.14", "chai": "^4.3.10", "debug": "^4.3.4", diff --git a/packages/vitest/src/node/hoistMocks.ts b/packages/vitest/src/node/hoistMocks.ts index e9ba9cd819b8..d824397201b4 100644 --- a/packages/vitest/src/node/hoistMocks.ts +++ b/packages/vitest/src/node/hoistMocks.ts @@ -1,7 +1,5 @@ import MagicString from 'magic-string' -import type { AwaitExpression, CallExpression, Identifier, ImportDeclaration, VariableDeclaration, Node as _Node } from 'estree' - -// TODO: should use findNodeBefore, but it's not typed +import type { AwaitExpression, CallExpression, ExportDefaultDeclaration, ExportNamedDeclaration, Identifier, ImportDeclaration, VariableDeclaration, Node as _Node } from 'estree' import { findNodeAround } from 'acorn-walk' import type { PluginContext } from 'rollup' import { esmWalker } from '@vitest/utils/ast' @@ -153,6 +151,17 @@ export function hoistMocks(code: string, id: string, parse: PluginContext['parse const declaredConst = new Set() const hoistedNodes: Positioned[] = [] + function createSyntaxError(node: Positioned, message: string) { + const _error = new SyntaxError(message) + Error.captureStackTrace(_error, createSyntaxError) + return { + name: 'SyntaxError', + message: _error.message, + stack: _error.stack, + frame: generateCodeFrame(highlightCode(id, code, colors), 4, node.start + 1), + } + } + esmWalker(ast, { onIdentifier(id, info, parentStack) { const binding = idToImportMap.get(id.name) @@ -192,6 +201,11 @@ export function hoistMocks(code: string, id: string, parse: PluginContext['parse hoistedNodes.push(node) if (methodName === 'hoisted') { + // check it's not a default export + const defaultExport = findNodeAround(ast, node.start, 'ExportDefaultDeclaration')?.node as Positioned | undefined + if (defaultExport?.declaration === node || (defaultExport?.declaration.type === 'AwaitExpression' && defaultExport.declaration.argument === node)) + throw createSyntaxError(defaultExport, 'Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first.') + const declarationNode = findNodeAround(ast, node.start, 'VariableDeclaration')?.node as Positioned | undefined const init = declarationNode?.declarations[0]?.init const isViHoisted = (node: CallExpression) => { @@ -211,6 +225,10 @@ export function hoistMocks(code: string, id: string, parse: PluginContext['parse && isViHoisted(init.argument)) /* const v = await vi.hoisted() */ if (canMoveDeclaration) { + // export const variable = vi.hoisted() + const nodeExported = findNodeAround(ast, declarationNode.start, 'ExportNamedDeclaration')?.node as Positioned | undefined + if (nodeExported?.declaration === declarationNode) + throw createSyntaxError(nodeExported, '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) } @@ -251,15 +269,10 @@ export function hoistMocks(code: string, id: string, parse: PluginContext['parse 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(highlightCode(id, code, colors), 4, insideCall.start + 1), - } - throw error + throw createSyntaxError( + insideCall, + `Cannot call ${getNodeName(insideCall)} inside ${getNodeName(outsideCall)}: both methods are hoisted to the top of the file and not actually called inside each other.`, + ) } // validate hoistedNodes doesn't have nodes inside other nodes diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index afbdedaabc19..a91783d65eb9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1285,8 +1285,8 @@ importers: specifier: workspace:* version: link:../utils acorn-walk: - specifier: ^8.3.1 - version: 8.3.1 + specifier: ^8.3.2 + version: 8.3.2 cac: specifier: ^6.7.14 version: 6.7.14 @@ -11376,7 +11376,7 @@ packages: resolution: {integrity: sha512-umOSDSDrfHbTNPuNpC2NSnnA3LUrqpevPb4T9jRx4MagXNS0rs+gwiTcAvqCRmsD6utzsrzNt+ebm00SNWiC3Q==} dependencies: acorn: 8.11.2 - acorn-walk: 8.3.1 + acorn-walk: 8.3.2 dev: true /acorn-import-assertions@1.8.0(acorn@8.11.2): @@ -11416,8 +11416,8 @@ packages: engines: {node: '>=0.4.0'} dev: true - /acorn-walk@8.3.1: - resolution: {integrity: sha512-TgUZgYvqZprrl7YldZNoa9OciCAyZR+Ejm9eXzKCmjsF5IKp/wgQ7Z/ZpjpGTIUPwrHQIcYeI8qDh4PsEwxMbw==} + /acorn-walk@8.3.2: + resolution: {integrity: sha512-cjkyv4OtNCIeqhHrfS81QWXoCBPExR/J62oyEqepVw8WaQeSqpW2uhuLPh1m9eWhDuOo/jUXVTlifvesOWp/4A==} engines: {node: '>=0.4.0'} /acorn@6.4.2: diff --git a/test/core/test/__snapshots__/injector-mock.test.ts.snap b/test/core/test/__snapshots__/injector-mock.test.ts.snap index bca1aaa629e8..81040cdd4e93 100644 --- a/test/core/test/__snapshots__/injector-mock.test.ts.snap +++ b/test/core/test/__snapshots__/injector-mock.test.ts.snap @@ -1,78 +1,122 @@ // 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 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 2`] = ` +" 2| + 3| vi.mock('./mocked', async () => { + 4| const variable = await vi.hoisted(() => 1) + | ^ + 5| }) + 6| " `; -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 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 4`] = ` -" 3| - 4| vi.mock('./mocked', async () => { - 5| await vi.hoisted(() => 1) +exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 2`] = ` +" 2| + 3| vi.mock('./mocked', async () => { + 4| await vi.hoisted(() => 1) | ^ - 6| }) - 7| " + 5| }) + 6| " `; -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 if awaited vi.hoisted is exported as default export 1`] = `"Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first."`; -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 if awaited vi.hoisted is exported as default export 2`] = ` +" 1| import { vi } from 'vitest' + 2| + 3| export default await vi.hoisted(async () => { + | ^ + 4| return {} + 5| })" +`; + +exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is exported as named export 1`] = `"Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first."`; + +exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is exported as named export 2`] = ` +" 1| import { vi } from 'vitest' + 2| + 3| export const values = await vi.hoisted(async () => { + | ^ + 4| return {} + 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 2`] = ` +" 2| + 3| vi.mock('./mocked', () => { + 4| const variable = vi.hoisted(() => 1) + | ^ + 5| console.log(variable) + 6| })" +`; + +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is exported as a named export 1`] = `"Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first."`; + +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is exported as a named export 2`] = ` +" 1| import { vi } from 'vitest' + 2| + 3| export const values = vi.hoisted(async () => { + | ^ + 4| return {} + 5| })" +`; + +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is exported as default 1`] = `"Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first."`; + +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is exported as default 2`] = ` +" 1| import { vi } from 'vitest' + 2| + 3| export default vi.hoisted(() => { + | ^ + 4| return {} + 5| })" `; -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 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 8`] = ` -" 3| - 4| vi.hoisted(() => { - 5| vi.mock('./mocked') +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 2`] = ` +" 2| + 3| vi.hoisted(() => { + 4| vi.mock('./mocked') | ^ - 6| }) - 7| " + 5| }) + 6| " `; -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 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 10`] = ` -" 3| - 4| const values = vi.hoisted(() => { - 5| vi.mock('./mocked') +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 2`] = ` +" 2| + 3| const values = await vi.hoisted(async () => { + 4| vi.mock('./mocked') | ^ - 6| }) - 7| " + 5| }) + 6| " `; -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 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 12`] = ` -" 3| - 4| await vi.hoisted(async () => { - 5| vi.mock('./mocked') +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 2`] = ` +" 2| + 3| const values = vi.hoisted(() => { + 4| vi.mock('./mocked') | ^ - 6| }) - 7| " + 5| }) + 6| " `; -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 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 14`] = ` -" 3| - 4| const values = await vi.hoisted(async () => { - 5| vi.mock('./mocked') +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 2`] = ` +" 2| + 3| await vi.hoisted(async () => { + 4| vi.mock('./mocked') | ^ - 6| }) - 7| " + 5| }) + 6| " `; diff --git a/test/core/test/injector-mock.test.ts b/test/core/test/injector-mock.test.ts index 6a160fba0955..7d730317540c 100644 --- a/test/core/test/injector-mock.test.ts +++ b/test/core/test/injector-mock.test.ts @@ -1214,57 +1214,118 @@ describe('throws an error when nodes are incompatible', () => { } it.each([ - ` - import { vi } from 'vitest' - - vi.mock('./mocked', () => { - const variable = vi.hoisted(() => 1) - console.log(variable) - }) - `, - ` + [ + 'vi.hoisted is called inside vi.mock', + `\ +import { vi } from 'vitest' + +vi.mock('./mocked', () => { + const variable = vi.hoisted(() => 1) + console.log(variable) +}) +`, + ], + [ + 'awaited vi.hoisted is called inside vi.mock', + `\ import { vi } from 'vitest' vi.mock('./mocked', async () => { await vi.hoisted(() => 1) }) - `, - ` +`, + ], + [ + 'awaited assigned vi.hoisted is called inside vi.mock', + `\ import { vi } from 'vitest' vi.mock('./mocked', async () => { const variable = await vi.hoisted(() => 1) }) - `, - ` +`, + ], + [ + 'vi.mock inside vi.hoisted', + `\ import { vi } from 'vitest' vi.hoisted(() => { vi.mock('./mocked') }) - `, - ` +`, + ], + [ + 'vi.mock is called inside assigned vi.hoisted', + `\ import { vi } from 'vitest' const values = vi.hoisted(() => { vi.mock('./mocked') }) - `, - ` +`, + ], + [ + 'vi.mock is called inside awaited vi.hoisted', + `\ import { vi } from 'vitest' await vi.hoisted(async () => { vi.mock('./mocked') }) - `, - ` +`, + ], + [ + 'vi.mock is called inside assigned awaited vi.hoisted', + `\ import { vi } from 'vitest' const values = await vi.hoisted(async () => { vi.mock('./mocked') }) - `, - ])('correctly throws an error', (code) => { +`, + ], + [ + 'vi.hoisted is exported as a named export', + `\ +import { vi } from 'vitest' + +export const values = vi.hoisted(async () => { + return {} +}) +`, + ], + [ + 'vi.hoisted is exported as default', + `\ +import { vi } from 'vitest' + +export default vi.hoisted(() => { + return {} +}) +`, + ], + [ + 'awaited vi.hoisted is exported as named export', + `\ +import { vi } from 'vitest' + +export const values = await vi.hoisted(async () => { + return {} +}) +`, + ], + [ + 'awaited vi.hoisted is exported as default export', + `\ +import { vi } from 'vitest' + +export default await vi.hoisted(async () => { + return {} +}) +`, + ], + ])('correctly throws an error if %s', (_, code) => { const error = getErrorWhileHoisting(code) expect(error.message).toMatchSnapshot() expect(stripAnsi(error.frame)).toMatchSnapshot()