fix(linter/plugins): reset visitor compilation state if error during compilation (#20166)

overlookmotel · overlookmotel · commit c4812ec50e88 · 2026-03-10T01:46:49.000Z
Fix a bug where internal state of visitor compilation could fail to be reset if an error is thrown during compilation, for example due to an invalid visitor being passed.

If this happens, previously some of the visitor functions defined for one file could stick around, and be erroneously included in the compiled visitor for next file, leading to them firing on that 2nd file when they shouldn't.

Fix that by resetting visitor compilation state in the `try` / `catch` which wraps the whole linting process.
diff --git a/apps/oxlint/src-js/plugins/lint.ts b/apps/oxlint/src-js/plugins/lint.ts
@@ -274,6 +274,13 @@ export function resetFile() {
  * in the correct initial state for linting the next file.
  */
 export function resetStateAfterError() {
+  // In case error occurred during visitor compilation, clear internal state of visitor compilation,
+  // so no leftovers bleed into next file.
+  // We could have a separate function to reset state which could be simpler and faster, but `resetStateAfterError`
+  // should never be called - only happens when rules return an invalid visitor or malfunction.
+  // So better to use the existing function, rather than bloat the package with more code which should never run.
+  finalizeCompiledVisitor();
+
   diagnostics.length = 0;
   ancestors.length = 0;
   resetFile();
diff --git a/apps/oxlint/test/fixtures/lint_invalid_visitor/.oxlintrc.json b/apps/oxlint/test/fixtures/lint_invalid_visitor/.oxlintrc.json
@@ -0,0 +1,7 @@
+{
+  "jsPlugins": ["./plugin.ts"],
+  "categories": { "correctness": "off" },
+  "rules": {
+    "error-plugin/error": "error"
+  }
+}
diff --git a/apps/oxlint/test/fixtures/lint_invalid_visitor/files/1.js b/apps/oxlint/test/fixtures/lint_invalid_visitor/files/1.js
@@ -0,0 +1 @@
+let x;
diff --git a/apps/oxlint/test/fixtures/lint_invalid_visitor/files/2.js b/apps/oxlint/test/fixtures/lint_invalid_visitor/files/2.js
@@ -0,0 +1 @@
+let y;
diff --git a/apps/oxlint/test/fixtures/lint_invalid_visitor/options.json b/apps/oxlint/test/fixtures/lint_invalid_visitor/options.json
@@ -0,0 +1,3 @@
+{
+  "singleThread": true
+}
diff --git a/apps/oxlint/test/fixtures/lint_invalid_visitor/output.snap.md b/apps/oxlint/test/fixtures/lint_invalid_visitor/output.snap.md
@@ -0,0 +1,22 @@
+# Exit code
+1
+
+# stdout
+```
+  x Error running JS plugin.
+  | File path: <fixture>/files/1.js
+  | TypeError: 'Program' property of visitor object is not a function
+
+  x error-plugin(error): Identifier in 2nd file: y
+   ,-[files/2.js:1:5]
+ 1 | let y;
+   :     ^
+   `----
+
+Found 0 warnings and 2 errors.
+Finished in Xms on 2 files with 1 rules using X threads.
+```
+
+# stderr
+```
+```
diff --git a/apps/oxlint/test/fixtures/lint_invalid_visitor/plugin.ts b/apps/oxlint/test/fixtures/lint_invalid_visitor/plugin.ts
@@ -0,0 +1,64 @@
+import assert from "node:assert";
+
+import type { Plugin, Rule } from "#oxlint/plugins";
+
+// Aim of this test is:
+//
+// 1. Check that errors thrown during visitor compilation are handled correctly and shown to user as diagnostics.
+// 2. Check that global state is reset after an error during visiting a file, so it's in correct initial state
+//    when Oxlint starts linting the next file.
+//
+// The 2nd is tricky to test because usually the order Oxlint lints files in is non-deterministic.
+// To make this test deterministic, we run it with `oxlint --threads 1`
+// (`options.json` file for this fixture contains `"singleThread": true`).
+// This guarantees that `1.js` is linted before `2.js`.
+//
+// This rule throws an error during visitor compilation when linting 1st file. If global state is not reset properly
+// before linting 2nd file, `Identifier` visitor for 1st file will not be cleaned up and will fire on 2nd file.
+
+let fileIndex = 0;
+
+const rule: Rule = {
+  create(context) {
+    const isFirstFileLinted = fileIndex === 0;
+    fileIndex++;
+
+    // Check the order files get linted in is what we expect
+    const isFirstFile = context.filename.endsWith("1.js");
+    assert(isFirstFile === isFirstFileLinted);
+
+    if (isFirstFile) {
+      return {
+        Identifier(node) {
+          context.report({
+            message: `Identifier in 1st file: ${node.name}`,
+            node,
+          });
+        },
+
+        // Illegal value, causes error during visitor compilation
+        Program: 123 as any,
+      };
+    }
+
+    return {
+      Identifier(node) {
+        context.report({
+          message: `Identifier in 2nd file: ${node.name}`,
+          node,
+        });
+      },
+    };
+  },
+};
+
+const plugin: Plugin = {
+  meta: {
+    name: "error-plugin",
+  },
+  rules: {
+    error: rule,
+  },
+};
+
+export default plugin;
