# F-03 — collectDiagnostics.js: Variable declaration scan via regex when AST has VarStatement nodes

**Severity:** Major  
**File:** `core/src/collectDiagnostics.js` (lines 104–141)  
**Category:** Compensating for incomplete AST use  
**Remediation appropriate:** Yes — Tier 1

---

## The issue

`collectDiagnostics.js` builds the `declaredVariables` and `globalVariables` maps with a
line-by-line scan that calls the old regex-based `parseVarDeclaration()`:

```javascript
// collectDiagnostics.js:104–141
for (let lineIndex = 0; lineIndex < document.lineCount; lineIndex += 1) {
    const declCls = facts.executionClassificationMap[lineIndex];
    if (declCls.isOutput || !declCls.isExecutable) continue;

    const lineText = document.lineAt(lineIndex).text;
    const declaration = parseVarDeclaration(lineText);       // ← regex on raw line
    // ...
    if (declaration) {
        declaration.variables.forEach((variableName) => {
            // ... populate declaredVariables, globalVariables
        });
    }
}
```

`parseVarDeclaration()` uses:
```javascript
text.match(/^\s*#?\s*VAR\s+(.+?)\s*:\s*([A-Z0-9_]+)\s*$/i)
// and
text.match(/^\s*#?\s*(STRING|SHORT|LONG|DATE|...)\s+(.+?)\s*$/i)
```

---

## What the AST already provides

The AST parser produces `VarStatement` nodes for both VAR declaration formats:

```javascript
// VAR name : TYPE  →  { type: "VarStatement", names: ["name"], varType: "TYPE", line: N }
// TYPE name, name  →  { type: "VarStatement", names: ["a", "b"], varType: "TYPE", line: N }
```

`documentFacts.js` already walks `VarStatement` nodes in `collectLocals()` to build
function-local variable maps for scope analysis. It is doing the same work — but discarding
the result for the global variable map, which is separately re-done in `collectDiagnostics.js`
via regex.

---

## Why this matters

1. **Redundant work.** The AST is already parsed once and cached. `collectDiagnostics.js` does
   a second full scan of the document text to extract the same information the AST provides for free.

2. **Divergence risk.** `parseVarDeclaration()` is a regex that must stay in sync with the
   AST parser's `VAR` and TYPE_KEYWORD handling. If one is updated and the other is not, the
   validator's `declaredVariables` map will diverge from the AST's understanding of the document.
   This has already happened once: the alternated-format type list in `parser.js` was a superset
   of `ACURITY_TYPES` (v1 Batch 1, Major finding, now fixed). The AST parser now drives from
   `TYPE_KEYWORDS` in the lexer — `parseVarDeclaration` still has its own hardcoded list:
   ```javascript
   /^\s*#?\s*(STRING|SHORT|LONG|DATE|DOUBLE|BOOLEAN|TIME|INTEGER|BIGINT)\s+/i
   ```
   If `BIGINT` is ever removed from the type set, it must be updated in two places.

3. **Scope detection gap.** The current loop uses `getFunctionScopeAtLine()` to decide whether
   a declaration is global or function-scoped. But the `functionScopes` come from the AST-based
   `buildFunctionScopesFromAst()`. There is therefore an implicit coupling between the regex-based
   declaration scan and the AST-based scope map — the line number returned by the regex must align
   exactly with the line number in the AST node (it does, but this is an assumption).

---

## Recommended fix

Move the `declaredVariables` and `globalVariables` collection into `documentFacts.js` as
AST-derived data, exposed on the `facts` object:

```javascript
// In documentFacts.js — new helper:
function buildDeclaredVariablesFromAst(ast, functionScopes) {
    const globalVariables = new Map();
    const declaredVariables = new Map();

    // Walk all statements at any depth
    walkStatements(ast.statements, (stmt, enclosingScope) => {
        if (stmt.type !== NodeType.VarStatement) return;
        for (const name of stmt.names) {
            const scopeKey = enclosingScope ? `${enclosingScope.startLine}:${name}` : `global:${name}`;
            if (!declaredVariables.has(scopeKey)) {
                declaredVariables.set(scopeKey, {
                    name, type: stmt.varType, line: stmt.line,
                    isFunctionScoped: !!enclosingScope,
                    isFunctionParameter: false
                });
            }
            if (!enclosingScope && !globalVariables.has(name)) {
                globalVariables.set(name, { type: stmt.varType, line: stmt.line,
                    isFunctionScoped: false, isFunctionParameter: false });
            }
        }
    });

    // Also add function parameters (already done in collectDiagnostics today)
    for (const scope of functionScopes) {
        scope.params.forEach((paramType, paramName) => {
            const scopeKey = `${scope.startLine}:${paramName}`;
            if (!declaredVariables.has(scopeKey)) {
                declaredVariables.set(scopeKey, {
                    name: paramName, type: paramType, line: scope.startLine,
                    isFunctionScoped: true, isFunctionParameter: true
                });
            }
        });
    }

    return { declaredVariables, globalVariables };
}
```

`collectDiagnostics.js` then simply:
```javascript
const { declaredVariables, globalVariables } = facts;  // no more loop, no parseVarDeclaration
```

The `facts` object gains two new keys. The line-by-line scan and `parseVarDeclaration` import
in `collectDiagnostics.js` are deleted.

---

## Impact

- Eliminates ~40 lines of regex-based scanning from `collectDiagnostics.js`
- Eliminates the `parseVarDeclaration` import from `collectDiagnostics.js`
- Eliminates the divergence risk between `parseVarDeclaration` type list and the lexer's `TYPE_KEYWORDS`
- Ensures that variable declarations are always understood consistently by both the AST and validators
- Small perf win: one fewer full-document scan per validation pass
