diff --git a/apps/oxlint/src-js/plugins/visitor.ts b/apps/oxlint/src-js/plugins/visitor.ts index ad7d4799c29fa..409d4ecbd2f90 100644 --- a/apps/oxlint/src-js/plugins/visitor.ts +++ b/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); + }, +]; diff --git a/apps/oxlint/test/fixtures/cfg/.oxlintrc.json b/apps/oxlint/test/fixtures/cfg/.oxlintrc.json index 76b546efca906..29d549d85be4a 100644 --- a/apps/oxlint/test/fixtures/cfg/.oxlintrc.json +++ b/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" } } diff --git a/apps/oxlint/test/fixtures/cfg/options.json b/apps/oxlint/test/fixtures/cfg/options.json deleted file mode 100644 index c6d966f1b525b..0000000000000 --- a/apps/oxlint/test/fixtures/cfg/options.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "singleThread": true -} diff --git a/apps/oxlint/test/fixtures/cfg/output.snap.md b/apps/oxlint/test/fixtures/cfg/output.snap.md index eb6f91e28a45f..8f1b450ca8d06 100644 --- a/apps/oxlint/test/fixtures/cfg/output.snap.md +++ b/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 diff --git a/apps/oxlint/test/fixtures/cfg/plugin.ts b/apps/oxlint/test/fixtures/cfg/plugin.ts index 7e909ea6bceec..46f72f66fbf5f 100644 --- a/apps/oxlint/test/fixtures/cfg/plugin.ts +++ b/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, }, };