Skip to content

Commit 39dcf57

Browse files
committed
feat(vba): connect form and report lifecycle events to their layout node
A form's own lifecycle handlers (Form_Open, Form_Load, Form_Unload, ...) produced no edge at all. The control-handler path deliberately refuses them: a form-level event fires on the form object, not on a control, so routing it there would synthesize a bogus form-instance-control node literally named "Form". Keep that refusal and give the form-level case its own target. When the owner segment is exactly `Form` or `Report` and the suffix is a known Access event name, emit an `event-handler` edge to the sibling form-layout / report-layout node, carrying metadata.scope: 'form' so a consumer can separate the two populations without re-parsing the Sub name. The local stub uses the same deterministic id VbaFormExtractor produces for that file, so INSERT OR REPLACE converges whichever file is indexed first. The gate is isAccessEventName, not the `Form_` prefix: `Form_Load` is an event, a class method called `Form_Helper` is not. On the EXPEDIENTES + GESTION_RIESGOS corpora: event-handler edges 695 -> 808 (+113 form-level), form-instance-control nodes unchanged at 2,485. Two existing tests move with the behaviour: the event whitelist now expects the third (form-level) edge while pinning the control-node count at two, and the hueco-5 search test accepts the `event-handler` kind searchNodes reports for any function with an outgoing event-handler edge. The DoCmd stub-resolution fixture now places Form_FormNC.cls beside its own .form.txt, which is how a Dysflow export always lays them out and what the extractor's sibling-path derivation has always assumed. Closes #247
1 parent 32aa264 commit 39dcf57

11 files changed

Lines changed: 585 additions & 46 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99

1010
## [Unreleased]
1111

