From 0aa1ff0fcb6b1e6788bcfc7f7bb721456f6050ad Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 25 Mar 2026 01:02:05 +0000 Subject: [PATCH] fix(linter/plugins): ensure `after` hook is always called in ESLint compat mode (#20721) In Oxlint, when using `createOnce` API, we ensure that `after` hooks are always called for all rules even in the event of a rule throwing an error during AST traversal. Uphold the same guarantee when a plugin using `createOnce` API runs in ESLint via `eslintCompatPlugin`, including in language server environments where the process lives on after an error is thrown. We now make sure that: 1. `after` hooks are always run even if an error occurs during AST visitation. 2. `after` hooks for all rules are always run after *all* the plugin's rules complete AST visitation (not just after each individual rule finishes AST visitation). These guarantees allow creating a reliable per-file cache shared between rules with a pattern like this: ```ts let cache: Data | null = null; let numRunningRules = 0; function setupCache(context) { if (cache === null) cache = new Data(context); numRunningRules++; } function teardownCache() { numRunningRules--; if (numRunningRules === 0) cache = null; } const rule1 = { createOnce(context) { return { before() { setupCache(context); }, Identifier(node) { // Use `cache` }, after() { teardownCache(); }, }; }, }; const rule2 = { // Same as above }; const rule3 = { // Same as above }; ``` We can build on this to create a dedicated API for the shared cache use case, and have it work reliably in ESLint as well as Oxlint. Related: #20700. --- apps/oxlint/src-js/package/compat.ts | 327 ++++++++++++++++-- .../test/fixtures/eslintCompat/.oxlintrc.json | 3 +- .../fixtures/eslintCompat/eslint.config.js | 1 + .../test/fixtures/eslintCompat/eslint.snap.md | 4 +- .../test/fixtures/eslintCompat/output.snap.md | 10 +- .../test/fixtures/eslintCompat/plugin.ts | 172 +++++---- .../.oxlintrc.json | 9 + .../eslint.config.js | 15 + .../eslint.snap.md | 38 ++ .../eslintCompat_error_in_after/files/1.js | 1 + .../eslintCompat_error_in_after/options.json | 3 + .../output.snap.md | 36 ++ .../eslintCompat_error_in_after/plugin.ts | 92 +++++ .../.oxlintrc.json | 9 + .../eslint.config.js | 15 + .../eslint.snap.md | 25 ++ .../eslintCompat_error_in_before/files/1.js | 1 + .../eslintCompat_error_in_before/options.json | 3 + .../output.snap.md | 24 ++ .../eslintCompat_error_in_before/plugin.ts | 102 ++++++ .../.oxlintrc.json | 9 + .../eslint.config.js | 15 + .../eslint.snap.md | 37 ++ .../files/1.js | 1 + .../options.json | 3 + .../output.snap.md | 35 ++ .../plugin.ts | 92 +++++ .../.oxlintrc.json | 9 + .../eslint.config.js | 15 + .../eslint.snap.md | 31 ++ .../eslintCompat_error_in_visit/files/1.js | 1 + .../eslintCompat_error_in_visit/options.json | 3 + .../output.snap.md | 29 ++ .../eslintCompat_error_in_visit/plugin.ts | 92 +++++ 34 files changed, 1171 insertions(+), 91 deletions(-) create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/.oxlintrc.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.config.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/files/1.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/options.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/output.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_after/plugin.ts create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/.oxlintrc.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.config.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/files/1.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/options.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/output.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_before/plugin.ts create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/.oxlintrc.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.config.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/files/1.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/options.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/output.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/plugin.ts create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/.oxlintrc.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.config.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/files/1.js create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/options.json create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/output.snap.md create mode 100644 apps/oxlint/test/fixtures/eslintCompat_error_in_visit/plugin.ts diff --git a/apps/oxlint/src-js/package/compat.ts b/apps/oxlint/src-js/package/compat.ts index 52a9b2ffe0698..f3fbec686077a 100644 --- a/apps/oxlint/src-js/package/compat.ts +++ b/apps/oxlint/src-js/package/compat.ts @@ -3,7 +3,7 @@ * Converts an Oxlint plugin using `createOnce` to a plugin which will run in ESLint. */ -import { debugAssertIsNonNull } from "../utils/asserts.ts"; +import { debugAssert, debugAssertIsNonNull } from "../utils/asserts.ts"; import type { Context, FileContext, LanguageOptions } from "../plugins/context.ts"; import type { CreateOnceRule, Plugin, Rule } from "../plugins/load.ts"; @@ -16,6 +16,13 @@ import type { SetNullable } from "../utils/types.ts"; // Empty visitor object, returned by `create` when `before` hook returns `false`. const EMPTY_VISITOR: Visitor = {}; +// State of an `after` hook. +// `AFTER_HOOK_INACTIVE` = doesn't need to run, `AFTER_HOOK_PENDING` = needs to run. +// This is a "poor man's enum" which minifies better than a TS enum. +type PendingState = typeof AFTER_HOOK_INACTIVE | typeof AFTER_HOOK_PENDING; +const AFTER_HOOK_INACTIVE = 0; +const AFTER_HOOK_PENDING = 1; + /** * Convert a plugin which used Oxlint's `createOnce` API to also work with ESLint. * @@ -38,23 +45,270 @@ export function eslintCompatPlugin(plugin: Plugin): Plugin { throw new Error("Plugin must have an object as `rules` property"); } + // Set up state for tracking if any `after` hooks need to be called + const afterHooksState = new AfterHooksState(); + // Make each rule in the plugin ESLint-compatible by calling `convertRule` on it for (const ruleName in rules) { - if (Object.hasOwn(rules, ruleName)) convertRule(rules[ruleName]); + if (Object.hasOwn(rules, ruleName)) convertRule(rules[ruleName], afterHooksState); } return plugin; } +/** + * Class containing state for tracking if any `after` hooks for a plugin's rules need to be called. + * + * # Aims + * + * Aims are: + * 1. `after` hook of each rule runs after all other AST visit functions, and CFG event handlers. + * 2. `after` hooks for *all* a plugin's rules run after *all* that plugin's rules have completed visiting AST. + * 3. `after` hooks for *all* a plugin's rules run before *any* of plugin's rules begin linting another file. + * 4. In the case of an error during AST traversal, `after` hooks are always still run. + * + * The above exactly matches the behavior when running a `createOnce` rule in Oxlint. + * + * # Why this is important + * + * All the complication comes from ensuring `after` hooks run even after an error during AST traversal. + * + * In ESLint CLI, an error will crash the process, so it doesn't particularly matter if `after` hooks run or not, + * but language servers will typically swallow errors, and keep the process running. + * + * Rules using `before` and `after` hooks will often rely on both hooks running in a predictable order, + * to maintain some internal state. For example, they may use `before` and `after` hooks to maintain a per-file + * cache of data which is shared between rules. The cache use case is why rule (2) above is important. + * + * Below is an example of using `before` and `after` hooks to maintain a per-file cache, shared between rules. + * It relies on all `before` hooks running before any rule starts visiting the AST, + * and all `after` hooks running after all rules have finished visiting the AST. + * + * ```ts + * let cache: Data | null = null; + * + * let numRunningRules = 0; + * + * const setupCache = (context) => { + * if (cache === null) cache = new Data(context); + * numRunningRules++; + * }; + * + * const teardownCache = () => { + * numRunningRules--; + * if (numRunningRules === 0) cache = null; + * }; + * + * const rule1 = { + * createOnce(context) { + * return { + * before() { + * setupCache(context); + * }, + * Identifier(node) { + * // Use `cache` + * }, + * after: teardownCache, + * }; + * }, + * }; + * + * const rule2 = { + * // Same as above + * }; + * + * const rule3 = { + * // Same as above + * }; + * ``` + * + * If `after` hooks did not always run, the next lint run could get stale state, and malfunction. + * If `after` hooks ran in the wrong order (e.g. after some `before` hooks for next file), + * `numRunningRules` would never get to 0, and cache would never be cleared. + * + * Note that because all rules run together in a single AST traversal, if a rule from plugin X throws an error, + * it can disrupt rules from plugin Y. This would make it hard to debug. + * + * # Mechanism + * + * ## Initialization + * + * Rules with an `after` hook register themselves by: + * + * 1. Calling `registerResetFunction` to register a function to run `after` hook and clean up internal state. + * This call adds the reset fn to `resetFunctions`, and adds `AFTER_HOOK_INACTIVE` to `pendingStates`. + * 2. Adding an `onCodePathEnd` CFG event handler to the visitor which calls `ruleFinished` at end of AST traversal. + * + * ## Per-file setup + * + * Before linting a file, `create` will call `setupAfterHook` which is created by `createContextAndVisitor`. + * This registers that the `after` hook for the rule needs to run, by setting `pendingStates[ruleIndex]` + * to `AFTER_HOOK_PENDING`, and incrementing `pendingCount`. + * + * If a cleanup microtask has not been scheduled yet, one is scheduled now (see reason below). + * + * ## Normal operation + * + * AST traversal for each rule ends with `ruleFinished` hook being called from `onCodePathEnd` CFG event handler. + * It increments `lintFinishedCount`. If `lintFinishedCount` equals `pendingCount`, all rules have finished linting + * the file, and `reset` is called, which calls all the pending `after` hooks. + * + * ## Error handling + * + * If an error is thrown during AST traversal, we ensure that `after` hooks are still run by 2 mechanisms: + * + * ### 1. Next microtick + * + * Before any rules began linting files, a microtask was scheduled, which runs on next micro-tick. + * All language servers we're aware of run each lint task in a separate tick, so this microtask will run in next tick + * after a linting run, before the next lint task starts. + * + * If the linting run completed successfully, the microtask does nothing. + * + * But if an error was thrown during AST traversal, this will be visible from the state of `pendingCount`. + * The microtask will run any `after` hooks which need to be run, and reset state to reflect that there are + * no more pending `after` hooks. + * + * ### 2. Fallback: Next lint run + * + * Before linting any file, the state of `pendingCount` is checked. + * If any `after` hooks are still pending, they are run immediately. + * They're run before the `context` objects in `createOnce` closures are updated to the next file, + * so they run with access to the old `context` object from the last file. + * + * This fallback should not be required, but it's included as "belt and braces", to handle if any language server + * or other environment running ESLint programmatically, does not pause a tick between linting runs. + */ +class AfterHooksState { + // Array of functions to call to clean up state and run `after` hook for rules which have an `after` hook. + resetFunctions: (() => void)[] = []; + + // Array of flags indicating if reset functions needs to be called. + // Each entry corresponds to an entry in `resetFunctions`. + pendingStates: PendingState[] = []; + + // Count of reset functions which need to be called. + // Equal to number of `AFTER_HOOK_PENDING` entries in `pendingStates`. + pendingCount: number = 0; + + // Count of rules with `after` hooks which have completed linting current file. + lintFinishedCount: number = 0; + + // `true` if a microtask has been scheduled to call `reset` yet. + resetIsScheduled: boolean = false; + + // `SourceCode` object for file currently being linted. + // Used to determine at start of `create` whether we're linting a new file, or still on current one. + sourceCode: SourceCode | null = null; + + // Function which is scheduled as the cleanup microtask. + private resetMicrotask: () => void = this.resetMicrotaskImpl.bind(this); + + /** + * Register a function to run `after` hook for a rule, and reset state. + * @param reset - Function to run `after` hook and reset state + * @returns Index of rule + */ + registerResetFunction(reset: () => void): number { + const { pendingStates } = this; + const index = pendingStates.length; + pendingStates.push(AFTER_HOOK_INACTIVE); + this.resetFunctions.push(reset); + return index; + } + + /** + * Register that a rule with `after` hook has completed linting a file. + * Called by `onCodePathEnd` CFG event handler which is added to visitor for rules with `after` hooks. + * + * If all rules with an `after` hook which needs to be run have completed linting the file, run all `after` hooks. + */ + ruleFinished(): void { + this.lintFinishedCount++; + if (this.lintFinishedCount === this.pendingCount) { + // All rules with `after` hooks have finished linting the file. Run all `after` hooks. + // `false` to throw any errors which occur in `after` hooks. + this.reset(false); + } + } + + /** + * Call all reset functions where corresponding entry in `pendingStates` is `AFTER_HOOK_PENDING`. + * Should only be called when some `after` hooks are pending. + * + * @param ignoreErrors - `true` to catch and silently ignore any errors which occur in `after` hooks. + * `false` to throw them, + * @throws {unknown} If `ignoreErrors` is `false` and an error occurs in any `after` hooks. + */ + reset(ignoreErrors: boolean): void { + debugAssert(this.pendingCount > 0, "`pendingCount` should be > 0"); + + const { resetFunctions, pendingStates } = this, + hooksLen = pendingStates.length; + + let hasError = false, + error: unknown; + + for (let i = 0; i < hooksLen; i++) { + if (pendingStates[i] !== AFTER_HOOK_INACTIVE) { + pendingStates[i] = AFTER_HOOK_INACTIVE; + + // Run reset function for rule. + // Capture any errors - make sure all rules are reset, before throwing. + try { + resetFunctions[i](); + } catch (e) { + if (hasError === false) { + hasError = true; + error = e; + } + } + } + } + + // Reset state + this.pendingCount = 0; + this.lintFinishedCount = 0; + + // Clear `sourceCode` to free it for garbage collection + this.sourceCode = null; + + // Throw error if there was one, unless `ignoreErrors` is `true` + if (hasError === true && ignoreErrors === false) throw error; + } + + /** + * Schedule a microtask to run `reset` functions. + */ + scheduleReset(): void { + queueMicrotask(this.resetMicrotask); + this.resetIsScheduled = true; + } + + /** + * Function which is scheduled as the cleanup microtask. + * `scheduleReset` uses `resetMicrotask` which is this method bound to `this`. + */ + private resetMicrotaskImpl(): void { + this.resetIsScheduled = false; + + if (this.pendingCount !== 0) { + // `true` to ignore errors. We're not in main "thread" of execution. + this.reset(true); + } + } +} + /** * Convert a rule. * * The `rule` object passed in is mutated in-place. * * @param rule - Rule to convert + * @param afterHooksState - State of `after` hooks * @throws {Error} If `rule` is not an object */ -function convertRule(rule: Rule) { +function convertRule(rule: Rule, afterHooksState: AfterHooksState) { // Validate type of `rule` if (rule === null || typeof rule !== "object") throw new Error("Rule must be an object"); @@ -70,10 +324,30 @@ function convertRule(rule: Rule) { rule.create = (eslintContext) => { // Lazily call `createOnce` on first invocation of `create` if (context === null) { - ({ context, visitor, beforeHook, setupAfterHook } = createContextAndVisitor(rule)); + ({ context, visitor, beforeHook, setupAfterHook } = createContextAndVisitor( + rule, + afterHooksState, + )); } debugAssertIsNonNull(visitor); + const eslintFileContext = Object.getPrototypeOf(eslintContext); + + // If this rule has an `after` hook, check if `sourceCode` has changed. + // If it has, this is first rule with an `after` hook to run on this file. + // If any `after` hooks were not run at end of last file, run them now. + if (setupAfterHook !== null) { + const { sourceCode } = eslintFileContext; + if (afterHooksState.sourceCode !== sourceCode) { + afterHooksState.sourceCode = sourceCode; + + if (afterHooksState.pendingCount !== 0) { + // `true` to ignore errors - any errors relate to *previous* file, so not appropriate to throw here + afterHooksState.reset(true); + } + } + } + // Copy properties from ESLint's context object to `context`. // ESLint's context object is an object of form `{ id, options, report }`, with all other properties // and methods on another object which is its prototype. @@ -82,7 +356,6 @@ function convertRule(rule: Rule) { options: { value: eslintContext.options }, report: { value: eslintContext.report }, }); - const eslintFileContext = Object.getPrototypeOf(eslintContext); Object.setPrototypeOf(context, eslintFileContext); // If `before` hook returns `false`, skip traversal by returning an empty object as visitor @@ -91,8 +364,12 @@ function convertRule(rule: Rule) { if (shouldRun === false) return EMPTY_VISITOR; } - // If there's an `after` hook, call `setupAfterHook` with the `Program` node of the current file - if (setupAfterHook !== null) setupAfterHook(eslintFileContext.sourceCode.ast); + // If there's an `after` hook, call `setupAfterHook` with the `Program` node of the current file. + // Schedule a microtask to run `after` hooks functions, if one hasn't already been scheduled. + if (setupAfterHook !== null) { + setupAfterHook(eslintFileContext.sourceCode.ast); + if (afterHooksState.resetIsScheduled === false) afterHooksState.scheduleReset(); + } // Return same visitor each time return visitor; @@ -165,10 +442,14 @@ const FILE_CONTEXT: FileContext = Object.freeze({ * Call `createOnce` method of rule, and return `Context`, `Visitor`, and `beforeHook` (if any). * * @param rule - Rule with `createOnce` method + * @param afterHooksState - State of `after` hooks * @returns Object with `context`, `visitor`, and `beforeHook` properties, * and `setupAfterHook` function if visitor has an `after` hook */ -function createContextAndVisitor(rule: CreateOnceRule): { +function createContextAndVisitor( + rule: CreateOnceRule, + afterHooksState: AfterHooksState, +): { context: Context; visitor: Visitor; beforeHook: BeforeHook | null; @@ -219,15 +500,28 @@ function createContextAndVisitor(rule: CreateOnceRule): { let program: Program | null = null; - // Pass function to populate `program` var back out to the `create` function generated by `convertRule`. - // `create` will call this function at the start of linting each file. + // Register a reset function. This calls `after` hook for the rule. + // Its called after all rules with `after` hooks have finished linting the file, or after an error occurs. + const ruleIndex = afterHooksState.registerResetFunction(() => { + // Clear `program` to free the AST for garbage collection + program = null; + // Call `after` hook + afterHook(); + }); + + // Create setup function. + // It is called by `create` before linting a file, passing `Program` node for the file. // Having `program` in a local var makes the `node === program` check in `onCodePathEnd` as cheap as it can be. // Otherwise it'd have to be `node === context.sourceCode.ast`, which would be slower. setupAfterHook = (ast: Program) => { + // Store `Program` in local var, for use in `onCodePathEnd` CFG event handler program = ast; + // Mark `after` hook for this rule as needing to be run + afterHooksState.pendingStates[ruleIndex] = AFTER_HOOK_PENDING; + afterHooksState.pendingCount++; }; - // Add `onCodePathEnd` CFG event handler to run `after` hook at end of AST traversal. + // Add `onCodePathEnd` CFG event handler to detect when this rule completes AST traversal. // This fires after all visit fns have been called (after `Program:exit`), and after all other CFG event handlers. type CodePathHandler = (this: Visitor, codePath: unknown, node: ESTreeNode) => void; @@ -236,18 +530,11 @@ function createContextAndVisitor(rule: CreateOnceRule): { (visitor as unknown as { onCodePathEnd: CodePathHandler }).onCodePathEnd = onCodePathEnd == null ? function (this: Visitor, _codePath: unknown, node: ESTreeNode) { - if (node === program) { - program = null; - afterHook(); - } + if (node === program) afterHooksState.ruleFinished(); } : function (this: Visitor, codePath: unknown, node: ESTreeNode) { onCodePathEnd.call(this, codePath, node); - - if (node === program) { - program = null; - afterHook(); - } + if (node === program) afterHooksState.ruleFinished(); }; } diff --git a/apps/oxlint/test/fixtures/eslintCompat/.oxlintrc.json b/apps/oxlint/test/fixtures/eslintCompat/.oxlintrc.json index 76e808fb6af5c..93306fd29c393 100644 --- a/apps/oxlint/test/fixtures/eslintCompat/.oxlintrc.json +++ b/apps/oxlint/test/fixtures/eslintCompat/.oxlintrc.json @@ -18,7 +18,8 @@ { "files": ["files/cfg.js"], "rules": { - "eslint-compat-plugin/create-once-cfg": "error" + "eslint-compat-plugin/create-once-cfg": "error", + "eslint-compat-plugin/create-once-cfg2": "error" } } ] diff --git a/apps/oxlint/test/fixtures/eslintCompat/eslint.config.js b/apps/oxlint/test/fixtures/eslintCompat/eslint.config.js index da470a3c2a65a..ffc47d06f7629 100644 --- a/apps/oxlint/test/fixtures/eslintCompat/eslint.config.js +++ b/apps/oxlint/test/fixtures/eslintCompat/eslint.config.js @@ -24,6 +24,7 @@ export default [ }, rules: { "eslint-compat-plugin/create-once-cfg": "error", + "eslint-compat-plugin/create-once-cfg2": "error", }, }, ]; diff --git a/apps/oxlint/test/fixtures/eslintCompat/eslint.snap.md b/apps/oxlint/test/fixtures/eslintCompat/eslint.snap.md index 906342715d564..5356bc40c7b81 100644 --- a/apps/oxlint/test/fixtures/eslintCompat/eslint.snap.md +++ b/apps/oxlint/test/fixtures/eslintCompat/eslint.snap.md @@ -118,8 +118,10 @@ filename: /files/2.js eslint-compat-plugin /files/cfg.js 0:1 error after hook: filename: /files/cfg.js eslint-compat-plugin/create-once-cfg + 0:1 error after hook: +filename: /files/cfg.js eslint-compat-plugin/create-once-cfg2 -✖ 50 problems (50 errors, 0 warnings) +✖ 51 problems (51 errors, 0 warnings) ``` # stderr diff --git a/apps/oxlint/test/fixtures/eslintCompat/output.snap.md b/apps/oxlint/test/fixtures/eslintCompat/output.snap.md index 774713bb05736..93a7960f19033 100644 --- a/apps/oxlint/test/fixtures/eslintCompat/output.snap.md +++ b/apps/oxlint/test/fixtures/eslintCompat/output.snap.md @@ -336,7 +336,15 @@ 2 | console.log(i); `---- -Found 0 warnings and 46 errors. + x eslint-compat-plugin(create-once-cfg2): after hook: + | filename: /files/cfg.js + ,-[files/cfg.js:1:1] + 1 | for (let i = 0; i < 3; i++) { + : ^ + 2 | console.log(i); + `---- + +Found 0 warnings and 47 errors. Finished in Xms on 3 files with 0 rules using X threads. ``` diff --git a/apps/oxlint/test/fixtures/eslintCompat/plugin.ts b/apps/oxlint/test/fixtures/eslintCompat/plugin.ts index 605c26f9f49e9..c3c6a3c8bf74e 100644 --- a/apps/oxlint/test/fixtures/eslintCompat/plugin.ts +++ b/apps/oxlint/test/fixtures/eslintCompat/plugin.ts @@ -201,78 +201,113 @@ const createOnceSelectorRule: Rule = { }; // This tests that `after` hook runs after all CFG event handlers. -// This rules is only run on `files/cfg.js`, to ensure CFG events handlers do not affect behavior of other rules +// These rules are only run on `files/cfg.js`, to ensure CFG events handlers do not affect behavior of other rules // which don't use CFG event listeners. -const createOnceCfgRule: Rule = { - createOnce(context) { - // Collect all visits and check them in `after` hook - const visits: { event: string; nodeType?: string }[] = []; - - return { - onCodePathStart(_codePath: unknown, node: ESTreeNode) { - visits.push({ event: "onCodePathStart", nodeType: node.type }); - }, - onCodePathEnd(_codePath: unknown, node: ESTreeNode) { - visits.push({ event: "onCodePathEnd", nodeType: node.type }); - }, - onCodePathSegmentStart(_segment: unknown, node: ESTreeNode) { - visits.push({ event: "onCodePathSegmentStart", nodeType: node.type }); - }, - onCodePathSegmentEnd(_segment: unknown, node: ESTreeNode) { - visits.push({ event: "onCodePathSegmentEnd", nodeType: node.type }); - }, - onUnreachableCodePathSegmentStart(_segment: unknown, node: ESTreeNode) { - visits.push({ event: "onUnreachableCodePathSegmentStart", nodeType: node.type }); - }, - onUnreachableCodePathSegmentEnd(_segment: unknown, node: ESTreeNode) { - visits.push({ event: "onUnreachableCodePathSegmentEnd", nodeType: node.type }); - }, - onCodePathSegmentLoop(_fromSegment: unknown, _toSegment: unknown, node: ESTreeNode) { - visits.push({ event: "onCodePathSegmentLoop", nodeType: node.type }); - }, - after() { - context.report({ - message: "after hook:\n" + `filename: ${context.filename}`, - node: SPAN, - }); - - visits.push({ event: "after" }); - - const expectedVisits: typeof visits = [ - { event: "onCodePathStart", nodeType: "Program" }, - { event: "onCodePathSegmentStart", nodeType: "Program" }, - { event: "onCodePathSegmentEnd", nodeType: "BinaryExpression" }, - { event: "onCodePathSegmentStart", nodeType: "BinaryExpression" }, - { event: "onCodePathSegmentEnd", nodeType: "UpdateExpression" }, - { event: "onCodePathSegmentStart", nodeType: "UpdateExpression" }, - { event: "onCodePathSegmentLoop", nodeType: "BlockStatement" }, - { event: "onCodePathSegmentEnd", nodeType: "BlockStatement" }, - { event: "onCodePathSegmentStart", nodeType: "BlockStatement" }, - { event: "onCodePathSegmentEnd", nodeType: "BlockStatement" }, - { event: "onUnreachableCodePathSegmentStart", nodeType: "BlockStatement" }, - { event: "onUnreachableCodePathSegmentEnd", nodeType: "ForStatement" }, - { event: "onCodePathSegmentStart", nodeType: "ForStatement" }, - { event: "onCodePathSegmentEnd", nodeType: "Program" }, - { event: "onCodePathEnd", nodeType: "Program" }, - { event: "after" }, - ]; - - if ( - visits.length !== expectedVisits.length || - visits.some( - (v, i) => - v.event !== expectedVisits[i].event || v.nodeType !== expectedVisits[i].nodeType, - ) - ) { +// Collect all visits and check them in `after` hook. +// Run 2 copies of the rule, to ensure that `after` hooks for both rules run after all visits for both rules. +const visits: { ruleNum: number; event: string; nodeType?: string }[] = []; + +function createCfgRule(ruleNum: number): Rule { + function addEvent(event: string, nodeType?: string) { + visits.push({ ruleNum, event, nodeType }); + } + + return { + createOnce(context) { + return { + before() { + addEvent("before"); + }, + onCodePathStart(_codePath: unknown, node: ESTreeNode) { + addEvent("onCodePathStart", node.type); + }, + onCodePathEnd(_codePath: unknown, node: ESTreeNode) { + addEvent("onCodePathEnd", node.type); + }, + onCodePathSegmentStart(_segment: unknown, node: ESTreeNode) { + addEvent("onCodePathSegmentStart", node.type); + }, + onCodePathSegmentEnd(_segment: unknown, node: ESTreeNode) { + addEvent("onCodePathSegmentEnd", node.type); + }, + onUnreachableCodePathSegmentStart(_segment: unknown, node: ESTreeNode) { + addEvent("onUnreachableCodePathSegmentStart", node.type); + }, + onUnreachableCodePathSegmentEnd(_segment: unknown, node: ESTreeNode) { + addEvent("onUnreachableCodePathSegmentEnd", node.type); + }, + onCodePathSegmentLoop(_fromSegment: unknown, _toSegment: unknown, node: ESTreeNode) { + addEvent("onCodePathSegmentLoop", node.type); + }, + after() { context.report({ - message: `Unexpected visits:\n${JSON.stringify(visits, null, 2)}`, + message: "after hook:\n" + `filename: ${context.filename}`, node: SPAN, }); - } - }, - } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present - }, -}; + + addEvent("after"); + + if (ruleNum === 1) return; + + const expectedVisits: typeof visits = [ + { ruleNum: 1, event: "before" }, + { ruleNum: 2, event: "before" }, + { ruleNum: 1, event: "onCodePathStart", nodeType: "Program" }, + { ruleNum: 2, event: "onCodePathStart", nodeType: "Program" }, + { ruleNum: 1, event: "onCodePathSegmentStart", nodeType: "Program" }, + { ruleNum: 2, event: "onCodePathSegmentStart", nodeType: "Program" }, + { ruleNum: 1, event: "onCodePathSegmentEnd", nodeType: "BinaryExpression" }, + { ruleNum: 2, event: "onCodePathSegmentEnd", nodeType: "BinaryExpression" }, + { ruleNum: 1, event: "onCodePathSegmentStart", nodeType: "BinaryExpression" }, + { ruleNum: 2, event: "onCodePathSegmentStart", nodeType: "BinaryExpression" }, + { ruleNum: 1, event: "onCodePathSegmentEnd", nodeType: "UpdateExpression" }, + { ruleNum: 2, event: "onCodePathSegmentEnd", nodeType: "UpdateExpression" }, + { ruleNum: 1, event: "onCodePathSegmentStart", nodeType: "UpdateExpression" }, + { ruleNum: 2, event: "onCodePathSegmentStart", nodeType: "UpdateExpression" }, + { ruleNum: 1, event: "onCodePathSegmentLoop", nodeType: "BlockStatement" }, + { ruleNum: 2, event: "onCodePathSegmentLoop", nodeType: "BlockStatement" }, + { ruleNum: 1, event: "onCodePathSegmentEnd", nodeType: "BlockStatement" }, + { ruleNum: 2, event: "onCodePathSegmentEnd", nodeType: "BlockStatement" }, + { ruleNum: 1, event: "onCodePathSegmentStart", nodeType: "BlockStatement" }, + { ruleNum: 2, event: "onCodePathSegmentStart", nodeType: "BlockStatement" }, + { ruleNum: 1, event: "onCodePathSegmentEnd", nodeType: "BlockStatement" }, + { ruleNum: 2, event: "onCodePathSegmentEnd", nodeType: "BlockStatement" }, + { ruleNum: 1, event: "onUnreachableCodePathSegmentStart", nodeType: "BlockStatement" }, + { ruleNum: 2, event: "onUnreachableCodePathSegmentStart", nodeType: "BlockStatement" }, + { ruleNum: 1, event: "onUnreachableCodePathSegmentEnd", nodeType: "ForStatement" }, + { ruleNum: 2, event: "onUnreachableCodePathSegmentEnd", nodeType: "ForStatement" }, + { ruleNum: 1, event: "onCodePathSegmentStart", nodeType: "ForStatement" }, + { ruleNum: 2, event: "onCodePathSegmentStart", nodeType: "ForStatement" }, + { ruleNum: 1, event: "onCodePathSegmentEnd", nodeType: "Program" }, + { ruleNum: 2, event: "onCodePathSegmentEnd", nodeType: "Program" }, + { ruleNum: 1, event: "onCodePathEnd", nodeType: "Program" }, + { ruleNum: 2, event: "onCodePathEnd", nodeType: "Program" }, + { ruleNum: 1, event: "after" }, + { ruleNum: 2, event: "after" }, + ]; + + if ( + visits.length !== expectedVisits.length || + visits.some( + (v, i) => + v.ruleNum !== expectedVisits[i].ruleNum || + v.event !== expectedVisits[i].event || + v.nodeType !== expectedVisits[i].nodeType, + ) + ) { + context.report({ + message: `Unexpected visits:\n${JSON.stringify(visits, null, 2)}`, + node: SPAN, + }); + } + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, + }; +} + +const createOnceCfgRule = createCfgRule(1); +const createOnceCfgRule2 = createCfgRule(2); // Tests that `before` hook returning `false` disables visiting AST for the file. const createOnceBeforeFalseRule: Rule = { @@ -385,6 +420,7 @@ export default eslintCompatPlugin({ "create-once": createOnceRule, "create-once-selector": createOnceSelectorRule, "create-once-cfg": createOnceCfgRule, + "create-once-cfg2": createOnceCfgRule2, "create-once-before-false": createOnceBeforeFalseRule, "create-once-before-only": createOnceBeforeOnlyRule, "create-once-after-only": createOnceAfterOnlyRule, diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/.oxlintrc.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/.oxlintrc.json new file mode 100644 index 0000000000000..a6e32b898fda9 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/.oxlintrc.json @@ -0,0 +1,9 @@ +{ + "jsPlugins": ["./plugin.ts"], + "categories": { "correctness": "off" }, + "rules": { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-after": "error", + "eslint-compat-plugin/tracking-late": "error" + } +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.config.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.config.js new file mode 100644 index 0000000000000..b6c7f976a3fd1 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.config.js @@ -0,0 +1,15 @@ +import plugin from "./plugin.ts"; + +export default [ + { + files: ["files/*.js"], + plugins: { + "eslint-compat-plugin": plugin, + }, + rules: { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-after": "error", + "eslint-compat-plugin/tracking-late": "error", + }, + }, +]; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.snap.md new file mode 100644 index 0000000000000..50674dba52668 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/eslint.snap.md @@ -0,0 +1,38 @@ +# Exit code +2 + +# stdout +``` +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-after", + "before: tracking-late", + "Identifier: tracking", + "Identifier: throw-in-after", + "Identifier: tracking-late", + "Program:exit: tracking", + "Program:exit: throw-in-after", + "Program:exit: tracking-late", + "onCodePathEnd: tracking", + "onCodePathEnd: throw-in-after", + "onCodePathEnd: tracking-late", + "after: tracking", + "after: throw-in-after", + "after: tracking-late" +] + +Oops! Something went wrong! :( + +ESLint: 9.39.4 + +Error: `after` hook threw +Occurred while linting /files/1.js +Rule: "eslint-compat-plugin/tracking-late" + at after (/plugin.ts:51:15) +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/files/1.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/files/1.js new file mode 100644 index 0000000000000..2756c24c45775 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/files/1.js @@ -0,0 +1 @@ +let x; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/options.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/options.json new file mode 100644 index 0000000000000..f380a596b9d7a --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/options.json @@ -0,0 +1,3 @@ +{ + "eslint": true +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/output.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/output.snap.md new file mode 100644 index 0000000000000..5107e553f4527 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/output.snap.md @@ -0,0 +1,36 @@ +# Exit code +1 + +# stdout +``` + x Error running JS plugin. + | File path: /files/1.js + | Error: `after` hook threw + | at after (/plugin.ts:51:15) + +Found 0 warnings and 1 error. +Finished in Xms on 1 file with 3 rules using X threads. +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-after", + "before: tracking-late", + "Identifier: tracking", + "Identifier: throw-in-after", + "Identifier: tracking-late", + "Program:exit: tracking", + "Program:exit: throw-in-after", + "Program:exit: tracking-late", + "onCodePathEnd: tracking", + "onCodePathEnd: throw-in-after", + "onCodePathEnd: tracking-late", + "after: tracking", + "after: throw-in-after", + "after: tracking-late" +] +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_after/plugin.ts b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/plugin.ts new file mode 100644 index 0000000000000..e8c30649c0e9c --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_after/plugin.ts @@ -0,0 +1,92 @@ +import { eslintCompatPlugin } from "#oxlint/plugins"; + +import type { Rule, Visitor } from "#oxlint/plugins"; + +// Record of all events, used to verify that other rules' `after` hooks run +// even after an error in one rule's `after` hook +const events: string[] = []; + +// Rule which records events. Runs before the throwing rule. +const trackingRule: Rule = { + createOnce() { + return { + before() { + events.push("before: tracking"); + }, + Identifier() { + events.push("Identifier: tracking"); + }, + "Program:exit"() { + events.push("Program:exit: tracking"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking"); + }, + after() { + events.push("after: tracking"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which throws in its `after` hook. +// Other rules' `after` hooks should still run. +const throwInAfterRule: Rule = { + createOnce() { + return { + before() { + events.push("before: throw-in-after"); + }, + Identifier() { + events.push("Identifier: throw-in-after"); + }, + "Program:exit"() { + events.push("Program:exit: throw-in-after"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: throw-in-after"); + }, + after() { + events.push("after: throw-in-after"); + throw new Error("`after` hook threw"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which records events. Runs after the throwing rule. +// Outputs full event log via `console.error` in `after` hook. +const trackingLateRule: Rule = { + createOnce(context) { + return { + before() { + events.push("before: tracking-late"); + }, + Identifier() { + events.push("Identifier: tracking-late"); + }, + "Program:exit"() { + events.push("Program:exit: tracking-late"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking-late"); + }, + after() { + events.push("after: tracking-late"); + // oxlint-disable-next-line eslint/no-console + console.error(`filename: ${context.filename}\nevents:\n${JSON.stringify(events, null, 2)}`); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +export default eslintCompatPlugin({ + meta: { + name: "eslint-compat-plugin", + }, + rules: { + tracking: trackingRule, + "throw-in-after": throwInAfterRule, + "tracking-late": trackingLateRule, + }, +}); diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/.oxlintrc.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/.oxlintrc.json new file mode 100644 index 0000000000000..4b50bb02ffe01 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/.oxlintrc.json @@ -0,0 +1,9 @@ +{ + "jsPlugins": ["./plugin.ts"], + "categories": { "correctness": "off" }, + "rules": { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-before": "error", + "eslint-compat-plugin/tracking-late": "error" + } +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.config.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.config.js new file mode 100644 index 0000000000000..2c08b3f39e56b --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.config.js @@ -0,0 +1,15 @@ +import plugin from "./plugin.ts"; + +export default [ + { + files: ["files/1.js"], + plugins: { + "test-plugin": plugin, + }, + rules: { + "test-plugin/tracking": "error", + "test-plugin/throw-in-before": "error", + "test-plugin/tracking-late": "error", + }, + }, +]; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.snap.md new file mode 100644 index 0000000000000..38a20ba4be5a8 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/eslint.snap.md @@ -0,0 +1,25 @@ +# Exit code +2 + +# stdout +``` +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-before", + "after: tracking" +] + +Oops! Something went wrong! :( + +ESLint: 9.39.4 + +Error: Error while loading rule 'test-plugin/throw-in-before': `before` hook threw +Occurred while linting /files/1.js + at before (/plugin.ts:44:15) +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/files/1.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/files/1.js new file mode 100644 index 0000000000000..2756c24c45775 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/files/1.js @@ -0,0 +1 @@ +let x; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/options.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/options.json new file mode 100644 index 0000000000000..f380a596b9d7a --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/options.json @@ -0,0 +1,3 @@ +{ + "eslint": true +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/output.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/output.snap.md new file mode 100644 index 0000000000000..c7a15e36d88dd --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/output.snap.md @@ -0,0 +1,24 @@ +# Exit code +1 + +# stdout +``` + x Error running JS plugin. + | File path: /files/1.js + | Error: `before` hook threw + | at before (/plugin.ts:44:15) + +Found 0 warnings and 1 error. +Finished in Xms on 1 file with 3 rules using X threads. +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-before", + "after: tracking" +] +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_before/plugin.ts b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/plugin.ts new file mode 100644 index 0000000000000..b86de3619a483 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_before/plugin.ts @@ -0,0 +1,102 @@ +import { eslintCompatPlugin } from "#oxlint/plugins"; + +import type { Rule, Visitor } from "#oxlint/plugins"; + +// Record of all events, used to verify that `after` hooks run correctly +// even after an error in one rule's `before` hook. +// In ESLint compat mode, `before` is called inside `create`, so `create` throws. +// ESLint crashes on `create` errors, but pending `after` hooks are still run. +const events: string[] = []; + +// Rule which records events. Runs before the throwing rule. +const trackingRule: Rule = { + createOnce(context) { + return { + before() { + events.push("before: tracking"); + }, + Identifier() { + events.push("Identifier: tracking"); + }, + "Program:exit"() { + events.push("Program:exit: tracking"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking"); + }, + after() { + events.push("after: tracking"); + // oxlint-disable-next-line eslint/no-console + console.error(`filename: ${context.filename}\nevents:\n${JSON.stringify(events, null, 2)}`); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which throws in its `before` hook. +// In ESLint compat mode, this causes `create` to throw, which crashes ESLint. +// This rule's `after` hook should NOT be called (because `before` hook threw). +const throwInBeforeRule: Rule = { + createOnce() { + return { + before() { + events.push("before: throw-in-before"); + throw new Error("`before` hook threw"); + }, + Identifier() { + events.push("Identifier: throw-in-before"); + }, + "Program:exit"() { + events.push("Program:exit: throw-in-before"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: throw-in-before"); + }, + after() { + // Should not be called because `before` hook threw, + // so `setupAfterHook` was never called + events.push("after: throw-in-before"); + // oxlint-disable-next-line eslint/no-console + console.error("`after` hook in `throw-in-before` rule should not be called"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which records events. Listed after the throwing rule in the plugin. +// ESLint crashes on throw-in-before's `create`, so this rule's `create` is never called. +// Outputs full event log via `console.error` in `after` hook. +const trackingLateRule: Rule = { + createOnce() { + return { + before() { + events.push("before: tracking-late"); + }, + Identifier() { + events.push("Identifier: tracking-late"); + }, + "Program:exit"() { + events.push("Program:exit: tracking-late"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking-late"); + }, + after() { + events.push("after: tracking-late"); + // oxlint-disable-next-line eslint/no-console + console.error("`after` hook in `tracking-late` rule should not be called"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +export default eslintCompatPlugin({ + meta: { + name: "eslint-compat-plugin", + }, + rules: { + tracking: trackingRule, + "throw-in-before": throwInBeforeRule, + "tracking-late": trackingLateRule, + }, +}); diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/.oxlintrc.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/.oxlintrc.json new file mode 100644 index 0000000000000..d11e1b5c27d38 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/.oxlintrc.json @@ -0,0 +1,9 @@ +{ + "jsPlugins": ["./plugin.ts"], + "categories": { "correctness": "off" }, + "rules": { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-visit": "error", + "eslint-compat-plugin/tracking-late": "error" + } +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.config.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.config.js new file mode 100644 index 0000000000000..4274a9da8a7d9 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.config.js @@ -0,0 +1,15 @@ +import plugin from "./plugin.ts"; + +export default [ + { + files: ["files/*.js"], + plugins: { + "eslint-compat-plugin": plugin, + }, + rules: { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-visit": "error", + "eslint-compat-plugin/tracking-late": "error", + }, + }, +]; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.snap.md new file mode 100644 index 0000000000000..7ef3f2a998abb --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/eslint.snap.md @@ -0,0 +1,37 @@ +# Exit code +2 + +# stdout +``` +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-visit", + "before: tracking-late", + "Identifier: tracking", + "Identifier: throw-in-visit", + "Identifier: tracking-late", + "Program:exit: tracking", + "Program:exit: throw-in-visit", + "Program:exit: tracking-late", + "onCodePathEnd: tracking", + "onCodePathEnd: throw-in-visit", + "after: tracking", + "after: throw-in-visit", + "after: tracking-late" +] + +Oops! Something went wrong! :( + +ESLint: 9.39.4 + +Error: `onCodePathEnd` CFG event handler threw +Occurred while linting /files/1.js +Rule: "eslint-compat-plugin/throw-in-visit" + at onCodePathEnd (/plugin.ts:48:15) +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/files/1.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/files/1.js new file mode 100644 index 0000000000000..2756c24c45775 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/files/1.js @@ -0,0 +1 @@ +let x; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/options.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/options.json new file mode 100644 index 0000000000000..f380a596b9d7a --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/options.json @@ -0,0 +1,3 @@ +{ + "eslint": true +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/output.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/output.snap.md new file mode 100644 index 0000000000000..ed32833f9c63f --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/output.snap.md @@ -0,0 +1,35 @@ +# Exit code +1 + +# stdout +``` + x Error running JS plugin. + | File path: /files/1.js + | Error: `onCodePathEnd` CFG event handler threw + | at onCodePathEnd (/plugin.ts:48:15) + +Found 0 warnings and 1 error. +Finished in Xms on 1 file with 3 rules using X threads. +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-visit", + "before: tracking-late", + "Identifier: tracking", + "Identifier: throw-in-visit", + "Identifier: tracking-late", + "Program:exit: tracking", + "Program:exit: throw-in-visit", + "Program:exit: tracking-late", + "onCodePathEnd: tracking", + "onCodePathEnd: throw-in-visit", + "after: tracking", + "after: throw-in-visit", + "after: tracking-late" +] +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/plugin.ts b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/plugin.ts new file mode 100644 index 0000000000000..b07656c9ebe71 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_onCodePathEnd/plugin.ts @@ -0,0 +1,92 @@ +import { eslintCompatPlugin } from "#oxlint/plugins"; + +import type { Rule, Visitor } from "#oxlint/plugins"; + +// Record of all events, used to verify that `after` hooks run in correct order +// even after an error during AST visitation +const events: string[] = []; + +// Rule which records events. Runs before the throwing rule. +const trackingRule: Rule = { + createOnce() { + return { + before() { + events.push("before: tracking"); + }, + Identifier() { + events.push("Identifier: tracking"); + }, + "Program:exit"() { + events.push("Program:exit: tracking"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking"); + }, + after() { + events.push("after: tracking"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which throws during AST visitation. +// Its `after` hook should still be called. +const throwInVisitRule: Rule = { + createOnce() { + return { + before() { + events.push("before: throw-in-visit"); + }, + Identifier() { + events.push("Identifier: throw-in-visit"); + }, + "Program:exit"() { + events.push("Program:exit: throw-in-visit"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: throw-in-visit"); + throw new Error("`onCodePathEnd` CFG event handler threw"); + }, + after() { + events.push("after: throw-in-visit"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which records events. Runs after the throwing rule. +// Outputs full event log via `console.error` in `after` hook. +const trackingLateRule: Rule = { + createOnce(context) { + return { + before() { + events.push("before: tracking-late"); + }, + Identifier() { + events.push("Identifier: tracking-late"); + }, + "Program:exit"() { + events.push("Program:exit: tracking-late"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking-late"); + }, + after() { + events.push("after: tracking-late"); + // oxlint-disable-next-line eslint/no-console + console.error(`filename: ${context.filename}\nevents:\n${JSON.stringify(events, null, 2)}`); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +export default eslintCompatPlugin({ + meta: { + name: "eslint-compat-plugin", + }, + rules: { + tracking: trackingRule, + "throw-in-visit": throwInVisitRule, + "tracking-late": trackingLateRule, + }, +}); diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/.oxlintrc.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/.oxlintrc.json new file mode 100644 index 0000000000000..d11e1b5c27d38 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/.oxlintrc.json @@ -0,0 +1,9 @@ +{ + "jsPlugins": ["./plugin.ts"], + "categories": { "correctness": "off" }, + "rules": { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-visit": "error", + "eslint-compat-plugin/tracking-late": "error" + } +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.config.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.config.js new file mode 100644 index 0000000000000..4274a9da8a7d9 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.config.js @@ -0,0 +1,15 @@ +import plugin from "./plugin.ts"; + +export default [ + { + files: ["files/*.js"], + plugins: { + "eslint-compat-plugin": plugin, + }, + rules: { + "eslint-compat-plugin/tracking": "error", + "eslint-compat-plugin/throw-in-visit": "error", + "eslint-compat-plugin/tracking-late": "error", + }, + }, +]; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.snap.md new file mode 100644 index 0000000000000..b923da4b58e21 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/eslint.snap.md @@ -0,0 +1,31 @@ +# Exit code +2 + +# stdout +``` +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-visit", + "before: tracking-late", + "Identifier: tracking", + "Identifier: throw-in-visit", + "after: tracking", + "after: throw-in-visit", + "after: tracking-late" +] + +Oops! Something went wrong! :( + +ESLint: 9.39.4 + +Error: `Identifier` visit function threw +Occurred while linting /files/1.js:1 +Rule: "eslint-compat-plugin/throw-in-visit" + at Identifier (/plugin.ts:42:15) +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/files/1.js b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/files/1.js new file mode 100644 index 0000000000000..2756c24c45775 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/files/1.js @@ -0,0 +1 @@ +let x; diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/options.json b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/options.json new file mode 100644 index 0000000000000..f380a596b9d7a --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/options.json @@ -0,0 +1,3 @@ +{ + "eslint": true +} diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/output.snap.md b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/output.snap.md new file mode 100644 index 0000000000000..5c1a49bd74ba1 --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/output.snap.md @@ -0,0 +1,29 @@ +# Exit code +1 + +# stdout +``` + x Error running JS plugin. + | File path: /files/1.js + | Error: `Identifier` visit function threw + | at Identifier (/plugin.ts:42:15) + +Found 0 warnings and 1 error. +Finished in Xms on 1 file with 3 rules using X threads. +``` + +# stderr +``` +filename: /files/1.js +events: +[ + "before: tracking", + "before: throw-in-visit", + "before: tracking-late", + "Identifier: tracking", + "Identifier: throw-in-visit", + "after: tracking", + "after: throw-in-visit", + "after: tracking-late" +] +``` diff --git a/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/plugin.ts b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/plugin.ts new file mode 100644 index 0000000000000..c149b1c9fccbc --- /dev/null +++ b/apps/oxlint/test/fixtures/eslintCompat_error_in_visit/plugin.ts @@ -0,0 +1,92 @@ +import { eslintCompatPlugin } from "#oxlint/plugins"; + +import type { Rule, Visitor } from "#oxlint/plugins"; + +// Record of all events, used to verify that `after` hooks run in correct order +// even after an error during AST visitation +const events: string[] = []; + +// Rule which records events. Runs before the throwing rule. +const trackingRule: Rule = { + createOnce() { + return { + before() { + events.push("before: tracking"); + }, + Identifier() { + events.push("Identifier: tracking"); + }, + "Program:exit"() { + events.push("Program:exit: tracking"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking"); + }, + after() { + events.push("after: tracking"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which throws during AST visitation. +// Its `after` hook should still be called. +const throwInVisitRule: Rule = { + createOnce() { + return { + before() { + events.push("before: throw-in-visit"); + }, + Identifier() { + events.push("Identifier: throw-in-visit"); + throw new Error("`Identifier` visit function threw"); + }, + "Program:exit"() { + events.push("Program:exit: throw-in-visit"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: throw-in-visit"); + }, + after() { + events.push("after: throw-in-visit"); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +// Rule which records events. Runs after the throwing rule. +// Outputs full event log via `console.error` in `after` hook. +const trackingLateRule: Rule = { + createOnce(context) { + return { + before() { + events.push("before: tracking-late"); + }, + Identifier() { + events.push("Identifier: tracking-late"); + }, + "Program:exit"() { + events.push("Program:exit: tracking-late"); + }, + onCodePathEnd() { + events.push("onCodePathEnd: tracking-late"); + }, + after() { + events.push("after: tracking-late"); + // oxlint-disable-next-line eslint/no-console + console.error(`filename: ${context.filename}\nevents:\n${JSON.stringify(events, null, 2)}`); + }, + } as unknown as Visitor; // TODO: Our types don't include CFG event handlers at present + }, +}; + +export default eslintCompatPlugin({ + meta: { + name: "eslint-compat-plugin", + }, + rules: { + tracking: trackingRule, + "throw-in-visit": throwInVisitRule, + "tracking-late": trackingLateRule, + }, +});