diff --git a/eslint-plugin-epic-web.js b/eslint-plugin-epic-web.js new file mode 100644 index 0000000..f5da306 --- /dev/null +++ b/eslint-plugin-epic-web.js @@ -0,0 +1,3 @@ +export { default } from './lint-rules/epic-web-plugin.js' +export { default as noManualDisposeRule } from './lint-rules/no-manual-dispose.js' +export { default as preferDisposeInTestsRule } from './lint-rules/prefer-dispose-in-tests.js' diff --git a/eslint.js b/eslint.js index 2ae1139..9c97508 100644 --- a/eslint.js +++ b/eslint.js @@ -251,6 +251,13 @@ export const config = [ }, }, + { + files: testFiles, + rules: { + 'epic-web/prefer-dispose-in-tests': WARN, + }, + }, + hasTestingLibrary ? { files: testFiles, diff --git a/lint-rules/epic-web-plugin.js b/lint-rules/epic-web-plugin.js index edb26fb..98ab18c 100644 --- a/lint-rules/epic-web-plugin.js +++ b/lint-rules/epic-web-plugin.js @@ -1,5 +1,6 @@ import { eslintCompatPlugin } from '@oxlint/plugins' import noManualDispose from './no-manual-dispose.js' +import preferDisposeInTests from './prefer-dispose-in-tests.js' const plugin = eslintCompatPlugin({ meta: { @@ -7,7 +8,9 @@ const plugin = eslintCompatPlugin({ }, rules: { 'no-manual-dispose': noManualDispose, + 'prefer-dispose-in-tests': preferDisposeInTests, }, }) export default plugin +export { noManualDispose, preferDisposeInTests } diff --git a/lint-rules/index.md b/lint-rules/index.md index a80baa9..8043d29 100644 --- a/lint-rules/index.md +++ b/lint-rules/index.md @@ -15,3 +15,4 @@ remaining ESLint-compatible. ## Rules - [`epic-web/no-manual-dispose`](./no-manual-dispose.md) +- [`epic-web/prefer-dispose-in-tests`](./prefer-dispose-in-tests.md) diff --git a/lint-rules/prefer-dispose-in-tests.js b/lint-rules/prefer-dispose-in-tests.js new file mode 100644 index 0000000..f1cc96c --- /dev/null +++ b/lint-rules/prefer-dispose-in-tests.js @@ -0,0 +1,464 @@ +const TEST_CALL_ROOTS = new Set(['test', 'it']) +const SUITE_CALL_ROOTS = new Set(['describe', 'suite', 'context']) +const HOOK_NAMES = new Set(['beforeEach', 'afterEach', 'beforeAll', 'afterAll']) +const SUITE_HOOK_NAMES = new Set(['beforeAll', 'afterAll']) + +const KNOWN_FRAMEWORK_HOOK_CALLS = new Set([ + 'vi.useFakeTimers', + 'vi.useRealTimers', + 'vi.clearAllMocks', + 'vi.resetAllMocks', + 'vi.restoreAllMocks', + 'jest.useFakeTimers', + 'jest.useRealTimers', + 'jest.clearAllMocks', + 'jest.resetAllMocks', + 'jest.restoreAllMocks', +]) + +const DEFAULT_OPTIONS = { + allowKnownFrameworkHooks: true, + minimumTestsForSuiteHooks: 2, +} + +function getCallPath(node) { + if (!node) return null + + if (node.type === 'ChainExpression') { + return getCallPath(node.expression) + } + + if (node.type === 'CallExpression') { + return getCallPath(node.callee) + } + + if (node.type === 'Identifier') { + return [node.name] + } + + if (node.type === 'MemberExpression') { + if (node.computed || node.property.type !== 'Identifier') return null + const objectPath = getCallPath(node.object) + if (!objectPath) return null + return [...objectPath, node.property.name] + } + + return null +} + +function isDescribeCallExpression(node) { + if (!node || node.type !== 'CallExpression') return false + const callPath = getCallPath(node) + if (!callPath || callPath.length === 0) return false + + const lastSegment = callPath.at(-1) + + if (SUITE_CALL_ROOTS.has(callPath[0])) { + if (lastSegment === 'each') return node.callee.type === 'CallExpression' + return true + } + + if (callPath.includes('describe')) { + if (lastSegment === 'each') return node.callee.type === 'CallExpression' + return true + } + + return false +} + +function isTestCallExpression(node) { + if (!node || node.type !== 'CallExpression') return false + const callPath = getCallPath(node) + if (!callPath || callPath.length === 0) return false + if (!TEST_CALL_ROOTS.has(callPath[0])) return false + if (callPath.includes('describe')) return false + + const lastSegment = callPath.at(-1) + if (HOOK_NAMES.has(lastSegment)) return false + if (lastSegment === 'step') return false + if (lastSegment === 'each') return node.callee.type === 'CallExpression' + + return true +} + +function getHookName(node) { + if (!node || node.type !== 'CallExpression') return null + const callPath = getCallPath(node) + if (!callPath || callPath.length === 0) return null + const lastSegment = callPath.at(-1) + if (!HOOK_NAMES.has(lastSegment)) return null + + if (callPath.length === 1) return lastSegment + + if ( + callPath.length === 2 && + (TEST_CALL_ROOTS.has(callPath[0]) || SUITE_CALL_ROOTS.has(callPath[0])) + ) { + return lastSegment + } + + return null +} + +function isFunctionNode(node) { + return ( + node?.type === 'FunctionExpression' || + node?.type === 'ArrowFunctionExpression' + ) +} + +function getHookCallback(node) { + return node.arguments.find((argument) => isFunctionNode(argument)) ?? null +} + +function walk(node, callback) { + const nodesToVisit = [node] + while (nodesToVisit.length > 0) { + const currentNode = nodesToVisit.pop() + if (!currentNode || typeof currentNode.type !== 'string') continue + callback(currentNode) + + for (const [key, value] of Object.entries(currentNode)) { + if (key === 'parent') continue + + if (Array.isArray(value)) { + for (let index = value.length - 1; index >= 0; index -= 1) { + nodesToVisit.push(value[index]) + } + continue + } + + if (value && typeof value.type === 'string') { + nodesToVisit.push(value) + } + } + } +} + +function containsThisExpression(node) { + let foundThisExpression = false + walk(node, (currentNode) => { + if (currentNode.type === 'ThisExpression') { + foundThisExpression = true + } + }) + return foundThisExpression +} + +function isNodeInsideRange(node, containerNode) { + return ( + Array.isArray(node.range) && + Array.isArray(containerNode.range) && + node.range[0] >= containerNode.range[0] && + node.range[1] <= containerNode.range[1] + ) +} + +function isVariableDefinedInNode(variable, containerNode) { + return variable.defs.some((definition) => { + if (!definition.name) return false + return isNodeInsideRange(definition.name, containerNode) + }) +} + +function findVariableInScope(scope, variableName) { + let currentScope = scope + while (currentScope) { + if (currentScope.set?.has(variableName)) { + return currentScope.set.get(variableName) + } + currentScope = currentScope.upper + } + return null +} + +function getRootIdentifiers(node) { + if (!node) return [] + + if (node.type === 'ChainExpression') { + return getRootIdentifiers(node.expression) + } + + if (node.type === 'Identifier') { + return [node] + } + + if (node.type === 'MemberExpression') { + return getRootIdentifiers(node.object) + } + + if (node.type === 'ObjectPattern') { + let identifiers = [] + for (const property of node.properties) { + if (!property) continue + if (property.type === 'Property') { + identifiers = identifiers.concat(getRootIdentifiers(property.value)) + } else if (property.type === 'RestElement') { + identifiers = identifiers.concat(getRootIdentifiers(property.argument)) + } + } + return identifiers + } + + if (node.type === 'ArrayPattern') { + let identifiers = [] + for (const element of node.elements) { + if (!element) continue + identifiers = identifiers.concat(getRootIdentifiers(element)) + } + return identifiers + } + + if (node.type === 'AssignmentPattern') { + return getRootIdentifiers(node.left) + } + + if (node.type === 'RestElement') { + return getRootIdentifiers(node.argument) + } + + return [] +} + +function writesOuterState(callbackNode, sourceCode) { + let writesOuterValue = false + + walk(callbackNode.body, (currentNode) => { + if (writesOuterValue) return + + let writeTarget = null + if (currentNode.type === 'AssignmentExpression') { + writeTarget = currentNode.left + } else if (currentNode.type === 'UpdateExpression') { + writeTarget = currentNode.argument + } + + if (!writeTarget) return + const rootIdentifiers = getRootIdentifiers(writeTarget) + if (!rootIdentifiers.length) return + + for (const rootIdentifier of rootIdentifiers) { + const identifierScope = sourceCode.getScope(rootIdentifier) + const variable = findVariableInScope(identifierScope, rootIdentifier.name) + + // If this is an unresolved/global write, treat it as shared mutable state. + if (!variable) { + writesOuterValue = true + return + } + + if (!isVariableDefinedInNode(variable, callbackNode)) { + writesOuterValue = true + return + } + } + }) + + return writesOuterValue +} + +function findContainingSuiteNode(node) { + let currentNode = node.parent + while (currentNode) { + if (currentNode.type === 'Program') return currentNode + + if ( + isFunctionNode(currentNode) && + currentNode.parent?.type === 'CallExpression' && + currentNode.parent.arguments.includes(currentNode) && + isDescribeCallExpression(currentNode.parent) + ) { + return currentNode.body.type === 'BlockStatement' + ? currentNode.body + : currentNode.body + } + + currentNode = currentNode.parent + } + + return null +} + +function getSuiteStatements(suiteNode) { + if (!suiteNode) return [] + if (suiteNode.type === 'Program') return suiteNode.body + if (suiteNode.type === 'BlockStatement') return suiteNode.body + return [] +} + +function analyzeSuiteNode(suiteNode) { + let testCount = 0 + let hasDirectSuiteHooks = false + + walk(suiteNode, (currentNode) => { + if (currentNode.type === 'CallExpression' && isTestCallExpression(currentNode)) { + testCount += 1 + } + }) + + for (const statement of getSuiteStatements(suiteNode)) { + if (statement.type !== 'ExpressionStatement') continue + if (statement.expression.type !== 'CallExpression') continue + const hookName = getHookName(statement.expression) + if (hookName && SUITE_HOOK_NAMES.has(hookName)) { + hasDirectSuiteHooks = true + break + } + } + + return { testCount, hasDirectSuiteHooks } +} + +function getTopLevelCallNames(callbackNode) { + const statements = + callbackNode.body.type === 'BlockStatement' + ? callbackNode.body.body + : [{ type: 'ExpressionStatement', expression: callbackNode.body }] + + const callNames = [] + + for (const statement of statements) { + if (statement.type !== 'ExpressionStatement') return null + + let expressionNode = statement.expression + + if (expressionNode.type === 'UnaryExpression' && expressionNode.operator === 'void') { + expressionNode = expressionNode.argument + } + + if (expressionNode.type === 'AwaitExpression') { + expressionNode = expressionNode.argument + } + + if (expressionNode.type !== 'CallExpression') return null + const callPath = getCallPath(expressionNode) + if (!callPath) return null + callNames.push(callPath.join('.')) + } + + return callNames +} + +function isKnownFrameworkHookCallback(callbackNode) { + const callNames = getTopLevelCallNames(callbackNode) + if (!callNames || callNames.length === 0) return false + return callNames.every((callName) => KNOWN_FRAMEWORK_HOOK_CALLS.has(callName)) +} + +function createRuleVisitors(context, state) { + function getSuiteAnalysis(suiteNode) { + const existingAnalysis = state.suiteAnalysisCache.get(suiteNode) + if (existingAnalysis) return existingAnalysis + + const nextAnalysis = analyzeSuiteNode(suiteNode) + state.suiteAnalysisCache.set(suiteNode, nextAnalysis) + return nextAnalysis + } + + return { + CallExpression(node) { + const hookName = getHookName(node) + if (!hookName) return + + const callbackNode = getHookCallback(node) + if (!callbackNode) return + + const suiteNode = findContainingSuiteNode(node) + if (!suiteNode) return + + const suiteAnalysis = getSuiteAnalysis(suiteNode) + if (suiteAnalysis.testCount === 0) { + // Setup files often have hooks but no colocated tests. + return + } + + // Hooks that rely on runner context, callback completion, or shared state + // are intentionally allowed because disposable refactors are less direct. + if (callbackNode.params.length > 0) return + if (containsThisExpression(callbackNode.body)) return + if (writesOuterState(callbackNode, state.sourceCode)) return + + const isSuiteHook = SUITE_HOOK_NAMES.has(hookName) + + if ( + isSuiteHook && + suiteAnalysis.testCount >= state.options.minimumTestsForSuiteHooks + ) { + return + } + + if ( + !isSuiteHook && + suiteAnalysis.hasDirectSuiteHooks && + suiteAnalysis.testCount >= state.options.minimumTestsForSuiteHooks + ) { + return + } + + if ( + state.options.allowKnownFrameworkHooks && + isKnownFrameworkHookCallback(callbackNode) + ) { + return + } + + context.report({ + node, + messageId: 'preferDisposables', + data: { hookName }, + }) + }, + } +} + +const preferDisposeInTestsRule = { + meta: { + type: 'suggestion', + docs: { + description: + 'Prefer disposable objects over lifecycle hooks when cleanup can be scoped to a test body', + }, + schema: [ + { + type: 'object', + properties: { + allowKnownFrameworkHooks: { type: 'boolean' }, + minimumTestsForSuiteHooks: { + type: 'integer', + minimum: 1, + }, + }, + additionalProperties: false, + }, + ], + messages: { + preferDisposables: + 'Prefer disposable setup (`using`/`await using` with `dispose`/`disposeAsync`) instead of {{hookName}} when cleanup can live in each test body.', + }, + }, + createOnce(context) { + const state = { + sourceCode: null, + suiteAnalysisCache: new WeakMap(), + options: DEFAULT_OPTIONS, + } + + return { + before() { + state.sourceCode = context.sourceCode + state.suiteAnalysisCache = new WeakMap() + const userOptions = Array.isArray(context.options) + ? (context.options[0] ?? {}) + : (context.options ?? {}) + state.options = { + ...DEFAULT_OPTIONS, + ...userOptions, + } + }, + ...createRuleVisitors(context, state), + } + }, +} + +export default preferDisposeInTestsRule +export { preferDisposeInTestsRule } diff --git a/lint-rules/prefer-dispose-in-tests.md b/lint-rules/prefer-dispose-in-tests.md new file mode 100644 index 0000000..efb1a74 --- /dev/null +++ b/lint-rules/prefer-dispose-in-tests.md @@ -0,0 +1,80 @@ +# `epic-web/prefer-dispose-in-tests` + +Prefer disposable setup (`using`/`await using` with `dispose`/`disposeAsync`) +instead of `beforeEach`/`afterEach`/`beforeAll`/`afterAll` when cleanup can +reasonably live in each test body. + +This rule is enabled as `warn` in test files by the shared config. + +## Runtime compatibility + +This rule is authored in Oxlint's optimized style (`createOnce` + `before`), +then exposed to ESLint via `eslintCompatPlugin` from `@oxlint/plugins`. + +That gives: + +- faster execution path in Oxlint JS plugins +- unchanged behavior in ESLint + +## Why + +Disposable test setup keeps setup and cleanup in the same lexical scope and +reduces hidden global lifecycle behavior in flat tests. + +## What it reports + +The rule reports lifecycle hooks that can usually be moved to disposable setup +inside each test. + +## What it intentionally allows + +To avoid noisy false positives, the rule skips reporting when disposable +refactors are often not straightforward: + +- setup files with hooks but no colocated tests +- hooks that use callback params (for example, `done`) or `this` +- hooks that mutate shared outer state +- common framework-level timer/mock lifecycle hooks +- suite-level shared setup in larger suites + +## Options + +```js +{ + allowKnownFrameworkHooks: true, + minimumTestsForSuiteHooks: 2, +} +``` + +- `allowKnownFrameworkHooks` (boolean, default `true`) + - Allows known framework lifecycle calls like `vi.useFakeTimers()`. +- `minimumTestsForSuiteHooks` (integer, default `2`) + - Minimum test count before suite-level shared hooks are considered + reasonable. + +## Example override + +```js +import { config as defaultConfig } from '@epic-web/config/eslint' +import epicWebPlugin from '@epic-web/config/eslint-plugin' + +/** @type {import("eslint").Linter.Config[]} */ +export default [ + ...defaultConfig, + { + files: ['**/*.test.*', '**/*.spec.*'], + plugins: { 'epic-web': epicWebPlugin }, + rules: { + 'epic-web/prefer-dispose-in-tests': [ + 'warn', + { minimumTestsForSuiteHooks: 3 }, + ], + }, + }, +] +``` + +## Source files + +- implementation: [`prefer-dispose-in-tests.js`](./prefer-dispose-in-tests.js) +- tests: [`prefer-dispose-in-tests.test.js`](./prefer-dispose-in-tests.test.js) diff --git a/lint-rules/prefer-dispose-in-tests.test.js b/lint-rules/prefer-dispose-in-tests.test.js new file mode 100644 index 0000000..db23ac3 --- /dev/null +++ b/lint-rules/prefer-dispose-in-tests.test.js @@ -0,0 +1,312 @@ +import { RuleTester } from 'eslint' +import { afterEach, beforeEach, describe, expect, it, test } from 'vitest' + +import plugin from './epic-web-plugin.js' + +const preferDisposeInTestsRule = + plugin.rules['prefer-dispose-in-tests'] + +RuleTester.describe = describe +RuleTester.it = it +RuleTester.itOnly = it.only +RuleTester.itSkip = it.skip +RuleTester.afterEach = afterEach +RuleTester.beforeEach = beforeEach + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + }, +}) + +test('is oxlint optimized and eslint compatible', () => { + expect(typeof preferDisposeInTestsRule.createOnce).toBe('function') + expect(typeof preferDisposeInTestsRule.create).toBe('function') +}) + +ruleTester.run('prefer-dispose-in-tests', preferDisposeInTestsRule, { + valid: [ + { + name: 'allows shared suite hooks for larger suites', + code: ` + describe('server', () => { + beforeAll(() => { + server.listen() + }) + + afterEach(() => { + server.resetHandlers() + }) + + afterAll(() => { + server.close() + }) + + test('first', () => { + expect(true).toBe(true) + }) + + test('second', () => { + expect(true).toBe(true) + }) + }) + `, + }, + { + name: 'allows hooks that mutate outer shared state', + code: ` + describe('user', () => { + let user + + beforeEach(() => { + user = createUser() + }) + + test('renders', () => { + expect(user).toBeDefined() + }) + }) + `, + }, + { + name: 'allows callback parameter hooks', + code: ` + describe('legacy done callback', () => { + beforeEach((done) => { + createUser(() => done()) + }) + + test('renders', () => { + expect(true).toBe(true) + }) + }) + `, + }, + { + name: 'allows hooks that use this context', + code: ` + describe('mocha style', function () { + beforeEach(function () { + this.timeout(1000) + }) + + test('works', function () { + expect(true).toBe(true) + }) + }) + `, + }, + { + name: 'allows known framework lifecycle helpers by default', + code: ` + describe('timers', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + afterEach(() => { + void vi.useRealTimers() + }) + + test('advances time', () => { + expect(true).toBe(true) + }) + }) + `, + }, + { + name: 'allows setup files with hooks and no tests', + filename: '/workspace/setup-tests.js', + code: ` + beforeEach(() => { + vi.clearAllMocks() + }) + `, + }, + { + name: 'allows beforeEach when suite also has shared beforeAll setup', + code: ` + describe('http server', () => { + beforeAll(() => { + server.listen() + }) + + beforeEach(() => { + server.resetHandlers() + }) + + afterAll(() => { + server.close() + }) + + test('request one', () => { + expect(true).toBe(true) + }) + + test('request two', () => { + expect(true).toBe(true) + }) + }) + `, + }, + { + name: 'allows suite hooks exactly at default threshold', + code: ` + describe('two tests is enough for suite hooks', () => { + beforeAll(() => { + connectDb() + }) + + afterAll(() => { + disconnectDb() + }) + + test('first', () => { + expect(true).toBe(true) + }) + + test('second', () => { + expect(true).toBe(true) + }) + }) + `, + }, + ], + invalid: [ + { + name: 'reports beforeEach in single-test suite', + code: ` + describe('user', () => { + beforeEach(() => { + createUser() + }) + + test('renders', () => { + expect(true).toBe(true) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'beforeEach' } }], + }, + { + name: 'reports afterEach in suite without shared suite hooks', + code: ` + describe('user', () => { + afterEach(() => { + cleanupUser() + }) + + test('renders', () => { + expect(true).toBe(true) + }) + + test('updates', () => { + expect(true).toBe(true) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'afterEach' } }], + }, + { + name: 'reports beforeAll in single-test suite', + code: ` + describe('user', () => { + beforeAll(() => { + server.listen() + }) + + test('renders', () => { + expect(true).toBe(true) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'beforeAll' } }], + }, + { + name: 'reports afterAll in single-test suite', + code: ` + describe('user', () => { + afterAll(() => { + server.close() + }) + + test('renders', () => { + expect(true).toBe(true) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'afterAll' } }], + }, + { + name: 'reports top-level beforeEach when tests are colocated', + code: ` + beforeEach(() => { + createUser() + }) + + test('renders', () => { + expect(true).toBe(true) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'beforeEach' } }], + }, + { + name: 'reports nested suite beforeEach with one local test', + code: ` + describe('outer', () => { + test('outer test', () => { + expect(true).toBe(true) + }) + + describe('inner', () => { + beforeEach(() => { + setupInner() + }) + + test('inner test', () => { + expect(true).toBe(true) + }) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'beforeEach' } }], + }, + { + name: 'reports framework hooks when allowKnownFrameworkHooks is disabled', + options: [{ allowKnownFrameworkHooks: false }], + code: ` + describe('timers', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + test('advances time', () => { + expect(true).toBe(true) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'beforeEach' } }], + }, + { + name: 'reports suite hooks when minimumTestsForSuiteHooks is higher', + options: [{ minimumTestsForSuiteHooks: 3 }], + code: ` + describe('two tests not enough when threshold is three', () => { + beforeAll(() => { + connectDb() + }) + + test('first', () => { + expect(true).toBe(true) + }) + + test('second', () => { + expect(true).toBe(true) + }) + }) + `, + errors: [{ messageId: 'preferDisposables', data: { hookName: 'beforeAll' } }], + }, + ], +}) diff --git a/oxlint-config.json b/oxlint-config.json index 5ff1196..3fb4779 100644 --- a/oxlint-config.json +++ b/oxlint-config.json @@ -113,6 +113,7 @@ ], "rules": { "eslint/no-restricted-imports": "off", + "epic-web/prefer-dispose-in-tests": "warn", "vitest/no-import-node-test": "error" } }, diff --git a/package.json b/package.json index d2f1381..fb818d3 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "./typescript": "./typescript.json", "./reset.d.ts": "./reset.d.ts", "./eslint": "./eslint.js", + "./eslint-plugin": "./eslint-plugin-epic-web.js", "./oxlint": "./oxlint-config.json" }, "prettier": "./prettier.js",