Merge pull request #36 from theodevelop/audit/bison-review

audit: bison review & fixes

Author: theodevelop

.gitignore

@@ -3,4 +3,7 @@ dist/
 client/out/
 server/out/
 *.vsix
-.env
+.env
+tests/_*
+docs/
+.vscode/

CHANGELOG.md

@@ -13,6 +13,8 @@ All notable changes to the **Bison/Flex Language Support** extension will be doc
 - **Flex — lowercase start condition names** (audit-C): all start-condition regex patterns used `[A-Z_][A-Z0-9_]*` (uppercase only). SC names that are valid C identifiers but lowercase (e.g. `%x comment`) were silently ignored, skipping `flex/undefined-sc` and `flex/unused-sc` diagnostics for them entirely.
 - **Flex — single-tab action separator in abbreviation ref scan** (audit-D): the heuristic that separates the pattern from the action used `\s{2,}`, which did not match a single-tab separator. `{identifier}` tokens inside the C action body (e.g. compound literals) were falsely counted as abbreviation references, suppressing `flex/unused-abbrev`.
 - **Cleanup**: removed two dead entries in the catch-all pattern set that contained a literal newline character and could never match a rule line.
+- **Bison — lowercase/mixed-case tokens in precedence declarations** (audit-E): `%left`/`%right`/`%nonassoc` used an uppercase-only regex `[A-Z_][A-Z0-9_]*`, silently dropping tokens like `kPLUS` or `tTOKEN` from the precedence table. This caused false `bison/undeclared-token` warnings and incorrect shift/reduce heuristic results for such tokens.
+- **Bison — `$N` references after nested sub-blocks in inline actions** (audit-F): the `extractDollarRefs` scanner used `/\{([^}]*)\}/` which stops at the first `}`, missing `$N` references that appear after a nested `{ … }` block inside the same action (e.g. `{ if (cond) { log(); } $$ = $5; }`). Replaced with a brace-depth scanner; the same fix was applied to `extractSymbols`, `getFirstSymbol`, and `extractRuleReferences` for consistency.
 
 ---
 

server/src/parser/bisonParser.ts

@@ -166,7 +166,7 @@ export function parseBisonDocument(text: string): BisonDocument {
     const precMatch = trimmed.match(/^%(left|right|nonassoc|precedence)\s+(.*)/);
     if (precMatch) {
       const kind = precMatch[1] as PrecedenceDeclaration['kind'];
-      const rawSymbols = precMatch[2].match(/[A-Z_][A-Z0-9_]*|"[^"]*"/g) || [];
+      const rawSymbols = precMatch[2].match(/[A-Za-z_][A-Za-z0-9_]*|"[^"]*"/g) || [];
       const symbols: string[] = [];
       const symbolRanges: Range[] = [];
       for (const raw of rawSymbols) {
@@ -389,6 +389,33 @@ function replaceStringLiterals(text: string): string {
     .replace(/'((?:[^'\\]|\\.)*)'/g, (_, content) => ` ${strLiteralPlaceholder(`'${content}'`)} `);
 }
 
+/**
+ * Remove all brace-balanced { ... } blocks from `text`, replacing each with `replacement`.
+ * Handles arbitrarily nested braces, unlike /\{[^}]*\}/ which stops at the first `}`.
+ * Unmatched `{` without a closing `}` (e.g. a multi-line action opener on its own line)
+ * are left out of the result — the Phase 3 brace tracker handles them separately.
+ */
+function removeBalancedBraces(text: string, replacement: string = ' '): string {
+  let result = '';
+  let depth = 0;
+  let pendingOpen = false;  // true while inside a block that hasn't been closed yet
+  for (let i = 0; i < text.length; i++) {
+    if (text[i] === '{') {
+      if (depth === 0) pendingOpen = true;
+      depth++;
+    } else if (text[i] === '}') {
+      depth = Math.max(0, depth - 1);
+      if (depth === 0 && pendingOpen) {
+        result += replacement;  // only emit placeholder when the block is fully closed
+        pendingOpen = false;
+      }
+    } else if (depth === 0) {
+      result += text[i];
+    }
+  }
+  return result;
+}
+
 /**
  * Extract all grammar symbols (identifiers) from a production RHS in order.
  *
@@ -399,8 +426,7 @@ function replaceStringLiterals(text: string): string {
  * `"("` apart from `"{"` (both have different placeholders).
  */
 function extractSymbols(text: string): string[] {
-  const cleaned = replaceStringLiterals(text)
-    .replace(/\{[^}]*\}/g, ' __midaction__ ')     // inline actions count as a symbol ($N position)
+  const cleaned = removeBalancedBraces(replaceStringLiterals(text), ' __midaction__ ')  // inline actions count as a symbol ($N position)
     .replace(/%prec\s+\S+/g, ' ')                 // remove %prec TOKEN
     .replace(/%empty/g, ' ')                      // remove %empty
     .replace(/\/\/.*$/g, ' ')                     // remove line comments
@@ -423,8 +449,7 @@ function extractSymbols(text: string): string[] {
  * `__s` (not all-caps) and is therefore not confused with a real terminal.
  */
 function getFirstSymbol(text: string): string | undefined {
-  const cleaned = replaceStringLiterals(text)
-    .replace(/\{[^}]*\}/g, ' ')                   // remove inline actions
+  const cleaned = removeBalancedBraces(replaceStringLiterals(text))   // remove inline actions
     .replace(/%prec\s+\S+/g, ' ')                 // remove %prec TOKEN
     .replace(/%empty/g, ' ')                      // remove %empty
     .replace(/\/\/.*$/g, ' ')                     // remove line comments
@@ -518,15 +543,27 @@ function parseTokenNames(text: string, type: string | undefined, lineNum: number
 
 /**
  * Scan the inline action block(s) on a single line for $n references.
- * Only handles single-line { ... } blocks; multi-line actions are not detected here.
+ * Uses a brace-depth scanner so that $n references appearing after a nested
+ * sub-block (e.g. `{ if (x) { foo(); } $$ = $1; }`) are not missed.
+ * Only handles single-line { ... } blocks; multi-line actions are tracked by
+ * the caller (Phase 3 loop in parseBisonDocument).
  * $$ and $<type>n are deliberately skipped.
  */
 function extractDollarRefs(text: string, lineNum: number, fullLine: string): DollarRef[] {
   const refs: DollarRef[] = [];
-  const actionRegex = /\{([^}]*)\}/g;
-  let actionMatch: RegExpExecArray | null;
-  while ((actionMatch = actionRegex.exec(text)) !== null) {
-    const actionContent = actionMatch[1];
+  let i = 0;
+  while (i < text.length) {
+    if (text[i] !== '{') { i++; continue; }
+    // Found the opening brace of an action block — scan to the matching '}'
+    let depth = 1;
+    let j = i + 1;
+    while (j < text.length && depth > 0) {
+      if (text[j] === '{') depth++;
+      else if (text[j] === '}') depth--;
+      j++;
+    }
+    // text[i+1 .. j-2] is the full content of this balanced action block
+    const actionContent = text.substring(i + 1, j - 1);
     const dollarRegex = /\$(\d+)/g;
     let m: RegExpExecArray | null;
     while ((m = dollarRegex.exec(actionContent)) !== null) {
@@ -540,6 +577,7 @@ function extractDollarRefs(text: string, lineNum: number, fullLine: string): Dol
         range: Range.create(lineNum, col >= 0 ? col : 0, lineNum, (col >= 0 ? col : 0) + fullMatch.length),
       });
     }
+    i = j; // advance past the entire balanced block
   }
   return refs;
 }
@@ -579,9 +617,7 @@ function extractRuleReferences(text: string, lineNum: number, fullLine: string,
 
   // Find identifiers in rule bodies (potential token/nonterminal references)
   // Skip: strings, actions (braces), %prec keyword (but keep its token), %empty, comments
-  const cleaned = text
-    .replace(/"(?:[^"\\]|\\.)*"/g, '')     // remove strings
-    .replace(/\{[^}]*\}/g, '')             // remove inline actions
+  const cleaned = removeBalancedBraces(text.replace(/"(?:[^"\\]|\\.)*"/g, ''))  // remove strings, then inline actions
     .replace(/%prec/g, '')                 // remove %prec keyword (keep the token name)
     .replace(/%empty/g, '')                // remove %empty
     .replace(/\/\/.*$/g, '');              // remove line comments

