fix(linter/plugins): pass all args to CFG event handlers when 2 rules use same handler

apps/oxlint/src-js/plugins/visitor.ts

@@ -97,6 +97,9 @@ import type { Node, Visitor } from "./types.ts";
 // Visit function for a specific AST node type.
 export type VisitFn = (node: Node) => void;
 
+// Visit function for a specific CFG event.
+type CfgVisitFn = (...args: unknown[]) => void;
+
 // Enter+exit pair, for non-leaf nodes in compiled visitor.
 export interface EnterExit {
   enter: VisitFn | null;
@@ -462,7 +465,7 @@ export function finalizeCompiledVisitor(): VisitorState {
 
     for (let i = activeCfgVisitorTypeIds.length - 1; i >= 0; i--) {
       const typeId = activeCfgVisitorTypeIds[i]!;
-      compiledVisitor[typeId] = mergeVisitFns(compilingCfgVisitor[typeId - NODE_TYPES_COUNT]!);
+      compiledVisitor[typeId] = mergeCfgVisitFns(compilingCfgVisitor[typeId - NODE_TYPES_COUNT]!);
     }
 
     // Reset state, ready for next time
@@ -486,7 +489,7 @@ export function finalizeCompiledVisitor(): VisitorState {
   return visitState;
 }
 
-// Array used by `mergeVisitFns` to store visit functions extracted from an array of `VisitProp`s.
+// Array used by `mergeVisitFns` and `mergeCfgVisitFns` to store visit functions extracted from an array of `VisitProp`s.
 // This array is used ephemerally, so we re-use same array for each merge.
 const visitFns: VisitFn[] = [];
 
@@ -612,3 +615,128 @@ const mergers: (Merger | null)[] = [
       visit5(node);
     },
 ];
+
+/**
+ * Merge array of CFG visit functions into a single function, which calls each of input functions in turn.
+ *
+ * The difference between this function and `mergeVisitFns` is that this function is for CFG event handlers.
+ * Unlike all other visit functions, CFG event handlers are called with more than 1 argument.
+ * We keep this separate from `mergeVisitFns` because the merger functions use `...args` to pass all arguments,
+ * which is likely less performant than the simpler version which passes a single argument.
+ * AST node visitation is a lot more common than CFG event visitation, and we want to keep the common case fast.
+ *
+ * The array passed is cleared (length set to 0), so the array can be reused.
+ *
+ * The merged function is statically defined and does not contain a loop, to hopefully allow
+ * JS engine to heavily optimize it.
+ *
+ * `cfgMergers` contains pre-defined functions to merge up to 5 CFG visit functions.
+ * Merger functions for merging more than 5 visit functions are created dynamically on demand.
+ *
+ * @param visitProps - Array of `VisitProp` objects
+ * @returns Function which calls all CFG visit functions in turn
+ */
+function mergeCfgVisitFns(visitProps: VisitProp[]): CfgVisitFn {
+  const numVisitFns = visitProps.length;
+
+  debugAssert(numVisitFns > 0, "`visitProps` should have at least 1 element");
+
+  let mergedFn: CfgVisitFn;
+  if (numVisitFns === 1) {
+    // Only 1 visit function, so no need to merge
+    debugAssertIsNonNull(visitProps[0].fn);
+    mergedFn = visitProps[0].fn;
+  } else {
+    // No need to sort in order of specificity, because each rule can only have 1 handler for each CFG event
+
+    // Get or create merger for merging `numVisitFns` functions
+    let merger: CfgMerger | null;
+    if (cfgMergers.length <= numVisitFns) {
+      while (cfgMergers.length < numVisitFns) {
+        cfgMergers.push(null);
+      }
+      merger = createCfgMerger(numVisitFns);
+      cfgMergers.push(merger);
+    } else {
+      merger = cfgMergers[numVisitFns];
+      if (merger === null) merger = cfgMergers[numVisitFns] = createCfgMerger(numVisitFns);
+    }
+
+    // Merge functions.
+    // Reuse a temporary array to avoid creating a new array for each merge.
+    // TODO: Make merger functions take an array of `VisitProp`s to avoid this operation?
+    debugAssert(visitFns.length === 0, "`visitFns` should be empty");
+
+    for (let i = 0; i < numVisitFns; i++) {
+      debugAssertIsNonNull(visitProps[i].fn);
+      visitFns.push(visitProps[i].fn!);
+    }
+    mergedFn = merger(...visitFns);
+
+    visitFns.length = 0;
+  }
+
+  // Empty `visitProps` array, so it can be reused
+  visitProps.length = 0;
+
+  return mergedFn;
+}
+
+type CfgMerger = (...visitFns: CfgVisitFn[]) => CfgVisitFn;
+
+/**
+ * Create a CFG merger function that merges `fnCount` functions.
+ *
+ * @param fnCount - Number of functions to be merged
+ * @returns Function to merge `fnCount` functions
+ */
+function createCfgMerger(fnCount: number): CfgMerger {
+  const args = [];
+  let body = "return (...args)=>{";
+  for (let i = 1; i <= fnCount; i++) {
+    args.push(`visit${i}`);
+    body += `visit${i}(...args);`;
+  }
+  body += "}";
+  args.push(body);
+  // oxlint-disable-next-line typescript-eslint/no-implied-eval
+  return new Function(...args) as CfgMerger;
+}
+
+// Pre-defined CFG mergers for merging up to 5 functions
+const cfgMergers: (CfgMerger | null)[] = [
+  null, // No merger for 0 functions
+  null, // No merger for 1 function
+  (visit1: CfgVisitFn, visit2: CfgVisitFn) =>
+    (...args: unknown[]) => {
+      visit1(...args);
+      visit2(...args);
+    },
+  (visit1: CfgVisitFn, visit2: CfgVisitFn, visit3: CfgVisitFn) =>
+    (...args: unknown[]) => {
+      visit1(...args);
+      visit2(...args);
+      visit3(...args);
+    },
+  (visit1: CfgVisitFn, visit2: CfgVisitFn, visit3: CfgVisitFn, visit4: CfgVisitFn) =>
+    (...args: unknown[]) => {
+      visit1(...args);
+      visit2(...args);
+      visit3(...args);
+      visit4(...args);
+    },
+  (
+    visit1: CfgVisitFn,
+    visit2: CfgVisitFn,
+    visit3: CfgVisitFn,
+    visit4: CfgVisitFn,
+    visit5: CfgVisitFn,
+  ) =>
+    (...args: unknown[]) => {
+      visit1(...args);
+      visit2(...args);
+      visit3(...args);
+      visit4(...args);
+      visit5(...args);
+    },
+];

apps/oxlint/test/fixtures/cfg/.oxlintrc.json

@@ -2,6 +2,7 @@
   "jsPlugins": ["./plugin.ts"],
   "categories": { "correctness": "off" },
   "rules": {
-    "error-plugin/error": "error"
+    "cfg-plugin/cfg": "error",
+    "cfg-plugin/noop": "error"
   }
 }

