diff --git a/__tests__/extraction-vba-preprocess.test.ts b/__tests__/extraction-vba-preprocess.test.ts index fbc0173e..31bc356a 100644 --- a/__tests__/extraction-vba-preprocess.test.ts +++ b/__tests__/extraction-vba-preprocess.test.ts @@ -444,3 +444,221 @@ describe('preprocessConditionalCompilation', () => { expect(out).toContain('SafeFallback'); }); }); + +/** + * Issue #51 — `fix(vba): conditional-compilation evaluator — Win32/Win16, + * True=-1 semantics, #Const support`. Three concrete gaps the previous + * evaluator had: + * + * 1. `Win32` / `Win16` were not in the identifier substitution table — + * the legacy `#If Win32 Then` guard (always True on modern Windows) + * blanked its ACTIVE branch. + * 2. The whitelist rejected `-` outright, so `#If Win64 = -1 Then` + * blanked its branch even though VBA's True = -1 makes the + * comparison True. + * 3. `#Const NAME = ` was not parsed — a user-defined + * `#Const MODO_DEBUG = True` was ignored, so `#If MODO_DEBUG Then` + * fell through to the unknown-identifier fallback (false) and the + * user's TRUE branch was blanked. + * + * The fix maps `Win32` → true and `Win16` → false alongside the existing + * VBA7/Win64/Mac table; substitutes `true`/`false` with `-1`/`0` so + * numeric equality comparisons match VBA's True = -1 semantics; accepts + * unary minus in the whitelist; and parses `#Const` into a per-call map + * consulted before the hardcoded constants. + * + * Each atom asserts both the kept-or-blanked behavior of the active + * branch AND line-count parity (the core invariant — downstream + * extraction's `startLine` values depend on it). + */ +describe('Issue #51: Win32/Win16 + True=-1 + #Const support', () => { + // ---- atom 1: Win32 is True on modern Windows ------------------------- + it('atom 1: #If Win32 Then keeps the branch on modern Windows', () => { + const src = [ + '#If Win32 Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(3); + expect(lines[0]).toBe(''); + expect(lines[1]).toContain('Debug.Print "x"'); + expect(lines[2]).toBe(''); + }); + + // ---- atom 2: Win16 is False (not a current target) -------------------- + it('atom 2: #If Win16 Then blanks the branch on modern Windows', () => { + const src = [ + '#If Win16 Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(3); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe(''); + expect(lines[2]).toBe(''); + }); + + // ---- atom 3: #Const True keeps the branch ---------------------------- + it('atom 3: #Const MODO_DEBUG = True then #If MODO_DEBUG Then keeps the branch', () => { + const src = [ + '#Const MODO_DEBUG = True', + '#If MODO_DEBUG Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(4); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe(''); + expect(lines[2]).toContain('Debug.Print "x"'); + expect(lines[3]).toBe(''); + }); + + // ---- atom 4: #Const False → #Else branch kept ------------------------ + it('atom 4: #Const MODO_DEBUG = False then #If / #Else selects the else branch', () => { + const src = [ + '#Const MODO_DEBUG = False', + '#If MODO_DEBUG Then', + 'Debug.Print "x"', + '#Else', + 'Debug.Print "y"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(6); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe(''); + expect(lines[2]).toBe(''); + expect(lines[3]).toBe(''); + expect(lines[4]).toContain('Debug.Print "y"'); + expect(lines[5]).toBe(''); + }); + + // ---- atom 5: #Const integer literal preserved through comparison ---- + it('atom 5: #Const X = 1 then #If X = 1 Then keeps the branch', () => { + const src = [ + '#Const X = 1', + '#If X = 1 Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(4); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe(''); + expect(lines[2]).toContain('Debug.Print "x"'); + expect(lines[3]).toBe(''); + }); + + // ---- atom 6: VBA True = -1 semantics for #If Win64 = -1 -------------- + it('atom 6: #If Win64 = -1 Then keeps the branch (VBA True = -1 semantics)', () => { + const src = [ + '#If Win64 = -1 Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(3); + expect(lines[0]).toBe(''); + expect(lines[1]).toContain('Debug.Print "x"'); + expect(lines[2]).toBe(''); + }); + + // ---- atom 7: #Const True cannot satisfy `= False` -------------------- + it('atom 7: #Const X = True then #If X = False Then blanks the branch', () => { + const src = [ + '#Const X = True', + '#If X = False Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(4); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe(''); + expect(lines[2]).toBe(''); + expect(lines[3]).toBe(''); + }); + + // ---- atom 8: unknown identifier still evaluates false (regression) --- + it('atom 8 (negative regression): unknown identifier still evaluates false', () => { + const src = [ + '#If NonExistent Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(3); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe(''); + expect(lines[2]).toBe(''); + }); + + // ---- defensive: line-count parity across every atom ------------------- + it('Issue #51 atoms preserve line-count parity across the suite', () => { + // Re-runs the same six positive atoms and checks line-count parity on + // each — the core invariant the preprocessor guarantees (downstream + // extraction's `startLine` values depend on it). + const sources = [ + // atom 1 + '#If Win32 Then\nDebug.Print "x"\n#End If', + // atom 3 + '#Const MODO_DEBUG = True\n#If MODO_DEBUG Then\nDebug.Print "x"\n#End If', + // atom 4 + '#Const MODO_DEBUG = False\n#If MODO_DEBUG Then\nDebug.Print "x"\n#Else\nDebug.Print "y"\n#End If', + // atom 5 + '#Const X = 1\n#If X = 1 Then\nDebug.Print "x"\n#End If', + // atom 6 + '#If Win64 = -1 Then\nDebug.Print "x"\n#End If', + // atom 7 + '#Const X = True\n#If X = False Then\nDebug.Print "x"\n#End If', + ]; + for (const src of sources) { + const out = preprocessConditionalCompilation(src); + expect(out.split('\n').length).toBe(src.split('\n').length); + } + }); + + // ---- defensive: #Const line itself is blanked (parity) ---------------- + it('Issue #51: #Const directive line is blanked (line-count parity)', () => { + const src = '#Const MODO_DEBUG = True\nSub X()\nEnd Sub'; + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(3); + expect(lines[0]).toBe(''); + expect(lines[1]).toBe('Sub X()'); + expect(lines[2]).toBe('End Sub'); + expect(out).not.toContain('#Const'); + }); + + // ---- defensive: #Const RHS as a quoted string is unsupported ---------- + it('Issue #51: #Const NAME = "literal" is unsupported (no entry, line blanked)', () => { + // VBA forbids string-literal #Const values (CC evaluates only at + // compile time, no runtime string comparison); the implementation + // must NOT store the entry, and the line must be blanked. + const src = [ + '#Const X = "hello"', + '#If X Then', + 'Debug.Print "x"', + '#End If', + ].join('\n'); + const out = preprocessConditionalCompilation(src); + const lines = out.split('\n'); + expect(lines).toHaveLength(4); + expect(lines[0]).toBe(''); // #Const line blanked regardless + // X is unknown — falls through to the unknown-identifier fallback + // (the conservative behavior preserved from the original + // implementation), which blanks the branch. + expect(lines[2]).toBe(''); + }); +}); diff --git a/src/extraction/vba-preprocess.ts b/src/extraction/vba-preprocess.ts index 25b4f680..b70723d0 100644 --- a/src/extraction/vba-preprocess.ts +++ b/src/extraction/vba-preprocess.ts @@ -183,13 +183,37 @@ const IF_DIRECTIVE = /^\s*#If\s+(.+?)\s+Then\s*$/i; const ELSEIF_DIRECTIVE = /^\s*#ElseIf\s+(.+?)\s+Then\s*$/i; const ELSE_DIRECTIVE = /^\s*#Else\s*$/i; const ENDIF_DIRECTIVE = /^\s*#(?:End\s*If|EndIf)\s*$/i; +// #Const NAME = — captured name (identifier) and value (anything +// to EOL). The directive line is always blanked; the value is fed through +// the same normalize+evaluate pipeline as a #If expression so it inherits +// the substitution table for free. +const CONST_DIRECTIVE = /^\s*#Const\s+([A-Za-z_][A-Za-z_0-9]*)\s*=\s*(.+?)\s*$/i; /** * Evaluate VBA conditional-compilation directives for the modern Windows * Access/VBA target this extractor is designed around: - * - VBA7 = true - * - Win64 = true - * - Mac = false + * - VBA7 = true (VBA7 runtime available) + * - Win64 = true (64-bit VBA host — Access/VBA on x64 Windows) + * - Win32 = true (legacy guard; always true on modern Windows, incl. Win64) + * - Win16 = false (legacy 16-bit Windows — not a current target) + * - Mac = false (Mac host) + * + * In addition, the file-scoped `#Const NAME = ` table is consulted + * before the hardcoded constants above — `#Const MODO_DEBUG = True` then + * `#If MODO_DEBUG Then` keeps the branch. The #Const line itself is + * blanked to preserve line-count parity. + * + * Truthiness follows VBA semantics: True = -1, False = 0. We achieve this + * by substituting the JS boolean literals `true`/`false` with their VBA + * numeric equivalents `-1`/`0` after the identifier+operator rewrites and + * before the whitelist check. This is a deliberate simplification — full + * VBA CC is bitwise on -1/0 (so `True And True = -1`), but the supported + * expression surface here is truthy comparison / `=` / `<>` / `And` / + * `Or` / `Not`, for which JS `&&`/`||`/`!` truthy evaluation is + * equivalent: any non-zero operand is truthy, matching VBA's "non-zero is + * true" convention. If a future task needs bitwise-precise `-1` semantics + * (e.g. distinguishing `#If X = 1` from `#If X = -1` on a `True` const), + * promote this evaluator to full integer arithmetic. * * Directives and inactive branch lines are replaced with empty strings so * downstream extraction keeps source-line parity. Unsupported/unsafe @@ -200,12 +224,34 @@ export function preprocessConditionalCompilation(src: string): string { const lines = src.split('\n'); const out: string[] = []; const stack: ConditionalFrame[] = []; + // Per-call #Const table. Name → post-evaluation numeric string (e.g. + // "-1" for True, "0" for False, "1" for an integer literal). Stored as a + // string so it can be substituted verbatim into the normalized + // expression. A name that fails to evaluate is NOT stored — the lookup + // simply misses and the conservative fallback (unknown identifier → + // false) applies, exactly as for an unrecognised hardcoded constant. + const constTable = new Map(); for (const line of lines) { + // Parse #Const BEFORE the other directives so the table is up-to-date + // when a subsequent #If expression references it. The line itself is + // always blanked (line-count parity invariant). + const constMatch = CONST_DIRECTIVE.exec(line); + if (constMatch) { + const name = (constMatch[1] ?? '').trim(); + const rhs = (constMatch[2] ?? '').trim(); + const value = evaluateConstRhs(rhs, constTable); + if (value !== null) { + constTable.set(name, value); + } + out.push(''); + continue; + } + const ifMatch = IF_DIRECTIVE.exec(line); if (ifMatch) { const parentActive = stack.every((frame) => frame.active); - const active = parentActive && evaluateConditionalExpression(ifMatch[1] ?? ''); + const active = parentActive && evaluateConditionalExpression(ifMatch[1] ?? '', constTable); stack.push({ parentActive, active, branchTaken: active }); out.push(''); continue; @@ -218,7 +264,7 @@ export function preprocessConditionalCompilation(src: string): string { const active = frame.parentActive && !frame.branchTaken && - evaluateConditionalExpression(elseIfMatch[1] ?? ''); + evaluateConditionalExpression(elseIfMatch[1] ?? '', constTable); frame.active = active; if (active) frame.branchTaken = true; } @@ -249,23 +295,76 @@ export function preprocessConditionalCompilation(src: string): string { return out.join('\n'); } -function evaluateConditionalExpression(expr: string): boolean { +/** + * Apply the full substitution pipeline to a conditional-compilation + * expression: identifier substitutions (#Const table first, then the + * hardcoded constants), operator rewrites, and the True→-1 / False→0 + * conversion that gives the evaluator correct VBA equality semantics. + * + * Returns the normalized string if it passes the whitelist (signed + * integer literal + operators + parens + whitespace only), or `null` if + * any substitution left a token that does not match the whitelist. A + * `null` return signals "do not evaluate" — the caller falls back to + * false (the conservative behaviour preserved from the original + * implementation). + */ +function normalizeConditionalExpression( + expr: string, + constTable: ReadonlyMap, +): string | null { let normalized = expr.trim(); normalized = normalized.replace(/\bThen\s*$/i, ''); normalized = normalized.replace(/<>/g, '!=='); normalized = normalized.replace(/(?=])=(?![=])/g, '==='); - normalized = normalized.replace(/\bVBA7\b/gi, 'true'); - normalized = normalized.replace(/\bWin64\b/gi, 'true'); - normalized = normalized.replace(/\bMac\b/gi, 'false'); - normalized = normalized.replace(/\bAnd\b/gi, '&&'); - normalized = normalized.replace(/\bOr\b/gi, '||'); - normalized = normalized.replace(/\bNot\b/gi, '!'); - normalized = normalized.trim(); - - if (!/^(?:true|false|\d+|&&|\|\||!|===|!==|\(|\)|\s)+$/.test(normalized)) { - return false; + + // #Const table first — user-defined constants shadow the hardcoded ones + // below. The replacement value is a numeric literal string (e.g. "-1"), + // so it cannot re-introduce identifiers or operators. Use a replacement + // function so the value is inserted verbatim — `String.replace(regex, + // string)` would otherwise interpret `$`/`\\` in the replacement. + for (const [name, value] of constTable) { + normalized = normalized.replace( + new RegExp(`\\b${escapeRegExp(name)}\\b`, 'gi'), + () => value, + ); + } + + normalized = normalized + .replace(/\bVBA7\b/gi, 'true') + .replace(/\bWin64\b/gi, 'true') + .replace(/\bWin32\b/gi, 'true') + .replace(/\bWin16\b/gi, 'false') + .replace(/\bMac\b/gi, 'false') + .replace(/\bAnd\b/gi, '&&') + .replace(/\bOr\b/gi, '||') + .replace(/\bNot\b/gi, '!') + // VBA CC truthiness: True = -1, False = 0. Substitute the JS boolean + // literals with their VBA numeric equivalents so a downstream + // `Win64 = -1` comparison (after the `=`→`===` rewrite) evaluates + // true. See the doc comment on `preprocessConditionalCompilation` + // for the chosen simplification. + .replace(/\btrue\b/gi, '-1') + .replace(/\bfalse\b/gi, '0') + .trim(); + + // Whitelist — after every substitution the expression should consist + // only of signed integer literals, operators, parens, and whitespace. + // The original whitelist accepted `true`/`false` literals; we have + // converted those to `-1`/`0` above, so the alternation is no longer + // needed. The signed-integer alternation accepts unary minus. + if (!/^(?:-?\d+|\s|&&|\|\||!|===|!==|\(|\))+?$/.test(normalized)) { + return null; } + return normalized; +} + +function evaluateConditionalExpression( + expr: string, + constTable: ReadonlyMap = new Map(), +): boolean { + const normalized = normalizeConditionalExpression(expr, constTable); + if (normalized === null) return false; try { return Boolean(Function(`"use strict"; return (${normalized});`)()); } catch { @@ -273,6 +372,44 @@ function evaluateConditionalExpression(expr: string): boolean { } } +/** + * Evaluate the RHS of a `#Const NAME = ` directive. Returns the + * value as a JS-substitutable numeric string (e.g. `"-1"`, `"0"`, + * `"1"`) so it can be substituted verbatim into subsequent #If + * expressions, OR `null` if the RHS is unsupported (whitelist failure, + * non-numeric/non-boolean evaluation result). + * + * Re-uses the same normalize pipeline as #If so recursive `#Const X = Y` + * references resolve against the existing table (but a self-reference is + * impossible: the new entry is added AFTER the RHS is evaluated, so it + * is invisible to its own evaluation). + */ +function evaluateConstRhs( + rhs: string, + constTable: ReadonlyMap, +): string | null { + const normalized = normalizeConditionalExpression(rhs, constTable); + if (normalized === null) return null; + let result: unknown; + try { + result = Function(`"use strict"; return (${normalized});`)(); + } catch { + return null; + } + if (typeof result === 'number' && Number.isFinite(result)) { + return String(Math.trunc(result)); + } + if (typeof result === 'boolean') { + return result ? '-1' : '0'; + } + return null; +} + +/** Escape regex metacharacters so a name can be interpolated safely. */ +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + /** * Split `line` into alternating code / string-literal segments and apply * `REM_MIDLINE` only to code segments. String segments are returned