From de544316f305d2384b6b49954533d11c402c0206 Mon Sep 17 00:00:00 2001 From: andres Date: Tue, 1 Sep 2026 20:15:33 +0200 Subject: [PATCH] refactor(vba): replace the positional extractor parameters with an options object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The VBA extraction knobs travelled as positional parameters through five files (project-config -> extraction/index -> parse-pool -> parse-worker -> tree-sitter -> vba-extractor). Adding one knob meant editing every signature and every call site. Introduce the leaf module `src/extraction/vba/options.ts` exporting `VbaExtractionOptions` (`targets`, `maxRaiseFanout`, `sqlWrappers`) and thread that ONE object end to end: - `VbaExtractor` takes `options` as its 3rd parameter. The positional 3rd/4th form is kept working through a `@deprecated` overload for one release; every in-repo call site moves to the object form. - `extractFromSource` gains a trailing `vbaOptions`; the legacy positionals merge into it per field, with the object winning. - `ParseTask` and the worker message carry `vbaOptions` as one field. `sqlWrappers` is declared and threaded now; the SQL-wrapper task consumes it. The object stays plain structured-cloneable data because it crosses the `structuredClone`-based parse-worker boundary — no functions, no RegExp, no Map. A compiled matcher belongs on the extractor context. Pure refactor: the VBA coverage probe over 00_EXPEDIENTES and 00_GESTION_RIESGOS is byte-identical to main. --- __tests__/extraction-vba-event-fanout.test.ts | 6 +- __tests__/extraction-vba-options.test.ts | 290 ++++++++++++++++++ src/extraction/index.ts | 25 +- src/extraction/parse-pool.ts | 14 +- src/extraction/parse-worker.ts | 16 +- src/extraction/tree-sitter.ts | 25 +- src/extraction/vba-extractor.ts | 75 ++++- src/extraction/vba/options.ts | 72 +++++ 8 files changed, 496 insertions(+), 27 deletions(-) create mode 100644 __tests__/extraction-vba-options.test.ts create mode 100644 src/extraction/vba/options.ts diff --git a/__tests__/extraction-vba-event-fanout.test.ts b/__tests__/extraction-vba-event-fanout.test.ts index 207d5cc8..6c7a6d79 100644 --- a/__tests__/extraction-vba-event-fanout.test.ts +++ b/__tests__/extraction-vba-event-fanout.test.ts @@ -25,7 +25,11 @@ import { VbaExtractor, DEFAULT_MAX_RAISE_FANOUT } from '../src/extraction/vba-ex import { loadVbaConfig, clearProjectConfigCache } from '../src/project-config'; function extract(filePath: string, source: string, maxRaiseFanout?: number) { - return new VbaExtractor(filePath, source, undefined, maxRaiseFanout).extract(); + // Issue #243: the options-object form. An absent `maxRaiseFanout` still + // means "use DEFAULT_MAX_RAISE_FANOUT", exactly as the old positional form + // did — the legacy positional form is pinned in + // `extraction-vba-options.test.ts`. + return new VbaExtractor(filePath, source, { maxRaiseFanout }).extract(); } /** Build a `.cls` with one event raised `n` times across N small Sub procs. */ diff --git a/__tests__/extraction-vba-options.test.ts b/__tests__/extraction-vba-options.test.ts new file mode 100644 index 00000000..ef77b483 --- /dev/null +++ b/__tests__/extraction-vba-options.test.ts @@ -0,0 +1,290 @@ +/** + * Issue #243 — `VbaExtractionOptions`, the one object that replaces seven + * positional VBA-extraction parameters across five files. + * + * This is a PURE REFACTOR, so every test here is an equivalence test: the new + * object form must produce the SAME `ExtractionResult` as the old positional + * form, and the knobs must keep gating exactly as they did. + * + * Acceptance criteria pinned here (from the issue body): + * - object form and legacy positional form produce deep-equal + * `ExtractionResult` (minus the wall-clock `durationMs`) + * - an option set on the object survives a REAL worker round-trip through + * `parse-pool` — i.e. it crosses `structuredClone` intact + * - `maxRaiseFanout` still gates as before (regression guard for #152) + * + * Real files, real extractor, a real `worker_threads.Worker`. No mocking. + */ +import { describe, it, expect } from 'vitest'; +import { Worker } from 'node:worker_threads'; +import { VbaExtractor, DEFAULT_MAX_RAISE_FANOUT } from '../src/extraction/vba-extractor'; +import { extractFromSource } from '../src/extraction/tree-sitter'; +import { ParseWorkerPool, type ParsePoolWorker } from '../src/extraction/parse-pool'; +import type { VbaExtractionOptions } from '../src/extraction/vba/options'; +import type { ExtractionResult, Language } from '../src/types'; + +const FILE = 'src/classes/OptionsProbe.cls'; + +/** + * A module that exercises BOTH knobs at once: + * - `#If DEV` makes the result depend on `targets` + * - `RaiseEvent Changed` makes it depend on `maxRaiseFanout` + */ +const VBA_SRC = [ + 'Attribute VB_Name = "OptionsProbe"', + 'Public Event Changed()', + '', + '#If DEV Then', + 'Public Sub DevOnly()', + ' Helper', + 'End Sub', + '#End If', + '', + 'Public Sub DoWork()', + ' RaiseEvent Changed', + ' Helper', + 'End Sub', + '', + 'Public Sub Helper()', + 'End Sub', + '', +].join('\n'); + +/** Build a `.cls` with one event raised `n` times, one raise site per Sub. */ +function buildClassWithRaisedEvent(eventName: string, n: number): string { + const lines = ['Attribute VB_Name = "FanoutProbe"', `Public Event ${eventName}()`]; + for (let i = 0; i < n; i++) { + lines.push(`Public Sub RaiseSite_${i}()`, ` RaiseEvent ${eventName}`, 'End Sub'); + } + return lines.join('\n'); +} + +/** + * `ExtractionResult.durationMs` and `Node.updatedAt` are wall-clock readings, + * so they differ between two runs of identical input. They are the ONLY fields + * an equivalence assertion may ignore. Everything else — every node field, + * every edge, every unresolved reference, every error — must match. + */ +function comparable(r: ExtractionResult): unknown { + const { durationMs: _durationMs, ...rest } = r; + return { + ...rest, + nodes: rest.nodes.map(({ updatedAt: _updatedAt, ...node }) => node), + }; +} + +const names = (r: ExtractionResult): string[] => r.nodes.map((n) => n.name).sort(); +const raiseEdges = (r: ExtractionResult) => r.edges.filter((e) => e.kind === 'raises-event'); + +describe('Issue #243 — VbaExtractor options object vs the legacy positionals', () => { + it('object form and legacy positional form produce deep-equal results', () => { + const objectForm = new VbaExtractor(FILE, VBA_SRC, { + targets: { DEV: true }, + maxRaiseFanout: 7, + }).extract(); + // The `@deprecated` overload: targets 3rd, maxRaiseFanout 4th. + const legacyForm = new VbaExtractor(FILE, VBA_SRC, { DEV: true }, 7).extract(); + + expect(comparable(objectForm)).toEqual(comparable(legacyForm)); + // Guard the guard: if `targets` were silently dropped, both runs would be + // equal AND wrong. `DevOnly` only survives preprocessing when DEV is true. + expect(names(objectForm)).toContain('DevOnly'); + }); + + it('targets threads through the object exactly as the positional did (DEV off)', () => { + const objectForm = new VbaExtractor(FILE, VBA_SRC, { targets: { DEV: false } }).extract(); + const legacyForm = new VbaExtractor(FILE, VBA_SRC, { DEV: false }).extract(); + + expect(comparable(objectForm)).toEqual(comparable(legacyForm)); + expect(names(objectForm)).not.toContain('DevOnly'); + }); + + it('no options, empty options, and explicit-undefined positionals are all the same run', () => { + const bare = new VbaExtractor(FILE, VBA_SRC).extract(); + const emptyObject = new VbaExtractor(FILE, VBA_SRC, {}).extract(); + const explicitUndefined = new VbaExtractor(FILE, VBA_SRC, undefined, undefined).extract(); + + expect(comparable(emptyObject)).toEqual(comparable(bare)); + expect(comparable(explicitUndefined)).toEqual(comparable(bare)); + // The zero-config default leaves the DEV branch out. + expect(names(bare)).not.toContain('DevOnly'); + }); + + it('an absent maxRaiseFanout still falls back to DEFAULT_MAX_RAISE_FANOUT', () => { + const src = buildClassWithRaisedEvent('Changed', DEFAULT_MAX_RAISE_FANOUT + 10); + const viaObject = new VbaExtractor(FILE, src, { targets: undefined }).extract(); + const viaLegacy = new VbaExtractor(FILE, src, undefined, undefined).extract(); + + expect(comparable(viaObject)).toEqual(comparable(viaLegacy)); + // Default 50 < 60 raise sites → the gate fires on both. + expect(raiseEdges(viaObject)).toHaveLength(0); + }); +}); + +describe('Issue #243 — maxRaiseFanout still gates as before (regression guard for #152)', () => { + it('over the threshold drops every raises-event edge and flags the node', () => { + const src = buildClassWithRaisedEvent('Changed', 12); + const r = new VbaExtractor(FILE, src, { maxRaiseFanout: 10 }).extract(); + + const event = r.nodes.find((n) => n.kind === 'event' && n.name === 'Changed'); + expect(event?.metadata?.highFanout).toBe(true); + expect(event?.metadata?.raiseCount).toBe(12); + expect(raiseEdges(r)).toHaveLength(0); + }); + + it('at the threshold every edge survives (the gate is a strict `>`)', () => { + const src = buildClassWithRaisedEvent('Changed', 10); + const r = new VbaExtractor(FILE, src, { maxRaiseFanout: 10 }).extract(); + + const event = r.nodes.find((n) => n.kind === 'event' && n.name === 'Changed'); + expect(event?.metadata?.highFanout).toBeUndefined(); + expect(raiseEdges(r)).toHaveLength(10); + }); + + it('the object form and the positional form gate identically', () => { + const src = buildClassWithRaisedEvent('Changed', 12); + const viaObject = new VbaExtractor(FILE, src, { maxRaiseFanout: 10 }).extract(); + const viaLegacy = new VbaExtractor(FILE, src, undefined, 10).extract(); + + expect(comparable(viaObject)).toEqual(comparable(viaLegacy)); + }); +}); + +describe('Issue #243 — extractFromSource merges the legacy positionals into vbaOptions', () => { + it('trailing vbaOptions matches the legacy 5th/6th positionals', () => { + const viaObject = extractFromSource(FILE, VBA_SRC, 'vba', undefined, undefined, undefined, true, { + targets: { DEV: true }, + maxRaiseFanout: 7, + }); + const viaLegacy = extractFromSource(FILE, VBA_SRC, 'vba', undefined, { DEV: true }, 7, true); + + expect(comparable(viaObject)).toEqual(comparable(viaLegacy)); + expect(names(viaObject)).toContain('DevOnly'); + }); + + it('the object wins per field when both forms are supplied', () => { + const src = buildClassWithRaisedEvent('Changed', 12); + // Positional says "cap at 1000" (no gate); the object says "cap at 10". + const merged = extractFromSource(FILE, src, 'vba', undefined, undefined, 1000, true, { + maxRaiseFanout: 10, + }); + const objectOnly = extractFromSource(FILE, src, 'vba', undefined, undefined, undefined, true, { + maxRaiseFanout: 10, + }); + const positionalOnly = extractFromSource(FILE, src, 'vba', undefined, undefined, 1000, true); + + expect(comparable(merged)).toEqual(comparable(objectOnly)); + expect(raiseEdges(merged)).toHaveLength(0); + // Sanity: the positional value alone really would NOT have gated. + expect(raiseEdges(positionalOnly)).toHaveLength(12); + }); + + it('a field absent from both forms falls through to the extractor default', () => { + const viaNothing = extractFromSource(FILE, VBA_SRC, 'vba'); + const viaEmptyObject = extractFromSource(FILE, VBA_SRC, 'vba', undefined, undefined, undefined, true, {}); + + expect(comparable(viaEmptyObject)).toEqual(comparable(viaNothing)); + }); +}); + +describe('Issue #243 — VbaExtractionOptions is plain, structured-cloneable data', () => { + it('survives structuredClone unchanged (no functions, RegExp, Map or class instances)', () => { + const options: VbaExtractionOptions = { + targets: { DEV: true, WIN64: false }, + maxRaiseFanout: 17, + sqlWrappers: ['EjecutarSQL', 'AbrirRecordset'], + }; + + // This is the exact serialization `worker.postMessage` performs. A + // function or a RegExp in the object would throw DataCloneError or lose + // its identity here — which is why the compiled matcher lives on the + // extractor context, not in this object. + expect(structuredClone(options)).toEqual(options); + }); +}); + +/** + * The REAL worker round-trip. A `worker_threads.Worker` speaks the same + * {load-grammars → grammars-loaded} / {parse → parse-result} protocol the + * production `parse-worker` does, and echoes the `vbaOptions` it received back + * across the boundary. Both hops are genuine `structuredClone`s, so this fails + * the moment a non-cloneable value is added to `VbaExtractionOptions`. + * + * It deliberately does NOT load tree-sitter grammars or a built `dist/`: the + * claim under test is that the OPTIONS survive the boundary, not that the + * extractor runs inside a worker (the extraction suite covers that). + */ +const ECHO_WORKER = ` +const { parentPort } = require('node:worker_threads'); +parentPort.on('message', (msg) => { + if (msg.type === 'load-grammars') { + parentPort.postMessage({ type: 'grammars-loaded' }); + return; + } + if (msg.type !== 'parse') return; + parentPort.postMessage({ + type: 'parse-result', + id: msg.id, + result: { + nodes: [], + edges: [], + unresolvedReferences: [], + // The echo channel: the options exactly as they arrived in the worker. + errors: [{ message: JSON.stringify(msg.vbaOptions ?? null) }], + durationMs: 0, + }, + }); +}); +`; + +describe('Issue #243 — vbaOptions survives a real parse-pool worker round-trip', () => { + it('arrives in the worker with every field intact', async () => { + const pool = new ParseWorkerPool({ + languages: ['vba' as Language], + size: 1, + createWorker: () => + new Worker(ECHO_WORKER, { eval: true }) as unknown as ParsePoolWorker, + }); + + const vbaOptions: VbaExtractionOptions = { + targets: { DEV: true, WIN64: false }, + maxRaiseFanout: 17, + sqlWrappers: ['EjecutarSQL', 'AbrirRecordset'], + }; + + try { + const result = await pool.requestParse({ + filePath: FILE, + content: VBA_SRC, + language: 'vba' as Language, + vbaOptions, + }); + + const echoed = JSON.parse(result.errors[0]!.message) as VbaExtractionOptions; + expect(echoed).toEqual(vbaOptions); + } finally { + await pool.destroy(); + } + }); + + it('an omitted vbaOptions arrives as undefined, not as a mangled object', async () => { + const pool = new ParseWorkerPool({ + languages: ['vba' as Language], + size: 1, + createWorker: () => + new Worker(ECHO_WORKER, { eval: true }) as unknown as ParsePoolWorker, + }); + + try { + const result = await pool.requestParse({ + filePath: FILE, + content: VBA_SRC, + language: 'vba' as Language, + }); + + expect(JSON.parse(result.errors[0]!.message)).toBeNull(); + } finally { + await pool.destroy(); + } + }); +}); diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 9b125ef7..98590d3e 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -21,6 +21,7 @@ import { } from '../types'; import { QueryBuilder } from '../db/queries'; import { extractFromSource } from './tree-sitter'; +import type { VbaExtractionOptions } from './vba/options'; import { readVbaSource, isVbaFamilyFile } from './vba-source'; import { ParseWorkerPool, resolveParsePoolSize, resolveParseTimeoutMs } from './parse-pool'; import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLanguage, initGrammars, loadGrammarsForLanguages, readGrammarWasmBytes } from './grammars'; @@ -1603,9 +1604,15 @@ export class ExtractionOrchestrator { // Issue #152: pull the optional `vba.maxRaiseFanout` knob from the // same `codegraph.json` → `vba` block the targets live in. The VBA // extractor applies the per-file fanout gate at this threshold. + // Issue #243: the knobs travel as ONE structured-cloneable options object + // from here to the extractor, on the pool path and the in-process path + // alike, so a new knob is a field here instead of a new positional in + // five signatures. const vbaConfig = loadVbaConfig(this.rootDir); - const vbaTargets = vbaConfig.targets; - const maxRaiseFanout = vbaConfig.maxRaiseFanout; + const vbaOptions: VbaExtractionOptions = { + targets: vbaConfig.targets, + maxRaiseFanout: vbaConfig.maxRaiseFanout, + }; // Issue #154 — gate the 3 Dysflow-specific VBA sub-extractors. `true` // (the default) keeps the pre-refactor behavior; `false` opts out so // form/report/manifest/sequence files are tracked as just a `file` @@ -1724,8 +1731,9 @@ export class ExtractionOrchestrator { // the worker boundary (or the in-process fallback) so every VBA // file sees the same gate. // Issue #154: also thread the optional `vba.dysflowExport` flag. - if (!pool) return Promise.resolve(extractFromSource(filePath, content, language, frameworkNames, vbaTargets, maxRaiseFanout, dysflowExport)); - return pool.requestParse({ filePath, content, language, frameworkNames, vbaTargets, maxRaiseFanout, dysflowExport }); + // Issue #243: both knobs now ride in the `vbaOptions` object. + if (!pool) return Promise.resolve(extractFromSource(filePath, content, language, frameworkNames, undefined, undefined, dysflowExport, vbaOptions)); + return pool.requestParse({ filePath, content, language, frameworkNames, vbaOptions, dysflowExport }); }; // --- Bounded rolling-window dispatch, ordered commit --- @@ -2256,9 +2264,13 @@ export class ExtractionOrchestrator { content, language, frameworkNames, - vbaReindexConfig.targets, - vbaReindexConfig.maxRaiseFanout, + undefined, + undefined, dysflowExport, + { + targets: vbaReindexConfig.targets, + maxRaiseFanout: vbaReindexConfig.maxRaiseFanout, + }, ); // Store in database @@ -2675,4 +2687,5 @@ export class ExtractionOrchestrator { // Re-export useful types and functions export { extractFromSource } from './tree-sitter'; +export type { VbaExtractionOptions } from './vba/options'; export { detectLanguage, isSourceFile, isLanguageSupported, isGrammarLoaded, getSupportedLanguages, initGrammars, loadGrammarsForLanguages, loadAllGrammars } from './grammars'; diff --git a/src/extraction/parse-pool.ts b/src/extraction/parse-pool.ts index 302a48b5..a79a61f8 100644 --- a/src/extraction/parse-pool.ts +++ b/src/extraction/parse-pool.ts @@ -29,6 +29,7 @@ import { Worker } from 'worker_threads'; import type { Language, ExtractionResult } from '../types'; +import type { VbaExtractionOptions } from './vba/options'; /** * Minimal worker surface the pool drives — satisfied by a real `worker_threads` @@ -51,9 +52,13 @@ export interface ParseTask { content: string; language: Language; frameworkNames?: string[]; - vbaTargets?: Record; - /** Issue #152: per-file fanout cap for `RaiseEvent` edges (VBA only). */ - maxRaiseFanout?: number; + /** + * Issue #243: every VBA-specific extraction knob in ONE structured-cloneable + * object (previously the separate `vbaTargets` / `maxRaiseFanout` fields). + * It crosses the worker boundary via `postMessage`, so it must stay plain + * data — see the contract in `./vba/options`. + */ + vbaOptions?: VbaExtractionOptions; /** * Issue #154 — gate the 3 Dysflow-specific VBA sub-extractors. `true` * (the default) keeps the pre-refactor behavior; `false` opts out so @@ -351,8 +356,7 @@ export class ParseWorkerPool { content: job.task.content, frameworkNames: job.task.frameworkNames, language: job.task.language, - vbaTargets: job.task.vbaTargets, - maxRaiseFanout: job.task.maxRaiseFanout, + vbaOptions: job.task.vbaOptions, dysflowExport: job.task.dysflowExport, }); } diff --git a/src/extraction/parse-worker.ts b/src/extraction/parse-worker.ts index 41adf14e..147be7c2 100644 --- a/src/extraction/parse-worker.ts +++ b/src/extraction/parse-worker.ts @@ -7,6 +7,7 @@ import { parentPort } from 'worker_threads'; import { extractFromSource } from './tree-sitter'; +import type { VbaExtractionOptions } from './vba/options'; import { detectLanguage, loadGrammarsForLanguages, resetParser } from './grammars'; import type { Language, ExtractionResult } from '../types'; @@ -55,14 +56,14 @@ import type { Language, ExtractionResult } from '../types'; const PARSER_RESET_INTERVAL = 5000; const parseCounts = new Map(); -parentPort!.on('message', async (msg: { type: string; id?: number; filePath?: string; content?: string; languages?: Language[]; frameworkNames?: string[]; language?: Language; vbaTargets?: Record; maxRaiseFanout?: number; dysflowExport?: boolean; grammarBuffers?: Record }) => { +parentPort!.on('message', async (msg: { type: string; id?: number; filePath?: string; content?: string; languages?: Language[]; frameworkNames?: string[]; language?: Language; vbaOptions?: VbaExtractionOptions; dysflowExport?: boolean; grammarBuffers?: Record }) => { if (msg.type === 'load-grammars') { // Grammar WASM bytes pre-read by the main thread (when provided) make this // a memory load instead of a per-spawn disk read — see issue #1231. await loadGrammarsForLanguages(msg.languages!, msg.grammarBuffers); parentPort!.postMessage({ type: 'grammars-loaded' }); } else if (msg.type === 'parse') { - const { id, filePath, content, frameworkNames, vbaTargets, maxRaiseFanout, dysflowExport } = msg; + const { id, filePath, content, frameworkNames, vbaOptions, dysflowExport } = msg; // Worker-side parse clock: reported back with the result so the pool can // tell a genuinely slow parse from a result whose delivery was delayed by // a stalled main thread (issue #1231 false timeouts). @@ -72,11 +73,12 @@ parentPort!.on('message', async (msg: { type: string; id?: number; filePath?: st // codegraph.json extension overrides) and sends it; fall back to detection // for older callers / safety. const language = msg.language ?? detectLanguage(filePath!, content); - // Issue #152: `maxRaiseFanout` threads the per-file `RaiseEvent` - // fanout gate from the project's `codegraph.json` → `vba.maxRaiseFanout` - // through the worker boundary to the VBA extractor. - // Issue #154: also thread the `vba.dysflowExport` opt-out flag. - const result: ExtractionResult = extractFromSource(filePath!, content!, language, frameworkNames, vbaTargets, maxRaiseFanout, dysflowExport); + // Issue #243: `vbaOptions` carries every VBA knob (conditional-compilation + // targets, the issue #152 `RaiseEvent` fanout gate, SQL wrappers) across + // the structuredClone worker boundary in one plain-data object. The + // deprecated `vbaTargets` / `maxRaiseFanout` positionals stay undefined. + // Issue #154: `dysflowExport` remains its own flag. + const result: ExtractionResult = extractFromSource(filePath!, content!, language, frameworkNames, undefined, undefined, dysflowExport, vbaOptions); // Periodic parser reset to reclaim WASM heap memory const count = (parseCounts.get(language) ?? 0) + 1; diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 140ed243..2a53f91b 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -30,6 +30,7 @@ import { DfmExtractor } from './dfm-extractor'; import { VueExtractor } from './vue-extractor'; import { MyBatisExtractor } from './mybatis-extractor'; import { VbaExtractor } from './vba-extractor'; +import type { VbaExtractionOptions } from './vba/options'; import { VbaFormExtractor } from './vba-form-extractor'; import { VbaTestManifestExtractor } from './vba-test-manifest-extractor'; import { VbaTestSequenceExtractor } from './vba-test-sequence-extractor'; @@ -6529,6 +6530,14 @@ export class TreeSitterExtractor { * just a `file` node. Read from `codegraph.json` via * `loadDysflowExportConfig(rootDir)` at the call site and threaded in here * so this function stays project-config-agnostic. + * + * `vbaOptions` (issue #243) is the object form of every VBA-specific knob and + * is what in-repo callers pass. The `vbaTargets` / `maxRaiseFanout` + * positionals are `@deprecated` and kept for one release; when both are + * supplied the OBJECT WINS per field. + * + * @param vbaTargets @deprecated Pass `vbaOptions.targets` instead. + * @param maxRaiseFanout @deprecated Pass `vbaOptions.maxRaiseFanout` instead. */ export function extractFromSource( filePath: string, @@ -6537,8 +6546,17 @@ export function extractFromSource( frameworkNames?: string[], vbaTargets?: Record, maxRaiseFanout?: number, - dysflowExport: boolean = true + dysflowExport: boolean = true, + vbaOptions?: VbaExtractionOptions ): ExtractionResult { + // Issue #243: merge the legacy positionals into the object. Per-field, the + // object wins on conflict; a field absent from BOTH stays undefined so the + // extractor applies its own default (e.g. DEFAULT_MAX_RAISE_FANOUT). + const mergedVbaOptions: VbaExtractionOptions = { + targets: vbaOptions?.targets ?? vbaTargets, + maxRaiseFanout: vbaOptions?.maxRaiseFanout ?? maxRaiseFanout, + sqlWrappers: vbaOptions?.sqlWrappers, + }; const detectedLanguage = language || detectLanguage(filePath, source); const fileExtension = path.extname(filePath).toLowerCase(); @@ -6675,8 +6693,9 @@ export function extractFromSource( // See `vba-code-extraction` spec (REQ-CODE-1..11). // Issue #152: `maxRaiseFanout` is threaded from the project's // `codegraph.json` → `vba.maxRaiseFanout` and gates `raises-event` - // edges for events with high per-file fanout. - const extractor = new VbaExtractor(filePath, source, vbaTargets, maxRaiseFanout); + // edges for events with high per-file fanout. Issue #243: it now rides + // in the merged options object alongside `targets` and `sqlWrappers`. + const extractor = new VbaExtractor(filePath, source, mergedVbaOptions); result = extractor.extract(); } else if (detectedLanguage === 'sql') { // Dysflow-exported saved Access queries — `queries/.sql`. Only diff --git a/src/extraction/vba-extractor.ts b/src/extraction/vba-extractor.ts index 0617271d..5115e4ac 100644 --- a/src/extraction/vba-extractor.ts +++ b/src/extraction/vba-extractor.ts @@ -45,6 +45,7 @@ import { stripVbaComments, } from './vba-preprocess'; import { VbaExtractorContext, VbaClassifier } from './vba/context'; +import type { VbaExtractionOptions } from './vba/options'; import type { VbaExtractionRule } from './vba/rules'; import { RULES as PROCEDURES_RULES, createProceduresClassifier } from './vba/procedures'; import { @@ -181,11 +182,51 @@ export function validateVbaRuleTables( } })(); +/** + * Issue #243 — normalize the `VbaExtractor` constructor's 3rd/4th arguments + * into a single {@link VbaExtractionOptions}. + * + * Two call shapes reach the implementation signature: + * + * - the current form: `new VbaExtractor(p, s, { targets, maxRaiseFanout })` + * - the `@deprecated` legacy form: `new VbaExtractor(p, s, targets, 50)` + * + * The runtime discriminator is the VALUE TYPES, not the key names. A legacy + * conditional-compilation targets map is a `Record`, so every + * one of its values is a boolean. A `VbaExtractionOptions` never has a boolean + * value: `targets` is an object, `maxRaiseFanout` a number, `sqlWrappers` an + * array. So "every own value is a boolean" identifies the legacy form even + * when a project happens to name a `#Const` `targets` or `sqlWrappers`. + * + * The empty object `{}` satisfies "every value is a boolean" vacuously and is + * therefore read as a legacy empty targets map — which is exactly equivalent + * to empty options: an empty targets map and `undefined` both mean "no project + * conditional-compilation overrides" to `preprocessConditionalCompilation`, + * and an absent `maxRaiseFanout` falls back to `DEFAULT_MAX_RAISE_FANOUT` + * either way. The two readings cannot diverge. + * + * `legacyMaxRaiseFanout` only applies to the legacy form; the object form + * cannot be combined with a 4th positional argument (no overload allows it). + */ +function normalizeVbaExtractorArgs( + optionsOrTargets: VbaExtractionOptions | Record, + legacyMaxRaiseFanout?: number, +): VbaExtractionOptions { + const isLegacyTargets = Object.values(optionsOrTargets).every( + (v) => typeof v === 'boolean', + ); + if (!isLegacyTargets) return optionsOrTargets as VbaExtractionOptions; + const targets = optionsOrTargets as Record; + return { + targets: Object.keys(targets).length > 0 ? targets : undefined, + maxRaiseFanout: legacyMaxRaiseFanout, + }; +} + export class VbaExtractor { private filePath: string; private source: string; private ctx: VbaExtractorContext; - private vbaTargets?: Record; /** * Issue #152: per-file fanout cap for `RaiseEvent ` edges. * When a single event is raised more than this many times in one file, @@ -195,17 +236,41 @@ export class VbaExtractor { * the orchestrator: callers that don't pass a value get the default. */ private maxRaiseFanout: number | undefined; + /** + * Issue #243: the full options object, retained so classifiers that grow + * a new knob read it from one place instead of a new private field per + * knob. `targets`/`maxRaiseFanout` keep their dedicated fields because + * they are read on hot paths. + */ + private options: VbaExtractionOptions; + /** + * Issue #243 — the options-object form. Every in-repo call site uses this. + */ + constructor(filePath: string, source: string, options?: VbaExtractionOptions); + /** + * @deprecated Pass a {@link VbaExtractionOptions} object instead: + * `new VbaExtractor(filePath, source, { targets, maxRaiseFanout })`. + * The positional 3rd/4th parameters are kept working for one release so + * out-of-repo callers are not broken by #243; they will be removed after. + */ constructor( filePath: string, source: string, vbaTargets?: Record, - maxRaiseFanout: number = DEFAULT_MAX_RAISE_FANOUT, + maxRaiseFanout?: number, + ); + constructor( + filePath: string, + source: string, + optionsOrTargets: VbaExtractionOptions | Record = {}, + legacyMaxRaiseFanout?: number, ) { + const options = normalizeVbaExtractorArgs(optionsOrTargets, legacyMaxRaiseFanout); this.filePath = filePath; this.source = source; - this.vbaTargets = vbaTargets; - this.maxRaiseFanout = maxRaiseFanout; + this.options = options; + this.maxRaiseFanout = options.maxRaiseFanout ?? DEFAULT_MAX_RAISE_FANOUT; this.ctx = new VbaExtractorContext(filePath); } @@ -257,7 +322,7 @@ export class VbaExtractor { () => preprocessConditionalCompilation( joined, - this.vbaTargets, + this.options.targets, this.ctx.timings, this.ctx.errors, this.filePath, diff --git a/src/extraction/vba/options.ts b/src/extraction/vba/options.ts new file mode 100644 index 00000000..290cf22d --- /dev/null +++ b/src/extraction/vba/options.ts @@ -0,0 +1,72 @@ +/** + * `VbaExtractionOptions` — the single object that carries every VBA + * extraction knob from `codegraph.json` down to `VbaExtractor` (issue #243). + * + * ## Why this module exists + * + * The knobs used to travel as positional parameters through five files: + * + * ``` + * project-config.ts loadVbaConfig() + * -> extraction/index.ts reads vbaConfig + * -> extraction/parse-pool.ts ParseTask fields + * -> extraction/parse-worker.ts message shape + * -> extraction/tree-sitter.ts extractFromSource(..., vbaTargets, maxRaiseFanout, ...) + * -> new VbaExtractor(filePath, source, vbaTargets, maxRaiseFanout) + * ``` + * + * Every new knob meant editing all five signatures and every call site. One + * object collapses that to "add a field here, read it in the extractor". + * + * ## Worker-boundary constraint — READ BEFORE ADDING A FIELD + * + * This object crosses the `parse-pool` -> `parse-worker` boundary through + * `worker.postMessage`, which is `structuredClone`-based. Every field MUST be + * plain, structured-cloneable data: primitives, plain objects, and arrays of + * those. NO functions, NO `RegExp`, NO `Map`/`Set`, NO class instances — a + * function throws `DataCloneError` at the boundary and a `RegExp` silently + * loses its `lastIndex`/identity semantics. + * + * A derived value that is not cloneable (a compiled wrapper `RegExp`, a + * lookup `Map`) belongs on the extractor context, built inside the extractor + * from the plain data in this object. + * + * This module is a LEAF: it imports nothing, so the config layer, the pool, + * the worker, and the extractor can all depend on it without a cycle. + */ + +/** + * Every VBA-specific extraction knob, in one structured-cloneable object. + * + * All fields are optional; `{}` is the zero-config default and must behave + * exactly like the pre-#243 "caller passed nothing" path. + */ +export interface VbaExtractionOptions { + /** + * Conditional-compilation targets — the `#Const` name -> truth map used to + * decide which `#If` branches stay active. Sourced from `codegraph.json` -> + * `vba.targets`. Undefined means "no project overrides"; the preprocessor + * falls back to its built-in defaults. + */ + targets?: Record; + /** + * Issue #152: per-file fanout cap for `RaiseEvent ` edges. An + * event raised from more than this many sites in one file is flagged + * `metadata.highFanout: true` and ALL its `raises-event` edges are dropped. + * Undefined means the caller did not choose, so `VbaExtractor` applies + * `DEFAULT_MAX_RAISE_FANOUT` (50). Sourced from `codegraph.json` -> + * `vba.maxRaiseFanout`. + */ + maxRaiseFanout?: number; + /** + * Names of project-specific procedures that wrap a SQL execution call + * (e.g. a shared `EjecutarSQL(sql)` helper), so a call to one of them is + * treated as a SQL execution site instead of an ordinary call. + * + * Declared here now and threaded end-to-end by #243; the SQL-wrapper task + * is the consumer. Kept as a `readonly string[]` of plain strings — the + * compiled matcher built from these names is NOT cloneable and therefore + * belongs on the extractor context, never in this object. + */ + sqlWrappers?: readonly string[]; +}