feat(vba): record each procedure's error policy - #287
Merged
Merged
Conversation
Nothing in the graph recorded how a procedure handles errors, and neither of the two questions that matters is greppable: an ABSENCE scoped to a procedure body has no text to match, and whether an `On Error Resume Next` is ever closed depends on the whole body rather than on any one line. In this corpus that hides 816 procedures with no protection at all and 516 procedures whose suppression scope runs to the `End Sub`. Every procedure's `function` node now carries an `errorPolicy` object: `protection`, `handlerLabel`, `handlerStartLine`, `handlerEndLine`, `behavior`, `handlerCount`, `resumeNextOpen` and `danglingTarget`. Decisions taken: - ZERO new node kinds and ZERO new edge kinds. Section 4 of `docs/vba-error-handling-plan.md` rejects a `label` node plus a `handles-error` edge because 96.5% of the corpus's 3,911 line labels are the same label (`errores`) doing the same job — one bit, which belongs in a field. Measured on the corpus: 14,649 nodes / 21,361 edges / 26,613 unresolved references before and after, byte-identical. - A label defined but never targeted by an `On Error GoTo` is CONTROL FLOW, not a handler. 137 labels in the corpus are exactly that (`siguiente`, `salir`, `fin`). Reporting one as a handler would give a confidently wrong answer to "does this procedure handle errors", the worst failure mode available here, so a label only opens a handler region when an `On Error GoTo` in the SAME procedure names it. - Every rule scans the MASKED line, so `s = "On Error GoTo errores"` sets nothing (the #209 discipline), and the procedure body's END is the existing `PROCEDURE_END_RE` — the same constant the `proc-end` rule in `enums-consts.ts` dispatches — rather than a second end detector, which is what makes the colon-separated single-line procedure form (#208) work here for free. The body's START is the node the procedures pre-walk already emitted, so there is no second `PROC_RE` dispatch either. - The classifier agrees with `scripts/vba-coverage-probe.mjs` by construction: same label regex, same statement-keyword guard, and the same treatment of `On Error GoTo 0` / `-1` as resets and of every other target as a handler label. Its `protection` census reproduces the probe's exactly — 3,774 handler / 227 resume-next / 816 none over 4,817 procedures. - `On Error GoTo -1` is valid VBA (it clears the current error) and appears 0 times in the corpus. It is treated as a reset and pinned by a fixture so the zero-occurrence case cannot rot untested. - `behavior` is deliberately left null. It is derived in E3, where the handler body's calls are already being classified; guessing it here would be fabrication. The classifier's `count` stays at 0 for its whole life, so a file whose only content is an `On Error` statement is still a file with no symbols and gains no module node. Closes #259 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d
The E2 acceptance line predicted exactly one dangling handler target in the corpus, carried over from the pre-probe hand census. E1 landed the committed probe and its reconciliation table already records the measured figure as 0; this line was the last place still asserting 1, so the implementer of E2 was being asked to find something that is not there. The corpus does contain a handler literally named `noExiste`, defined in its own procedure and therefore correctly not dangling — almost certainly what the hand census mistook for an unresolved target. The protection distribution on the same line is updated to the measured 3,774 / 227 / 816 for the same reason. Refs #259 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d
ardelperal
force-pushed
the
feat/issue-259
branch
from
September 2, 2026 16:02
f6adbf6 to
d398002
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task E2 of
docs/vba-error-handling-plan.md. Everyfunctionnode now carries anerrorPolicyobject describing how that procedure handles errors.What it adds
A new classifier
src/extraction/vba/errors.tsexporting a four-ruleRULEStable,registered in
VBA_RULE_TABLESand listed inREQUIRED_DISPATCH_TABLESso theload-time invariant covers it. The four rules are exactly the ones the issue tabulates —
on-error-label,on-error-resume-next,on-error-reset,line-label— and all fourare
scan: 'masked'andrequires: 'inside-procedure'.Decisions
labelnode plus a
handles-erroredge; this ships the field instead.region when an
On Error GoToin the same procedure names it. This is the gate thatkeeps 137 corpus labels (
siguiente,salir,fin) from producing a confidentlywrong "this procedure handles errors".
procedures pre-walk already emitted (no second
PROC_RE); its END is the existingPROCEDURE_END_REthatenums-consts.ts'sproc-endrule dispatches (no second enddetector). That is what makes the colon-separated single-line procedure form work here
for free. Unlike its sibling consumers, this one tests the masked line, so a quoted
": End Sub"cannot close a procedure early.On Error GoTo -1is treated as a reset and pinned by a fixture even though itappears 0 times in the corpus.
LINE_LABEL_REandNOT_A_LABELarethe probe's, not the simplified
^\s*(\w+):\s*$the issue sketches — the strict sketchwould have missed every trailing-statement handler (
errores: MsgBox "x") and turnedit into a dangling target.
0and-1— and only those two — are resets on bothsides; every other target, including a numeric VBA line number, is a handler label.
Corpus measurements
Roots:
00_EXPEDIENTES/src,00_GESTION_RIESGOS/src,HPS_SOLICITUDES/src(320
.bas/.clsfiles, 4,817 declared procedures).protectiondistribution — the extractor reproduces the probe's census exactly:npx tsx scripts/vba-coverage-probe.mjserrorPolicyon the nodeshandlerresume-nextnoneNode and edge invariant — merge-blocking, and it holds. Same corpus, same script,
run once with this branch's three source files and once with
origin/main's:danglingTarget: non-null for ZERO procedures. The issue's acceptance criterion asksfor exactly 1 and says to name it. There is none, and this is not a miss — §5/E1 of the
plan already records the same divergence: the original hand census counted 1 dangling
GoTotarget, and the committed probe reports 0 because it resolves labels perprocedure scope. This classifier agrees with the probe. The corpus does contain a handler
literally named
noExiste(1 occurrence), which is defined in its own procedure and istherefore correctly not dangling — that is very likely the label the original census
mistook for an unresolved target.
resumeNextOpen: 516 procedures. The probe reports 534Resume Nextscopes neverclosed; the two count different things (534 scopes spread over 516 procedures), so
they are consistent, not contradictory.
Tests
New
__tests__/extraction-vba-error-policy.test.ts— 25 tests, all green. It covers everyacceptance-criteria checkbox that is a unit test: the three protection classes, exact
handlerStartLine/handlerEndLinebracketing, an untargeted label leavingprotection: 'none',handlerCount: 2,resumeNextOpenwith and without the reset,On Error GoTo -1,danglingTarget: 'noExiste', per-procedure dangling scope, aProperty Getwith a handler,behaviorstaying null, and both named regression guards — #208 (colon-separatedsingle-line procedure) and #209 (
s = "On Error GoTo errores"sets nothing, plus a realOn Error GoTosharing a line with a quoted one still counting once). Two tests pin thezero-node/zero-edge constraint directly.
__tests__/stats-vba-rules.test.tswas updated for the new concern (7 concerns, 23 rules).Verification
npx tsc --noEmit— clean.vba-coverage-probe-errors27/27,vba-coverage-probe16/16, the ruletable/fields/dispatcher/import-invariant suites,
extraction-vba(229), and every other*vba*suite — all green (35 files, 472 passed / 1 skipped in the last batch alone).npx vitest runOOMs on this machine). 21 failures,all pre-existing and environmental: Windows
EPERM/EBUSYtemp-dir removal inworktree-detection(15),multi-repo-workspace(2),extraction(2), plus 2 innpm-sdk(bundle resolution). None touch VBA extraction.What I could not verify
53a5826, not on the currentorigin/main(df250a7).feat(vba): resolve the table or query named by DLookup and friends #285 landed while this was being written and added its own line to the same
### New Featureslist, so theCHANGELOG.mdentries will conflict on merge. Theresolution is to keep BOTH lines. Nothing else overlaps — feat(vba): resolve the table or query named by DLookup and friends #285 touched
src/extraction/vba/sql-wrapper.tsand a new test file only.Closes #259
🤖 Generated with Claude Code
https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d