12+
### New Features
13+
14+
- A form's own lifecycle handlers — what runs when it opens, loads or closes — now connect to the form in the graph, the same way a button's click handler already did. (#247)
15+
1216
### Changed
1317

1418
- Project-scoped CodeGraph resources can now be released non-interactively through the CLI or the opt-in MCP tool before deleting a worktree, without stopping unrelated projects. (#234)

‎__tests__/extraction-vba-control-modeling.test.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,16 @@ describe('huecos 3 & 5: VBA event-handler and Form_Load integration', () => {
308308
if (!cg) return;
309309
const hits = cg.searchNodes('Form_Load', { languages: ['vba'] });
310310

311-
// Filter to function-kind hits (skip the file node and any incidental
312-
// text matches in identifier bodies).
313-
const fnHits = hits.filter((h) => h.node.kind === 'function');
311+
// Filter to procedure-kind hits (skip the file node and any incidental
312+
// text matches in identifier bodies). `searchNodes` reports a function
313+
// that has an outgoing `event-handler` edge as kind `event-handler`
314+
// rather than `function`; since issue #247 wired form-level lifecycle
315+
// handlers to the sibling layout node, `Form_Load` is one of those, so
316+
// both kinds have to be accepted here. The assertion below — the
317+
// qualifiedName carries the owning form's prefix — is unchanged.
318+
const fnHits = hits.filter(
319+
(h) => h.node.kind === 'function' || h.node.kind === 'event-handler',
320+
);
314321
expect(fnHits.length).toBeGreaterThan(0);
315322

316323
// Every Form_Load hit must be qualified with its owning form:

‎__tests__/extraction-vba-event-whitelist.test.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,15 @@ End Sub`;
6868
source,
6969
).extract();
7070

71-
expect(result.edges.filter((edge) => edge.kind === 'event-handler'))
72-
.toHaveLength(2);
71+
// THREE handler edges: the two control handlers, plus `Form_Load`.
72+
// Since issue #247 a form-level lifecycle handler is wired to the
73+
// sibling form-layout node instead of being dropped. It is still not a
74+
// control handler, which is what the control-node count below pins:
75+
// only MotivoBorrado and ComandoGrabar get a `form-instance-control`.
76+
const eventEdges = result.edges.filter((edge) => edge.kind === 'event-handler');
77+
expect(eventEdges).toHaveLength(3);
78+
expect(eventEdges.filter((edge) => edge.metadata?.scope === 'form'))
79+
.toHaveLength(1);
7380
expect(result.nodes.filter((node) => node.kind === 'form-instance-control'))
7481
.toHaveLength(2);
7582
});
Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
/**
2+
* extraction-vba-form-lifecycle-events.test.ts
3+
*
4+
* Acceptance tests for issue #247 — a form's (or report's) OWN lifecycle
5+
* handlers must produce an `event-handler` edge to the sibling
6+
* `form-layout` / `report-layout` node.
7+
*
8+
* Before #247 those handlers produced nothing: `parseEventHandlerName`
9+
* refuses `Form_*` on purpose, because routing a form-level event through
10+
* the control path would synthesize a `form-instance-control` node
11+
* literally named `Form`. The fix keeps that refusal and gives the
12+
* form-level case its own target instead.
13+
*
14+
* The two invariants that make this safe are asserted directly:
15+
* 1. The stub the code-behind emits carries the SAME deterministic id
16+
* `VbaFormExtractor` produces for the real layout node, so the
17+
* `INSERT OR REPLACE` convergence works whichever file is indexed
18+
* first. We do not hardcode the id — we run the real form extractor
19+
* on the real sibling file and compare.
20+
* 2. NO extra `form-instance-control` node appears. A bogus `Form`
21+
* control node is exactly the failure mode this change had to avoid.
22+
*
23+
* Fixture layout (under __tests__/fixtures/vba-form-lifecycle/):
24+
* Form_Lifecycle.cls — Form_Open / Form_Load / Form_Unload (events),
25+
* Form_Helper (NOT an event), cmdSave_Click
26+
* (a control handler, must be unchanged).
27+
* Form_Lifecycle.form.txt — UI with cmdSave + lblTitulo.
28+
* Report_Lifecycle.cls — Report_Open (event), Report_Helper (not an
29+
* event), txtTotal_Click (control handler).
30+
* Report_Lifecycle.report.txt — UI with txtTotal.
31+
*
32+
* Real files, real extractors, no mocking.
33+
*/
34+
import { describe, it, expect } from 'vitest';
35+
import * as fs from 'fs';
36+
import * as path from 'path';
37+
import { VbaExtractor } from '../src/extraction/vba-extractor';
38+
import { VbaFormExtractor } from '../src/extraction/vba-form-extractor';
39+
import type { Edge, ExtractionResult, Node } from '../src/types';
40+
41+
const FIXTURE_DIR = path.join(__dirname, 'fixtures', 'vba-form-lifecycle');
42+
const FORM_CLS = path.join(FIXTURE_DIR, 'Form_Lifecycle.cls');
43+
const FORM_TXT = path.join(FIXTURE_DIR, 'Form_Lifecycle.form.txt');
44+
const REPORT_CLS = path.join(FIXTURE_DIR, 'Report_Lifecycle.cls');
45+
const REPORT_TXT = path.join(FIXTURE_DIR, 'Report_Lifecycle.report.txt');
46+
47+
function readFixture(p: string): string {
48+
return fs.readFileSync(p, 'utf8');
49+
}
50+
51+
function extractCode(p: string): ExtractionResult {
52+
return new VbaExtractor(p, readFixture(p)).extract();
53+
}
54+
55+
function extractLayout(p: string): ExtractionResult {
56+
return new VbaFormExtractor(p, readFixture(p)).extract();
57+
}
58+
59+
/** The `function` node for a Sub declared in the given extract. */
60+
function fn(result: ExtractionResult, name: string): Node | undefined {
61+
return result.nodes.find((n) => n.kind === 'function' && n.name === name);
62+
}
63+
64+
/** Every `event-handler` edge leaving the named Sub. */
65+
function handlerEdges(result: ExtractionResult, subName: string): Edge[] {
66+
const sub = fn(result, subName);
67+
if (!sub) return [];
68+
return result.edges.filter(
69+
(e) => e.kind === 'event-handler' && e.source === sub.id,
70+
);
71+
}
72+
73+
const formCode = extractCode(FORM_CLS);
74+
const formLayout = extractLayout(FORM_TXT);
75+
const reportCode = extractCode(REPORT_CLS);
76+
const reportLayout = extractLayout(REPORT_TXT);
77+
78+
// =============================================================================
79+
// AC #1 — `Private Sub Form_Load()` in `Form_X.cls` → `event-handler` edge to
80+
// the `Form_X` layout node, `eventName: 'Load'`, `scope: 'form'`.
81+
// =============================================================================
82+
describe('issue-247 AC#1: form-level handlers bind to the sibling form-layout node', () => {
83+
const realLayout = formLayout.nodes.find((n) => n.kind === 'form-layout');
84+
85+
it('the sibling .form.txt really does emit exactly one form-layout node', () => {
86+
// Guards the fixture itself — every id comparison below is against
87+
// this node, so an empty or duplicated layout extract would make the
88+
// rest of the suite vacuous.
89+
const layouts = formLayout.nodes.filter((n) => n.kind === 'form-layout');
90+
expect(layouts).toHaveLength(1);
91+
});
92+
93+
it.each([
94+
['Form_Load', 'Load'],
95+
['Form_Open', 'Open'],
96+
['Form_Unload', 'Unload'],
97+
])(
98+
'%s emits one event-handler edge with eventName=%s and scope=form',
99+
(subName, eventName) => {
100+
const edges = handlerEdges(formCode, subName);
101+
expect(
102+
edges,
103+
`expected exactly one event-handler edge from ${subName}`,
104+
).toHaveLength(1);
105+
const edge = edges[0]!;
106+
expect(edge.metadata?.eventName).toBe(eventName);
107+
expect(edge.metadata?.scope).toBe('form');
108+
expect(edge.provenance).toBe('heuristic');
109+
// The target id must be the one the REAL form extractor produces for
110+
// the sibling file — that identity is what makes INSERT OR REPLACE
111+
// converge on a single node instead of leaving an orphan stub.
112+
expect(edge.target).toBe(realLayout?.id);
113+
},
114+
);
115+
116+
it('the local stub the code-behind emits matches the real layout node id and kind', () => {
117+
const stub = formCode.nodes.find((n) => n.kind === 'form-layout');
118+
expect(stub, 'expected a local form-layout stub in the .cls extract').toBeDefined();
119+
expect(stub?.id).toBe(realLayout?.id);
120+
expect(stub?.name).toBe(realLayout?.name);
121+
expect(stub?.filePath).toBe(FORM_TXT);
122+
// Exactly one distinct stub id, however many form-level handlers the
123+
// class declares — three handlers must not produce three different ids.
124+
const stubs = formCode.nodes.filter((n) => n.kind === 'form-layout');
125+
expect(new Set(stubs.map((n) => n.id)).size).toBe(1);
126+
});
127+
});
128+
129+
// =============================================================================
130+
// AC #2 — `Private Sub Report_Open()` in `Report_Y.cls` → the report layout
131+
// node (kind `report-layout`, sibling `.report.txt`).
132+
// =============================================================================
133+
describe('issue-247 AC#2: report-level handlers bind to the sibling report-layout node', () => {
134+
it('Report_Open targets the real report-layout node', () => {
135+
const realLayout = reportLayout.nodes.find((n) => n.kind === 'report-layout');
136+
expect(realLayout, 'expected a report-layout node from the .report.txt').toBeDefined();
137+
138+
const edges = handlerEdges(reportCode, 'Report_Open');
139+
expect(edges).toHaveLength(1);
140+
const edge = edges[0]!;
141+
expect(edge.target).toBe(realLayout?.id);
142+
expect(edge.metadata?.eventName).toBe('Open');
143+
expect(edge.metadata?.scope).toBe('form');
144+
});
145+
146+
it('the report stub is a report-layout, never a form-layout', () => {
147+
expect(reportCode.nodes.filter((n) => n.kind === 'form-layout')).toHaveLength(0);
148+
expect(reportCode.nodes.filter((n) => n.kind === 'report-layout')).toHaveLength(1);
149+
});
150+
});
151+
152+
// =============================================================================
153+
// AC #3 — `Private Sub Form_Helper()` → NO edge. The gate is
154+
// `isAccessEventName`, not the `Form_` prefix.
155+
// =============================================================================
156+
describe('issue-247 AC#3: a Form_-prefixed method that is not an Access event gets no edge', () => {
157+
it('Form_Helper emits no event-handler edge', () => {
158+
expect(fn(formCode, 'Form_Helper'), 'fixture must declare Form_Helper').toBeDefined();
159+
expect(handlerEdges(formCode, 'Form_Helper')).toHaveLength(0);
160+
});
161+
162+
it('Report_Helper emits no event-handler edge', () => {
163+
expect(fn(reportCode, 'Report_Helper')).toBeDefined();
164+
expect(handlerEdges(reportCode, 'Report_Helper')).toHaveLength(0);
165+
});
166+
});
167+
168+
// =============================================================================
169+
// AC #4 — a control handler is completely unchanged: same node id, same edge.
170+
// =============================================================================
171+
describe('issue-247 AC#4: control handlers are untouched', () => {
172+
it('cmdSave_Click still targets the real form-instance-control node id', () => {
173+
const realControl = formLayout.nodes.find(
174+
(n) => n.kind === 'form-instance-control' && n.name === 'cmdSave',
175+
);
176+
expect(realControl, 'expected a cmdSave control node from the .form.txt').toBeDefined();
177+
178+
const edges = handlerEdges(formCode, 'cmdSave_Click');
179+
expect(edges).toHaveLength(1);
180+
const edge = edges[0]!;
181+
expect(edge.target).toBe(realControl?.id);
182+
expect(edge.metadata?.eventName).toBe('Click');
183+
// A control handler carries NO scope — that field is what lets a
184+
// consumer separate the two populations.
185+
expect(edge.metadata?.scope).toBeUndefined();
186+
});
187+
188+
it('txtTotal_Click on the report side is likewise unchanged', () => {
189+
const realControl = reportLayout.nodes.find(
190+
(n) => n.kind === 'form-instance-control' && n.name === 'txtTotal',
191+
);
192+
const edges = handlerEdges(reportCode, 'txtTotal_Click');
193+
expect(edges).toHaveLength(1);
194+
expect(edges[0]?.target).toBe(realControl?.id);
195+
expect(edges[0]?.metadata?.scope).toBeUndefined();
196+
});
197+
});
198+
199+
// =============================================================================
200+
// AC #5 — `form-instance-control` node count must not move. A bogus control
201+
// node named `Form` (or `Report`) is the exact failure this change avoids.
202+
// =============================================================================
203+
describe('issue-247 AC#5: no bogus Form/Report control node is created', () => {
204+
it('the form code-behind emits one control stub — cmdSave, and nothing else', () => {
205+
const controls = formCode.nodes.filter((n) => n.kind === 'form-instance-control');
206+
expect(controls.map((n) => n.name)).toEqual(['cmdSave']);
207+
});
208+
209+
it('the report code-behind emits one control stub — txtTotal, and nothing else', () => {
210+
const controls = reportCode.nodes.filter((n) => n.kind === 'form-instance-control');
211+
expect(controls.map((n) => n.name)).toEqual(['txtTotal']);
212+
});
213+
214+
it('no control stub is named Form or Report in either code-behind', () => {
215+
const bogus = [...formCode.nodes, ...reportCode.nodes].filter(
216+
(n) =>
217+
n.kind === 'form-instance-control' &&
218+
['form', 'report'].includes(n.name.toLowerCase()),
219+
);
220+
expect(bogus, 'a form-level event must never synthesize a control node').toHaveLength(0);
221+
});
222+
});
223+
224+
// =============================================================================
225+
// AC #6 — a non-code-behind `.cls` is still skipped entirely. A service class
226+
// with an underscore method must not gain a layout stub just because the new
227+
// branch exists.
228+
// =============================================================================
229+
describe('issue-247 AC#6: the Form_/Report_ basename guard still holds', () => {
230+
it('a plain service class with a Form_Load method emits no event-handler edge', () => {
231+
const servicePath = path.join(FIXTURE_DIR, 'ServicioInformes.cls');
232+
const source = [
233+
'Attribute VB_Name = "ServicioInformes"',
234+
'Option Explicit',
235+
'',
236+
'Public Sub Form_Load()',
237+
' Debug.Print "not code-behind"',
238+
'End Sub',
239+
'',
240+
'Public Sub GenerarHTML_Principal()',
241+
' Debug.Print "not an event"',
242+
'End Sub',
243+
'',
244+
].join('\r\n');
245+
// In-memory source with a path that is NOT `Form_*` / `Report_*`. The
246+
// extractor never reads from disk, so no fixture file is needed.
247+
const result = new VbaExtractor(servicePath, source).extract();
248+
expect(fn(result, 'Form_Load')).toBeDefined();
249+
expect(result.edges.filter((e) => e.kind === 'event-handler')).toHaveLength(0);
250+
expect(result.nodes.filter((n) => n.kind === 'form-layout')).toHaveLength(0);
251+
expect(result.nodes.filter((n) => n.kind === 'form-instance-control')).toHaveLength(0);
252+
});
253+
});
254+
255+
// =============================================================================
256+
// AC #7 — a code-behind whose layout file was never exported still gets the
257+
// node, and the id is still derived from the sibling PATH.
258+
//
259+
// This is deliberate, and it is where most of the corpus benefit comes from:
260+
// a real Dysflow export can carry a `Form_X.cls` whose `Form_X.form.txt` was
261+
// not exported, and those forms are the majority in some projects. The
262+
// synthesized layout node is the only node the form has, so dropping it would
263+
// drop the handler edge with it. It also normalizes to the same Access object
264+
// identity a `DoCmd.OpenForm "X"` target does, so the navigation stub can
265+
// resolve onto it.
266+
//
267+
// This mirrors the control branch's long-standing behaviour: a handler whose
268+
// control is missing from the sibling still gets its stub.
269+
// =============================================================================
270+
describe('issue-247 AC#7: a code-behind with no exported layout still gets its node', () => {
271+
it('a Form_*.cls with no .form.txt beside it still emits the layout node and edge', () => {
272+
const orphanPath = path.join(FIXTURE_DIR, 'Form_NoLayoutSibling.cls');
273+
expect(
274+
fs.existsSync(path.join(FIXTURE_DIR, 'Form_NoLayoutSibling.form.txt')),
275+
'the fixture directory must NOT contain this sibling',
276+
).toBe(false);
277+
278+
const source = [
279+
'Attribute VB_Name = "Form_NoLayoutSibling"',
280+
'Option Explicit',
281+
'',
282+
'Private Sub Form_Load()',
283+
'End Sub',
284+
'',
285+
].join('\r\n');
286+
const result = new VbaExtractor(orphanPath, source).extract();
287+
expect(fn(result, 'Form_Load')).toBeDefined();
288+
289+
const layouts = result.nodes.filter((n) => n.kind === 'form-layout');
290+
expect(layouts).toHaveLength(1);
291+
// The id is keyed on the sibling `.form.txt` path, never on the `.cls`
292+
// path — that is what lets the real node overwrite it if the layout is
293+
// exported later.
294+
expect(layouts[0]?.filePath).toBe(
295+
path.join(FIXTURE_DIR, 'Form_NoLayoutSibling.form.txt'),
296+
);
297+
expect(handlerEdges(result, 'Form_Load')).toHaveLength(1);
298+
});
299+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
VERSION 1.0 CLASS
2+
BEGIN
3+
MultiUse = -1 'True
4+
END
5+
Attribute VB_Name = "Form_Lifecycle"
6+
Option Compare Database
7+
Option Explicit
8+
9+
' =============================================================================
10+
' Form_Lifecycle — fixture for issue #247 (form-level lifecycle handlers).
11+
'
12+
' Holds one of each shape the procedures sweep has to tell apart:
13+
' Form_Open / Form_Load / Form_Unload — form-level lifecycle events, which
14+
' must bind to the SIBLING form-layout node, never to a control.
15+
' Form_Helper — an ordinary method that merely
16+
' starts with `Form_`. `Helper` is not an Access event name, so it must
17+
' produce NO event-handler edge at all.
18+
' cmdSave_Click — a plain control handler. Its node
19+
' id and its edge must be byte-identical to what they were before #247.
20+
' =============================================================================
21+
22+
Private Sub Form_Open(Cancel As Integer)
23+
Me.cmdSave.Enabled = True
24+
End Sub
25+
26+
Private Sub Form_Load()
27+
Dim helper As Object
28+
Set helper = CreateObject("Scripting.Dictionary")
29+
End Sub
30+
31+
Private Sub Form_Unload(Cancel As Integer)
32+
Me.cmdSave.Enabled = False
33+
End Sub
34+
35+
Private Sub Form_Helper()
36+
' Not an Access event — `Helper` is not in ACCESS_EVENT_NAMES.
37+
Me.cmdSave.Caption = "Guardar"
38+
End Sub
39+
40+
Private Sub cmdSave_Click()
41+
Me.cmdSave.Caption = "Guardado"
42+
End Sub

0 commit comments

Comments
 (0)