tests/test-diagnostic-codes.ts

@@ -505,6 +505,53 @@ console.log('\n=== TEST: Flex audit checks ===');
   assert(unused.length === 0, 'Bug-D: abbreviation used in pattern before single-tab action must not produce flex/unused-abbrev');
 }
 
+// ─────────────────────────────────────────────────────────────────────────────
+// Bison audit checks
+// ─────────────────────────────────────────────────────────────────────────────
+console.log('\n=== TEST: Bison audit checks ===');
+
+{
+  // audit-E: lowercase/mixed-case tokens in %left/%right/%nonassoc must be recorded
+  // in doc.precedence so they don't produce false bison/undeclared-token diagnostics.
+  const src = [
+    '%token kPLUS kMINUS kNUM',
+    '%left kPLUS kMINUS',
+    '%%',
+    'start : expr ;',
+    'expr : expr kPLUS expr { $$ = $1 + $3; }',
+    '     | expr kMINUS expr { $$ = $1 - $3; }',
+    '     | kNUM',
+    '     ;',
+    '%%',
+  ].join('\n');
+  const doc = parseBisonDocument(src);
+  assert(
+    doc.precedence.length === 1 && doc.precedence[0].symbols.includes('kPLUS') && doc.precedence[0].symbols.includes('kMINUS'),
+    'audit-E: lowercase tokens kPLUS/kMINUS recorded in doc.precedence',
+  );
+  const diags = computeBisonDiagnostics(doc, src);
+  const undeclared = diags.filter(d => d.code === 'bison/undeclared-token');
+  assert(undeclared.length === 0, 'audit-E: no false bison/undeclared-token for lowercase precedence tokens (got ' + undeclared.length + ')');
+}
+
+{
+  // audit-F: $N after a nested sub-block inside an action must be detected.
+  // Pattern: A { if (cond) { skip(); } $5; }  — 1 symbol, $5 is out-of-bounds.
+  // Old /\{[^}]*\}/ missed $5 because it appears after the inner `}`.
+  const src = [
+    '%token A',
+    '%%',
+    'start : stmt ;',
+    'stmt : A { if (1) { int x = 0; } $$ = $5; }',
+    '     ;',
+    '%%',
+  ].join('\n');
+  const doc = parseBisonDocument(src);
+  const diags = computeBisonDiagnostics(doc, src);
+  const oob = diags.filter(d => d.code === 'bison/out-of-bounds' && d.message.includes('$5'));
+  assert(oob.length >= 1, 'audit-F: $5 after nested sub-block in action is detected as out-of-bounds');
+}
+
 // ─────────────────────────────────────────────────────────────────────────────
 // Results
 // ─────────────────────────────────────────────────────────────────────────────