apps/oxlint/test/fixtures/cfg/output.snap.md

@@ -3,7 +3,7 @@
 
 # stdout
 ```
-  x error-plugin(error): Visited nodes:
+  x cfg-plugin(cfg): Visited nodes:
   | * onCodePathStart                     Program
   | * onCodePathSegmentStart              Program
   | * onCodePathSegmentEnd                Literal
@@ -21,7 +21,7 @@
    `----
 
 Found 0 warnings and 1 error.
-Finished in Xms on 1 file with 1 rules using X threads.
+Finished in Xms on 1 file with 2 rules using X threads.
 ```
 
 # stderr

apps/oxlint/test/fixtures/cfg/plugin.ts

@@ -44,12 +44,27 @@ const rule: Rule = {
   },
 };
 
+// Purpose of this 2nd rule is to ensure that all arguments are passed to the CFG event handler functions
+// when 2 event handler functions are merged into a single function.
+// https://github.com/oxc-project/oxc/issues/18555
+const rule2: Rule = {
+  // @ts-expect-error - TODO: Make the types for CFG events work
+  create(_context) {
+    return {
+      onCodePathSegmentLoop(_fromSegment: any, _toSegment: any, _node: Node) {
+        // Do nothing
+      },
+    };
+  },
+};
+
 const plugin: Plugin = {
   meta: {
-    name: "error-plugin",
+    name: "cfg-plugin",
   },
   rules: {
-    error: rule,
+    cfg: rule,
+    noop: rule2,
   },
 };