fix: throw FixtureAccessError if suite hook accesses undefined fixture

packages/expect/src/jest-expect.ts

@@ -4,7 +4,7 @@ import type { Constructable } from '@vitest/utils'
 import type { AsymmetricMatcher } from './jest-asymmetric-matchers'
 import type { Assertion, ChaiPlugin } from './types'
 import { isMockFunction } from '@vitest/spy'
-import { assertTypes } from '@vitest/utils/helpers'
+import { assertTypes, ordinal } from '@vitest/utils/helpers'
 import c from 'tinyrainbow'
 import { JEST_MATCHERS_OBJECT } from './constants'
 import {
@@ -642,12 +642,12 @@ export const JestChaiExpect: ChaiPlugin = (chai, utils) => {
       const isCalled = times <= callCount
       this.assert(
         nthCall && equalsArgumentArray(nthCall, args),
-        `expected ${ordinalOf(
+        `expected ${ordinal(
           times,
         )} "${spyName}" call to have been called with #{exp}${
           isCalled ? `` : `, but called only ${callCount} times`
         }`,
-        `expected ${ordinalOf(
+        `expected ${ordinal(
           times,
         )} "${spyName}" call to not have been called with #{exp}`,
         args,
@@ -1046,7 +1046,7 @@ export const JestChaiExpect: ChaiPlugin = (chai, utils) => {
       const results
         = action === 'return' ? spy.mock.results : spy.mock.settledResults
       const result = results[nthCall - 1]
-      const ordinalCall = `${ordinalOf(nthCall)} call`
+      const ordinalCall = `${ordinal(nthCall)} call`
 
       this.assert(
         condition(spy, nthCall, value),
@@ -1213,32 +1213,13 @@ export const JestChaiExpect: ChaiPlugin = (chai, utils) => {
   )
 }
 
-function ordinalOf(i: number) {
-  const j = i % 10
-  const k = i % 100
-
-  if (j === 1 && k !== 11) {
-    return `${i}st`
-  }
-
-  if (j === 2 && k !== 12) {
-    return `${i}nd`
-  }
-
-  if (j === 3 && k !== 13) {
-    return `${i}rd`
-  }
-
-  return `${i}th`
-}
-
 function formatCalls(spy: MockInstance, msg: string, showActualCall?: any) {
   if (spy.mock.calls.length) {
     msg += c.gray(
       `\n\nReceived:\n\n${spy.mock.calls
         .map((callArg, i) => {
           let methodCall = c.bold(
-            `  ${ordinalOf(i + 1)} ${spy.getMockName()} call:\n\n`,
+            `  ${ordinal(i + 1)} ${spy.getMockName()} call:\n\n`,
           )
           if (showActualCall) {
             methodCall += diff(showActualCall, callArg, {
@@ -1275,7 +1256,7 @@ function formatReturns(
       `\n\nReceived:\n\n${results
         .map((callReturn, i) => {
           let methodCall = c.bold(
-            `  ${ordinalOf(i + 1)} ${spy.getMockName()} call return:\n\n`,
+            `  ${ordinal(i + 1)} ${spy.getMockName()} call return:\n\n`,
           )
           if (showActualReturn) {
             methodCall += diff(showActualReturn, callReturn.value, {

packages/runner/src/errors.ts

@@ -24,6 +24,14 @@ export class FixtureDependencyError extends Error {
   public name = 'FixtureDependencyError'
 }
 
+export class FixtureAccessError extends Error {
+  public name = 'FixtureAccessError'
+}
+
+export class FixtureParseError extends Error {
+  public name = 'FixtureParseError'
+}
+
 export class AroundHookSetupError extends Error {
   public name = 'AroundHookSetupError'
 }

packages/runner/src/fixture.ts

@@ -1,7 +1,7 @@
 import type { FixtureFn, Suite, VitestRunner } from './types'
 import type { File, FixtureOptions, TestContext } from './types/tasks'
-import { createDefer, filterOutComments, isObject } from '@vitest/utils/helpers'
-import { FixtureDependencyError } from './errors'
+import { createDefer, filterOutComments, isObject, ordinal } from '@vitest/utils/helpers'
+import { FixtureAccessError, FixtureDependencyError, FixtureParseError } from './errors'
 import { getTestFixtures } from './map'
 import { getCurrentSuite } from './suite'
 
@@ -269,12 +269,14 @@ export async function callFixtureCleanupFrom(context: object, fromIndex: number)
   cleanupFnArray.length = fromIndex
 }
 
+type SuiteHook = 'beforeAll' | 'afterAll' | 'aroundAll'
+
 export interface WithFixturesOptions {
   /**
    * Whether this is a suite-level hook (beforeAll/afterAll/aroundAll).
    * Suite hooks can only access file/worker scoped fixtures and static values.
    */
-  suiteHook?: 'beforeAll' | 'afterAll' | 'aroundAll'
+  suiteHook?: SuiteHook
   /**
    * The test context to use. If not provided, the hookContext passed to the
    * returned function will be used.
@@ -548,19 +550,24 @@ function resolveDeps(
   return pendingFixtures
 }
 
-function validateSuiteHook(fn: Function, hook: string, error: Error | undefined) {
-  const usedProps = getUsedProps(fn)
+function validateSuiteHook(fn: Function, hook: SuiteHook, suiteError: Error | undefined) {
+  const usedProps = getUsedProps(fn, { sourceError: suiteError, suiteHook: hook })
   if (usedProps.size) {
-    console.warn(`The ${hook} hook uses fixtures "${[...usedProps].join('", "')}", but has no access to context. Did you forget to call it as "test.${hook}()" instead of "${hook}()"? This will throw an error in a future major. See https://vitest.dev/guide/test-context#suite-level-hooks`)
-    if (error) {
-      const processor = (globalThis as any).__vitest_worker__?.onFilterStackTrace || ((s: string) => s || '')
-      const stack = processor(error.stack || '')
-      console.warn(stack)
+    const error = new FixtureAccessError(
+      `The ${hook} hook uses fixtures "${[...usedProps].join('", "')}", but has no access to context. `
+      + `Did you forget to call it as "test.${hook}()" instead of "${hook}()"?\n`
+      + `If you used internal "suite" task as the first argument previously, access it in the second argument instead. `
+      + `See https://vitest.dev/guide/test-context#suite-level-hooks`,
+    )
+    if (suiteError) {
+      error.stack = suiteError.stack?.replace(suiteError.message, error.message)
     }
+    throw error
   }
 }
 
 const kPropsSymbol = Symbol('$vitest:fixture-props')
+const kPropNamesSymbol = Symbol('$vitest:fixture-prop-names')
 
 interface FixturePropsOptions {
   index?: number
@@ -574,7 +581,21 @@ export function configureProps(fn: Function, options: FixturePropsOptions): void
   })
 }
 
-function getUsedProps(fn: Function): Set<string> {
+function memoProps(fn: Function, props: Set<string>): Set<string> {
+  (fn as any)[kPropNamesSymbol] = props
+  return props
+}
+
+interface PropsParserOptions {
+  sourceError?: Error | undefined
+  suiteHook?: SuiteHook
+}
+
+function getUsedProps(fn: Function, { sourceError, suiteHook }: PropsParserOptions = {}): Set<string> {
+  if (kPropNamesSymbol in fn) {
+    return fn[kPropNamesSymbol] as Set<string>
+  }
+
   const {
     index: fixturesIndex = 0,
     original: implementation = fn,
@@ -591,24 +612,31 @@ function getUsedProps(fn: Function): Set<string> {
   }
   const match = fnString.match(/[^(]*\(([^)]*)/)
   if (!match) {
-    return new Set()
+    return memoProps(fn, new Set())
   }
 
   const args = splitByComma(match[1])
   if (!args.length) {
-    return new Set()
+    return memoProps(fn, new Set())
   }
 
   const fixturesArgument = args[fixturesIndex]
 
   if (!fixturesArgument) {
-    return new Set()
+    return memoProps(fn, new Set())
   }
 
   if (!(fixturesArgument[0] === '{' && fixturesArgument.endsWith('}'))) {
-    throw new Error(
-      `The first argument inside a fixture must use object destructuring pattern, e.g. ({ test } => {}). Instead, received "${fixturesArgument}".`,
+    const ordinalArgument = ordinal(fixturesIndex + 1)
+    const error = new FixtureParseError(
+      `The ${ordinalArgument} argument inside a fixture must use object destructuring pattern, e.g. ({ task } => {}). `
+      + `Instead, received "${fixturesArgument}".`
+      + `${(suiteHook ? ` If you used internal "suite" task as the ${ordinalArgument} argument previously, access it in the ${ordinal(fixturesIndex + 2)} argument instead.` : '')}`,
     )
+    if (sourceError) {
+      error.stack = sourceError.stack?.replace(sourceError.message, error.message)
+    }
+    throw error
   }
 
   const _first = fixturesArgument.slice(1, -1).replace(/\s/g, '')
@@ -618,12 +646,16 @@ function getUsedProps(fn: Function): Set<string> {
 
   const last = props.at(-1)
   if (last && last.startsWith('...')) {
-    throw new Error(
+    const error = new FixtureParseError(
       `Rest parameters are not supported in fixtures, received "${last}".`,
     )
+    if (sourceError) {
+      error.stack = sourceError.stack?.replace(sourceError.message, error.message)
+    }
+    throw error
   }
 
-  return new Set(props)
+  return memoProps(fn, new Set(props))
 }
 
 function splitByComma(s: string) {

packages/utils/src/helpers.ts

@@ -360,6 +360,25 @@ function isMergeableObject(item: any): item is object {
   return isPlainObject(item) && !Array.isArray(item)
 }
 
+export function ordinal(i: number): string {
+  const j = i % 10
+  const k = i % 100
+
+  if (j === 1 && k !== 11) {
+    return `${i}st`
+  }
+
+  if (j === 2 && k !== 12) {
+    return `${i}nd`
+  }
+
+  if (j === 3 && k !== 13) {
+    return `${i}rd`
+  }
+
+  return `${i}th`
+}
+
 /**
  * Deep merge :P
  *

test/cli/test/__snapshots__/fails.test.ts.snap

@@ -85,17 +85,17 @@ Error: Error thrown in afterEach fixture
 Error: Error thrown in beforeEach fixture"
 `;
 
-exports[`should fail test-extend/fixture-rest-params.test.ts 1`] = `"Error: The first argument inside a fixture must use object destructuring pattern, e.g. ({ test } => {}). Instead, received "...rest"."`;
+exports[`should fail test-extend/fixture-rest-params.test.ts 1`] = `"FixtureParseError: The 1st argument inside a fixture must use object destructuring pattern, e.g. ({ task } => {}). Instead, received "...rest"."`;
 
-exports[`should fail test-extend/fixture-rest-props.test.ts 1`] = `"Error: Rest parameters are not supported in fixtures, received "...rest"."`;
+exports[`should fail test-extend/fixture-rest-props.test.ts 1`] = `"FixtureParseError: Rest parameters are not supported in fixtures, received "...rest"."`;
 
-exports[`should fail test-extend/fixture-without-destructuring.test.ts 1`] = `"Error: The first argument inside a fixture must use object destructuring pattern, e.g. ({ test } => {}). Instead, received "context"."`;
+exports[`should fail test-extend/fixture-without-destructuring.test.ts 1`] = `"FixtureParseError: The 1st argument inside a fixture must use object destructuring pattern, e.g. ({ task } => {}). Instead, received "context"."`;
 
-exports[`should fail test-extend/test-rest-params.test.ts 1`] = `"Error: The first argument inside a fixture must use object destructuring pattern, e.g. ({ test } => {}). Instead, received "...rest"."`;
+exports[`should fail test-extend/test-rest-params.test.ts 1`] = `"FixtureParseError: The 1st argument inside a fixture must use object destructuring pattern, e.g. ({ task } => {}). Instead, received "...rest"."`;
 
-exports[`should fail test-extend/test-rest-props.test.ts 1`] = `"Error: Rest parameters are not supported in fixtures, received "...rest"."`;
+exports[`should fail test-extend/test-rest-props.test.ts 1`] = `"FixtureParseError: Rest parameters are not supported in fixtures, received "...rest"."`;
 
-exports[`should fail test-extend/test-without-destructuring.test.ts 1`] = `"Error: The first argument inside a fixture must use object destructuring pattern, e.g. ({ test } => {}). Instead, received "context"."`;
+exports[`should fail test-extend/test-without-destructuring.test.ts 1`] = `"FixtureParseError: The 1st argument inside a fixture must use object destructuring pattern, e.g. ({ task } => {}). Instead, received "context"."`;
 
 exports[`should fail test-timeout.test.ts 1`] = `
 "Error: Test timed out in 20ms.

test/cli/test/scoped-fixtures.test.ts

@@ -5,15 +5,15 @@ import type { TestFsStructure } from '../../test-utils'
 import { playwright } from '@vitest/browser-playwright'
 import { beforeEach, describe, expect, test } from 'vitest'
 import { rolldownVersion } from 'vitest/node'
-import { replaceRoot, runInlineTests, stripIndent } from '../../test-utils'
+import { runInlineTests, stripIndent } from '../../test-utils'
 
 // "it" is used inside subtests, we can't "import" it because vitest will inject __vite_ssr_import__
 declare const it: TestAPI
 
 if (rolldownVersion) {
   beforeEach(({ skip }) => {
     // TODO: remove skip when we only test against rolldown
-    // oxc has a dofferent output of inlined functions in "runInlineTests"
+    // oxc has a different output of inlined functions in "runInlineTests"
     // it keeps comments and formats the output
     skip()
   })
@@ -661,7 +661,7 @@ test('beforeAll/afterAll hooks throw error when accessing test-scoped fixtures',
 })
 
 test('global beforeAll/afterAll hooks throw error when accessing any fixture', async () => {
-  const { stderr, fixtures, fs } = await runFixtureTests(({ log }) => {
+  const { stderr, fixtures } = await runFixtureTests(({ log }) => {
     return it
       .extend('fileValue', { scope: 'file' }, () => {
         log('fileValue setup')
@@ -678,12 +678,62 @@ test('global beforeAll/afterAll hooks throw error when accessing any fixture', a
     },
   })
 
-  expect(fixtures).toMatchInlineSnapshot(`">> fixture | beforeAll | file: undefined"`)
-  expect(replaceRoot(stderr, fs.root)).toMatchInlineSnapshot(`
-    "stderr | basic.test.ts
-    The beforeAll hook uses fixtures "fileValue", but has no access to context. Did you forget to call it as "test.beforeAll()" instead of "beforeAll()"? This will throw an error in a future major. See https://vitest.dev/guide/test-context#suite-level-hooks
-        at <root>/basic.test.ts:4:3
-        at <root>/basic.test.ts:11:1
+  expect(fixtures).toMatchInlineSnapshot(`""`)
+  expect(stderr).toMatchInlineSnapshot(`
+    "
+    ⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯
+
+     FAIL  basic.test.ts [ basic.test.ts ]
+    FixtureAccessError: The beforeAll hook uses fixtures "fileValue", but has no access to context. Did you forget to call it as "test.beforeAll()" instead of "beforeAll()"?
+    If you used internal "suite" task as the first argument previously, access it in the second argument instead. See https://vitest.dev/guide/test-context#suite-level-hooks
+     ❯ basic.test.ts:4:3
+          2| import { extendedTest, expect, expectTypeOf, describe, beforeAll, afte…
+          3| const results = await (({ extendedTest, beforeAll }) => {
+          4|   beforeAll(({
+           |   ^
+          5|     fileValue
+          6|   }) => {
+     ❯ basic.test.ts:11:1
+
+    ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
+
+    "
+  `)
+})
+
+test('global beforeAll/afterAll hooks throw error when accessing any fixture', async () => {
+  const { stderr, fixtures } = await runFixtureTests(({ log }) => {
+    return it
+      .extend('fileValue', { scope: 'file' }, () => {
+        log('fileValue setup')
+        return 'file-scoped'
+      })
+  }, {
+    'basic.test.ts': ({ extendedTest, beforeAll }) => {
+      beforeAll<{ fileValue: string }>((suite) => {
+        console.log('>> fixture | beforeAll | file:', suite.fileValue)
+      })
+      extendedTest('test1', ({}) => {})
+    },
+  })
+
+  expect(fixtures).toMatchInlineSnapshot(`""`)
+  expect(stderr).toMatchInlineSnapshot(`
+    "
+    ⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯
+
+     FAIL  basic.test.ts [ basic.test.ts ]
+    FixtureParseError: The 1st argument inside a fixture must use object destructuring pattern, e.g. ({ task } => {}). Instead, received "suite". If you used internal "suite" task as the 1st argument previously, access it in the 2nd argument instead.
+     ❯ basic.test.ts:4:3
+          2| import { extendedTest, expect, expectTypeOf, describe, beforeAll, afte…
+          3| const results = await (({ extendedTest, beforeAll }) => {
+          4|   beforeAll((suite) => {
+           |   ^
+          5|     console.log(">> fixture | beforeAll | file:", suite.fileValue);
+          6|   });
+     ❯ basic.test.ts:9:1
+
+    ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
 
     "
   `)