diff --git a/CHANGELOG.md b/CHANGELOG.md index 95aec5b0..99e7e259 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes - VBA's `unresolved_refs` table now reports the syntactic shape of each unresolved reference — call sites, form-property reads and writes, `DoCmd.OpenQuery` arguments, and bare identifiers each get their own row kind — so a SQL filter for "missing callee" stops drowning in DAO-field and form-control noise. The legacy `references` kind is preserved for any path the round does not reclassify, so SQL filters that key on it keep working. (#108) +- VBA's post-extraction call-stub resolver now correctly declines runtime-object call stubs (`DAO.*`, `fso.*`, `ListBox.AddItem`, `Collection.Add`, `err.*`, `VBA.*`, …) instead of pointing them at themselves; a user class or module that happens to share a runtime-object name is still linked. Stubs now carry a `repointDecision` field on their edge metadata so consumers can tell a runtime-object decline apart from a genuinely-missing callee. (#110, supersedes #109) ## [1.6.2] - 2026-07-12 diff --git a/__tests__/extraction-vba-stub-resolver.test.ts b/__tests__/extraction-vba-stub-resolver.test.ts new file mode 100644 index 00000000..f8f733c9 --- /dev/null +++ b/__tests__/extraction-vba-stub-resolver.test.ts @@ -0,0 +1,249 @@ +/** + * Strict-TDD unit/E2E coverage for the runtime-object skip in the VBA + * post-extraction call-stub resolver (issue #110, supersedes #109). + * + * Background: `codegraph-vba` synthesizes a `stub:true` `calls` edge for every + * `Receiver.Member` call whose target isn't resolvable at extraction time. For + * runtime objects (DAO, FileSystemObject, intrinsic collections, ...) that + * target is NEVER user code, so the stub used to sit in the graph pointing at + * itself — poisoning a consumer's `WHERE stub=true` "missing callee" lint with + * runtime-object noise. The resolver now DECLINES those stubs explicitly + * (`repointDecision='declined-runtime'`) while preserving: + * - class-typed and `.bas`-qualified repoints (Tests 2, 3), + * - shadow user classes that happen to share a runtime-object name (Test 4), + * - genuinely-missing user callees as `stub:true` (Test 5). + * + * Each test builds its OWN isolated temp project (fixture gate) and drives the + * real `CodeGraph.indexAll()` pipeline end-to-end — no DB mocking. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { CodeGraph } from '../src'; +import { isRuntimeObject, RUNTIME_OBJECTS } from '../src/resolution/vba-runtime-objects'; +import type { Edge } from '../src/types'; + +const CLS_HEADER = ['VERSION 1.0 CLASS', 'BEGIN', " MultiUse = -1 'True", 'END']; + +/** Track every project we spin up so afterEach can close + remove them. */ +const openProjects: Array<{ cg: CodeGraph; dir: string }> = []; + +afterEach(async () => { + while (openProjects.length > 0) { + const { cg, dir } = openProjects.pop()!; + try { + await cg.close(); + } catch { + // ignore close errors + } + if (fs.existsSync(dir)) fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +/** + * Write `files` (relative path → source) into a fresh temp dir, index it, and + * return the live CodeGraph. `.bas` go under src/modules, `.cls` under + * src/classes by convention — but the caller supplies the full relative path. + */ +async function buildProject(files: Record): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vba-runtime-skip-')); + for (const [rel, src] of Object.entries(files)) { + const full = path.join(dir, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, src); + } + const cg = await CodeGraph.init(dir, { index: false }); + openProjects.push({ cg, dir }); + await cg.indexAll(); + return cg; +} + +/** Outgoing `calls` edges of the (single) function named `name`. */ +function callEdgesFrom(cg: CodeGraph, name: string): Edge[] { + const fn = cg + .searchNodes(name, { languages: ['vba'], kinds: ['function'] }) + .find((n) => n.node.name === name); + if (!fn) return []; + return cg.getOutgoingEdges(fn.node.id).filter((e) => e.kind === 'calls'); +} + +describe('VBA call-stub resolver — runtime-object skip (#110)', () => { + it('Test 1: a runtime-object call (DAO.*) stays stub:true and is declined as declined-runtime', async () => { + const cg = await buildProject({ + 'src/modules/Caller.bas': [ + 'Attribute VB_Name = "Caller"', + 'Option Explicit', + '', + 'Public Sub CallerSub()', + ' DAO.BeginTrans', + 'End Sub', + '', + ].join('\n'), + }); + + const edges = callEdgesFrom(cg, 'CallerSub'); + const daoEdge = edges.find((e) => e.metadata?.member === 'BeginTrans'); + expect(daoEdge).toBeDefined(); + expect(daoEdge?.metadata?.receiverType).toBe('DAO'); + expect(daoEdge?.metadata?.stub).toBe(true); + expect(daoEdge?.metadata?.repointDecision).toBe('declined-runtime'); + + // The synthetic stub never resolves to a REAL node — there is no user + // function whose qualifiedName is 'DAO.BeginTrans'. + const real = cg + .searchNodes('DAO.BeginTrans', { languages: ['vba'] }) + .filter((n) => n.node.name === 'DAO.BeginTrans'); + // Only the synthetic stub (if still present) may carry this name; there is + // no additional real declaration. + expect(real.every((n) => n.node.metadata?.stub === true || n.node.name === 'DAO.BeginTrans')).toBe(true); + }); + + it('Test 2: a class-typed call resolves to the real cross-file method (stub:false)', async () => { + const cg = await buildProject({ + 'src/classes/ACAuditoriaOperaciones.cls': [ + ...CLS_HEADER, + 'Attribute VB_Name = "ACAuditoriaOperaciones"', + 'Option Explicit', + '', + 'Public Sub Registrar()', + 'End Sub', + '', + ].join('\n'), + 'src/modules/Caller.bas': [ + 'Attribute VB_Name = "Caller"', + 'Option Explicit', + '', + 'Public Sub CallerSub()', + ' Dim x As ACAuditoriaOperaciones', + ' x.Registrar', + 'End Sub', + '', + ].join('\n'), + }); + + const registrar = cg + .searchNodes('Registrar', { languages: ['vba'], kinds: ['function'] }) + .find((n) => n.node.name === 'Registrar' && n.node.filePath.endsWith('ACAuditoriaOperaciones.cls')); + expect(registrar).toBeDefined(); + if (!registrar) return; + + const incoming = cg.getIncomingEdges(registrar.node.id).filter((e) => e.kind === 'calls'); + expect(incoming.length).toBeGreaterThan(0); + for (const edge of incoming) { + expect(edge.metadata?.stub).not.toBe(true); + } + }); + + it('Test 3: a .bas-qualified call resolves to the real bare-name node via module narrowing (stub:false)', async () => { + const cg = await buildProject({ + 'src/modules/mdlCursor.bas': [ + 'Attribute VB_Name = "mdlCursor"', + 'Option Explicit', + '', + 'Public Function MouseCursor() As Long', + 'End Function', + '', + ].join('\n'), + 'src/modules/Caller.bas': [ + 'Attribute VB_Name = "Caller"', + 'Option Explicit', + '', + 'Public Sub CallerSub()', + ' mdlCursor.MouseCursor', + 'End Sub', + '', + ].join('\n'), + }); + + const mouseCursor = cg + .searchNodes('MouseCursor', { languages: ['vba'], kinds: ['function'] }) + .find((n) => n.node.name === 'MouseCursor' && n.node.filePath.endsWith('mdlCursor.bas')); + expect(mouseCursor).toBeDefined(); + if (!mouseCursor) return; + + const incoming = cg.getIncomingEdges(mouseCursor.node.id).filter((e) => e.kind === 'calls'); + expect(incoming.length).toBeGreaterThan(0); + for (const edge of incoming) { + expect(edge.metadata?.stub).not.toBe(true); + } + }); + + it('Test 4: a shadow user class named DAO is preserved (repointed-to-real, skip bypassed)', async () => { + const cg = await buildProject({ + 'src/classes/DAO.cls': [ + ...CLS_HEADER, + 'Attribute VB_Name = "DAO"', + 'Option Explicit', + '', + 'Public Sub Execute()', + 'End Sub', + '', + ].join('\n'), + 'src/modules/Caller.bas': [ + 'Attribute VB_Name = "Caller"', + 'Option Explicit', + '', + 'Public Sub CallerSub()', + ' DAO.Execute', + 'End Sub', + '', + ].join('\n'), + }); + + const execute = cg + .searchNodes('Execute', { languages: ['vba'], kinds: ['function'] }) + .find((n) => n.node.name === 'Execute' && n.node.filePath.endsWith('DAO.cls')); + expect(execute).toBeDefined(); + if (!execute) return; + + const incoming = cg.getIncomingEdges(execute.node.id).filter((e) => e.kind === 'calls'); + expect(incoming.length).toBeGreaterThan(0); + for (const edge of incoming) { + expect(edge.metadata?.stub).not.toBe(true); + expect(edge.metadata?.repointDecision).toBe('reponted-to-real'); + } + }); + + it('Test 5: a genuinely-missing user callee stays stub:true and is declined as declined-not-found', async () => { + const cg = await buildProject({ + 'src/modules/Caller.bas': [ + 'Attribute VB_Name = "Caller"', + 'Option Explicit', + '', + 'Public Sub CallerSub()', + ' Dim m_x As DoesNotExistClass', + ' m_x.DoesNotExistSub', + 'End Sub', + '', + ].join('\n'), + }); + + const edges = callEdgesFrom(cg, 'CallerSub'); + const missing = edges.filter((e) => e.metadata?.member === 'DoesNotExistSub'); + // Exactly ONE calls edge for the missing callee (no double-emission). + expect(missing).toHaveLength(1); + const edge = missing[0]; + expect(edge?.metadata?.receiverType).toBe('DoesNotExistClass'); + expect(edge?.metadata?.stub).toBe(true); + expect(edge?.metadata?.repointDecision).toBe('declined-not-found'); + }); + + it('Test 6 (meta): the canonical runtime-object list classifies receivers case-insensitively', () => { + // Runtime objects (any case) → true. + expect(isRuntimeObject('DAO')).toBe(true); + expect(isRuntimeObject('dao')).toBe(true); + expect(isRuntimeObject('Fso')).toBe(true); + expect(isRuntimeObject('[DAO]')).toBe(true); + expect(isRuntimeObject(' Collection ')).toBe(true); + // Non-runtime user receivers → false. + expect(isRuntimeObject('ACAuditoriaOperaciones')).toBe(false); + expect(isRuntimeObject('mdlCursor')).toBe(false); + expect(isRuntimeObject('')).toBe(false); + expect(isRuntimeObject(undefined)).toBe(false); + // The frozen list carries the documented seed entries. + for (const expected of ['dao', 'fso', 'err', 'listbox', 'collection', 'docmd']) { + expect(RUNTIME_OBJECTS.has(expected)).toBe(true); + } + }); +}); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index c66b47e1..96b6bb17 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -27,6 +27,30 @@ import { loadWorkspacePackages, type WorkspacePackages } from './workspace-packa import { logDebug } from '../errors'; import type { ReExport } from './types'; import { LRUCache } from './lru-cache'; +import { isRuntimeObject } from './vba-runtime-objects'; + +/** + * Outcome of resolving a single VBA call-stub (issue #110). Recorded on the + * stub's incoming `calls` edge as `metadata.repointDecision` for + * observability so a consumer can tell a runtime-object decline apart from a + * genuinely-missing callee: + * - `reponted-to-real` — a real user target was found; edge repointed (stub:false). + * - `declined-runtime` — receiver is a runtime object with no user shadow; kept stub:true. + * - `declined-ambiguous` — 2+ real candidates; can't safely pick one; kept stub:true. + * - `declined-not-found` — no real target and not a runtime object (genuine miss); kept stub:true. + * (The `reponted-to-real` spelling is the wire value consumers already key on.) + */ +export type RepointDecision = + | 'reponted-to-real' + | 'declined-runtime' + | 'declined-ambiguous' + | 'declined-not-found'; + +/** Resolver verdict for one VBA call-stub: the decision plus the real target when repointing. */ +interface StubResolution { + decision: RepointDecision; + target?: Node; +} /** Node kinds that can declare supertypes (extends/implements). */ const SUPERTYPE_BEARING_KINDS = new Set([ @@ -1885,14 +1909,25 @@ export class ReferenceResolver { let repointedCount = 0; for (const stub of stubs) { - const target = this.resolveVbaCallStubTarget(stub, stubIds); + const resolution = this.resolveVbaCallStubTarget(stub, stubIds); const incoming = this.queries.getIncomingEdges(stub.id, ['calls']); - if (!target) { - // Ambiguous or unmatched — leave the stub and its edges untouched. + if (resolution.decision !== 'reponted-to-real' || !resolution.target) { + // #110: declined — stamp WHY on each incoming edge for observability + // (runtime-object vs ambiguous vs genuine miss) and keep the edge + // `stub:true` pointing at the synthetic node. Re-pointing the edge to + // its EXISTING target is a metadata-only update. The stub node stays + // in the graph as the (still-unresolved) call target; a consumer's + // `stub=true` lint now sees a clean, classified signal. + for (const edge of incoming) { + if (edge.id === undefined) continue; + const meta = { ...(edge.metadata ?? {}), repointDecision: resolution.decision }; + this.queries.repointEdgeTarget(edge.id, edge.target, JSON.stringify(meta)); + } continue; } + const target = resolution.target; for (const edge of incoming) { if (edge.id === undefined) continue; // defensive — DB reads always set it const tupleKey = `${edge.source}\0${target.id}`; @@ -1906,8 +1941,8 @@ export class ReferenceResolver { seenTuples.add(tupleKey); // F5: clear the stub flag but KEEP synthesizedBy/receiverType/member // — the edge is still a heuristic VBA-name-resolution edge, just no - // longer a dead end. - const meta = { ...(edge.metadata ?? {}), stub: false }; + // longer a dead end. #110: record the repoint decision too. + const meta = { ...(edge.metadata ?? {}), stub: false, repointDecision: 'reponted-to-real' }; this.queries.repointEdgeTarget(edge.id, target.id, JSON.stringify(meta)); repointedCount++; } @@ -1996,32 +2031,42 @@ export class ReferenceResolver { * strategy (exact qualifiedName match, then `.bas` module-scoped * fallback). */ - private resolveVbaCallStubTarget(stub: Node, stubIds: Set): Node | null { + private resolveVbaCallStubTarget(stub: Node, stubIds: Set): StubResolution { const isRealCandidate = (n: Node) => n.id !== stub.id && !stubIds.has(n.id) && n.kind === 'function' && n.language === 'vba'; + // `stub.qualifiedName` is `${receiver}.${member}`. The receiver drives the + // runtime-object classification when no real target is found (#110). + const dot = stub.qualifiedName.indexOf('.'); + const receiver = dot > 0 ? stub.qualifiedName.slice(0, dot) : stub.qualifiedName; + + // #110: the two-step name resolution runs FIRST. A runtime-object receiver + // that ALSO has a real user declaration (a "shadow" class/module named, + // e.g., `DAO`) is repointed here like any other real symbol (FR-2.1) — so + // the runtime-object skip below only fires when NO real target exists, + // which is exactly the noise the consumer's `stub=true` lint wants gone. + const decline = (): StubResolution => ({ + decision: isRuntimeObject(receiver) ? 'declined-runtime' : 'declined-not-found', + }); + // Step 1: exact qualifiedName match (class-typed stubs land here — see // #12a's resolved-type rename). const exact = this.queries .getNodesByQualifiedNameExact(stub.qualifiedName) .filter(isRealCandidate); - if (exact.length === 1) return exact[0]!; - if (exact.length >= 2) return null; + if (exact.length === 1) return { decision: 'reponted-to-real', target: exact[0]! }; + if (exact.length >= 2) return { decision: 'declined-ambiguous' }; - // Step 2: `.bas`-qualified fallback. `stub.qualifiedName` is - // `${receiver}.${member}` where `receiver` didn't resolve to a + // Step 2: `.bas`-qualified fallback. The `receiver` didn't resolve to a // project-class type at extraction time (kept as raw receiver text — // the `.bas`-qualified-call case, since the extractor's - // `resolveReceiverType` only substitutes for DECLARED local class - // vars). - const dot = stub.qualifiedName.indexOf('.'); - if (dot <= 0) return null; - const receiver = stub.qualifiedName.slice(0, dot); + // `resolveReceiverType` only substitutes for DECLARED local class vars). + if (dot <= 0) return decline(); const member = stub.qualifiedName.slice(dot + 1); - if (!member) return null; + if (!member) return decline(); // Bare-name candidates in ANY `.bas` file — real `.bas` function // qualifiedNames carry no module prefix (unlike `.cls` methods), so a @@ -2032,7 +2077,7 @@ export class ReferenceResolver { (n) => isRealCandidate(n) && n.filePath.toLowerCase().endsWith('.bas'), ); - if (memberCandidates.length === 0) return null; + if (memberCandidates.length === 0) return decline(); // Narrow to the `.bas` file(s) whose module identity (VB_Name, i.e. the // sibling `module` node's name) equals `receiver`, case-insensitive — @@ -2044,11 +2089,12 @@ export class ReferenceResolver { .filter((n) => n.kind === 'module' && n.language === 'vba') .map((n) => n.filePath), ); - if (moduleFiles.size === 0) return null; + if (moduleFiles.size === 0) return decline(); const narrowed = memberCandidates.filter((n) => moduleFiles.has(n.filePath)); - if (narrowed.length === 1) return narrowed[0]!; - return null; // 0 or 2+ after narrowing — decline + if (narrowed.length === 1) return { decision: 'reponted-to-real', target: narrowed[0]! }; + if (narrowed.length >= 2) return { decision: 'declined-ambiguous' }; + return decline(); // 0 after narrowing — runtime-object or genuine miss } private gateLanguage(result: ResolvedRef | null, ref: UnresolvedRef): ResolvedRef | null { diff --git a/src/resolution/vba-runtime-objects.ts b/src/resolution/vba-runtime-objects.ts new file mode 100644 index 00000000..40f11763 --- /dev/null +++ b/src/resolution/vba-runtime-objects.ts @@ -0,0 +1,63 @@ +/** + * Canonical list of VBA/Access runtime objects and singletons whose + * `Receiver.Member` calls are NEVER user-defined code — DAO, FileSystemObject + * (`fso`), intrinsic collections, error/debug intrinsics, and Access + * application singletons. + * + * Consumed by the post-extraction call-stub resolver + * (`ReferenceResolver.resolveVbaCallStubTarget`, see `./index.ts`) to DECLINE + * repointing a synthetic `calls` stub whose receiver is a runtime object and + * has no real user declaration in the project. Declining keeps the edge + * `stub:true`, so a consumer's `WHERE stub=true` guardrail returns GENUINE + * missing callees only, free of runtime-object noise (issue #110, supersedes + * #109). + * + * A user class or module that happens to share a runtime-object name (a + * "shadow" declaration, e.g. a user `.cls` literally named `DAO`) is still + * linked: the resolver runs its normal two-step name resolution FIRST and + * only falls back to this list when no real target exists, so a shadow + * declaration is repointed exactly like any other real symbol (FR-2.1). + * + * NOTE — this is complementary to, NOT a duplicate of, the VBA extractor's + * own `RUNTIME_RECEIVER_BLACKLIST` (`src/extraction/vba/constants.ts`). That + * set is case-sensitive PascalCase and suppresses stub SYNTHESIS for the + * common receivers at extraction time; this set is lowercased and catches the + * runtime-object stubs that still reached the graph (lowercase receivers, + * `Dim x As DAO.*` typed locals whose receiver survives as raw text, etc.). + * The extractor blacklist deliberately stays untouched (AC-4: `src/extraction` + * has 0 diff); the two layers are independent by design. + * + * Entries are lowercased so matching is case-insensitive against a stub's + * receiver. + */ +export const RUNTIME_OBJECTS: ReadonlySet = new Set([ + 'dao', + 'fso', + 'err', + 'listbox', + 'combobox', + 'textbox', + 'forms', + 'reports', + 'debug', + 'collection', + 'vba', + 'application', + 'screen', + 'docmd', + 'currentdb', + 'currentproject', + 'codedata', + 'codeproject', +]); + +/** + * True iff `receiver` (any case) names a known VBA/Access runtime object. + * Leading/trailing brackets and surrounding whitespace are stripped + * defensively so a bracketed receiver (`[DAO]`) still matches. + */ +export function isRuntimeObject(receiver: string | null | undefined): boolean { + if (!receiver) return false; + const key = receiver.replace(/^\[/, '').replace(/\]$/, '').trim().toLowerCase(); + return RUNTIME_OBJECTS.has(key); +}