# Batch 7 — Validators (Style/Naming) — Findings

**Date:** 2026-03-26
**Files reviewed:** `validators/keywordCapsValidator.js`, `validators/typeKeywordCapsValidator.js`, `validators/namingValidator.js`, `validators/argumentStringValidator.js`, `validators/interpolationValidator.js`, `validators/usagePatternValidator.js`
**Status:** Complete

---

## Summary

All six files in this batch carry the systemic `require('vscode')` violation — bringing the total
count in `core/` to **twelve** (two in Batch 1 and 5, four in Batch 6, six here). The violation
is now present in every validator file reviewed. The root-cause fix (replace `new vscode.Range`
with a plain position object throughout `core/`) must be applied globally, not file by file.

Beyond the architecture finding, the most significant correctness issue is in
`typeKeywordCapsValidator.js`: the `findParameterTypeMatches` function fails to advance its
search position for correctly-cased parameters, which causes wrong column offsets for any
subsequent mismatch in a multi-parameter function declaration. `namingValidator.js` emits
full-line ranges for all naming diagnostics rather than targeting the variable name token.
The `interpolationValidator.js` has an undocumented silent-no-op when the caller omits
`executionClassificationMap`. `usagePatternValidator.js` calls `getFunctionScopeAtLine` once
per regex match per line rather than once per line.

No security issues found in this batch.

---

## Findings

### `keywordCapsValidator.js`

#### [CRITICAL] `require('vscode')` in core/ — line 6

**Issue:** `const vscode = require("vscode")` at line 6; used to construct `vscode.Range` at
lines 46–51. This is the **seventh** violation of the non-negotiable architecture rule.

**Recommendation:** Remove the `vscode` import. Replace `new vscode.Range(l, s, l, e)` with a
plain `{ start: { line: l, character: s }, end: { line: l, character: e } }` object, following
the pattern in `variableValidator.js`.

---

#### [MINOR] Dead variable `currentIndex` — line 77

**Issue:** `let currentIndex = 0` is declared at line 77 inside `findKeywordMatches` but is
never read or written after declaration. It is dead code.

**Recommendation:** Remove the declaration.

---

#### [MINOR] Regex rebuilt on every `findKeywordMatches` call — line 80

**Issue:** Lines 80–81 construct the full keyword pattern regex on every call to
`findKeywordMatches`. Since this function is called once per non-skipped line, in a 1 000-line
document with no execution classification map the regex is constructed 1 000 times. The
`ACURITY_KEYWORDS` set is constant — the pattern never changes between calls.

**Recommendation:** Hoist the regex construction to module scope (or build it once on first call
and cache it). The sort + join + `new RegExp(...)` chain is O(k log k) per call; at module scope
it runs once.

---

### `typeKeywordCapsValidator.js`

#### [CRITICAL] `require('vscode')` in core/ — line 7

**Issue:** `const vscode = require("vscode")` at line 7; used to construct `vscode.Range` at
line 49. This is the **eighth** violation.

**Recommendation:** Same as above — replace with plain position object.

---

#### [MAJOR] `currentPos` not advanced for correctly-cased parameters — `findParameterTypeMatches`, line 147

**Issue:** `findParameterTypeMatches` iterates parameters and searches for each parameter's
type colon starting at `currentPos`. However, `currentPos` is only updated (line 178) when the
type is already a known type **and** is not already uppercase — i.e., only when a mismatch is
detected. For correctly-cased parameters (no mismatch), `currentPos` is never advanced.

Consequence: in a multi-parameter function like
`FUNCTION foo(a: STRING, b: short): LONG`, when `b: short` is checked, `colonPos` (via
`fullLine.indexOf(":", currentPos)`) finds the colon for `a: STRING` again (since `currentPos`
was not advanced past it). The diagnostic column for `short` is therefore reported at the wrong
position.

**Recommendation:** Advance `currentPos` unconditionally after processing each parameter,
regardless of whether a mismatch was found. A safe advance point is `colonPos + typeKeyword.length`.

---

### `namingValidator.js`

#### [CRITICAL] `require('vscode')` in core/ — line 6

**Issue:** `const vscode = require("vscode")` at line 6; used to construct `vscode.Range` at
lines 135, 168, and 183. This is the **ninth** violation.

**Recommendation:** Remove the `vscode` import. The Range values here are full-line spans
(`0` to `lineText.length`), which can be expressed as plain position objects without `vscode`.

---

#### [MINOR] Full-line diagnostic ranges for naming violations — lines 134, 168, 183

**Issue:** All three naming diagnostics (ACU-NMG-001, ACU-NMG-002, ACU-NMG-003) use a
full-line range (`lineIndex, 0` to `lineIndex, lineText.length`). The variable name that
triggered the diagnostic is known at that point (`variableName` from the loop). A full-line
underline is imprecise and makes it harder for the user to identify which token is at fault,
particularly on `VAR` lines that declare multiple variables.

**Recommendation:** Target the variable name token in the declaration line. Use a helper like
`buildTokenRange(lineText, lineIndex, variableName, 0)` (already available from `parser.js`).

---

#### [MINOR] Dead conditional in `stripExistingVariablePrefix` — lines 101–104

**Issue:** After the early-return guard at line 97 (`if (prefixLength === 0 || prefixLength > maxPrefixLength) return`),
the condition `if (prefixLength > 0 && prefixLength <= maxPrefixLength)` on line 102 is
always true — both clauses are guaranteed by the earlier guard. The `if` is dead and misleading.

**Recommendation:** Remove the `if` wrapper and use the `variableName = variableName.substring(prefixLength)` assignment unconditionally.

---

### `argumentStringValidator.js`

#### [CRITICAL] `require('vscode')` in core/ — line 7

**Issue:** `const vscode = require("vscode")` at line 7; used to construct `vscode.Range` at
lines 133, 147, 158, 188, 196, and 239. This is the **tenth** violation.

**Recommendation:** Remove the `vscode` import; replace with plain position objects.

---

#### [MINOR] `extractStringLiteralArg` only handles double-quoted strings — line 104

**Issue:** `extractStringLiteralArg` (line 104) requires the argument to start and end with `"`.
Single-quoted string literals are silently skipped. If Architect scripts pass single-quoted dates
or table synonyms, the validators in this file produce no diagnostic.

**Recommendation:** Extend the check to accept `'` as an alternative quote character, or
document explicitly in the JSDoc that single-quoted arguments are out of scope.

---

### `interpolationValidator.js`

#### [CRITICAL] `require('vscode')` in core/ — line 6

**Issue:** `const vscode = require("vscode")` at line 6; used to construct `vscode.Range` at
lines 60, 72, and 84. This is the **eleventh** violation.

**Recommendation:** Remove the `vscode` import; replace with plain position objects.

---

#### [MINOR] Silent no-op when `executionClassificationMap` is absent — line 39

**Issue:** Line 39: `if (!executionClassificationMap || !executionClassificationMap[lineIndex].isOutput)`
— when `executionClassificationMap` is `null`, the condition is always true and every line is
skipped. The function returns an empty array without any indication that validation was bypassed.

This is the mirror of the pattern in `keywordCapsValidator.js` (lines 23–29) where the map is
**optional** and skipping without it is the intended degraded mode. However, for interpolation
validation the map is not optional — without it the validator cannot know which lines are output
lines, and skipping is silently wrong rather than safely degraded.

The `macroValidator.js` finding from Batch 6 (Minor: output-line guard inactive when
`executionClassificationMap` not provided) describes the same pattern.

**Recommendation:** Add a guard at the top of `validateStringInterpolation` that returns early
with an empty array if `executionClassificationMap` is absent, or — better — add a `console.warn`
or log entry so the caller is made aware. Document in JSDoc that the map is required for this
validator to produce any output.

---

### `usagePatternValidator.js`

#### [CRITICAL] `require('vscode')` in core/ — line 11

**Issue:** `const vscode = require("vscode")` at line 11; used to construct `vscode.Range`
inside `createCallRange` at line 29. This is the **twelfth** violation.

**Recommendation:** Remove the `vscode` import; replace `new vscode.Range(...)` in
`createCallRange` with a plain position object.

---

#### [MINOR] `getFunctionScopeAtLine` called once per regex match, not once per line — line 102

**Issue:** The inner `while` loop (line 100) fires for every function-call pattern match on a
line. `getFunctionScopeAtLine(functionScopes, lineIndex)` at line 102 is called inside this
loop. A line with three function calls triggers three scope lookups for the same `lineIndex`.

This is the same redundancy flagged in Batch 5 (`collectDiagnostics.js:80`). The scope for a
given `lineIndex` cannot change mid-line.

**Recommendation:** Hoist the `getFunctionScopeAtLine` / `getScopeKey` / `getScopeRecord` calls
to before the inner `while` loop, assigning them once per `lineIndex`.

---

## Observations

### Lifecycle checks are count-based, not correspondence-based — `usagePatternValidator.js`

ACU-PAT-001 fires only when `close.length === 0` while cursor ops exist. If a scope opens two
indexes but closes only one, no diagnostic is produced. ACU-PAT-002 fires only when one of
save/restore count is zero and the other is nonzero — a 2:1 mismatch is silent. These are
inherent limitations of a single-pass line scan without handle tracking. The stale-handle
tracking (ACU-PAT-010) provides partial handle-aware coverage but is limited to the same scope.

This is a known design trade-off, not a bug. It warrants a comment in the source.

---

### No unit tests for naming convention logic — `namingValidator.js`

`namingValidator.js` has several configuration-dependent paths (VS Code workspace settings for
prefix characters), three distinct diagnostic rule IDs, and non-trivial prefix-stripping logic.
No syntax test cases cover naming convention violations. Given the complexity, this is the
highest-priority test gap in this batch.

---

### Hardcoded output delimiter default — `interpolationValidator.js`, line 25

`outputDelimiters` defaults to `{ oicc: "^", cicc: "^" }`. If the Architect runtime is
configured with different output delimiter characters, the interpolation targets will be parsed
incorrectly and all ACU-INT diagnostics will either miss or misfire. This default is baked into
the validator rather than being sourced from a configuration registry. Acceptable as a known
assumption, but should be documented.

---

### `typeKeywordCapsValidator.js` — case-insensitive regex vs. uppercase-only validation

Both `findVarTypeMatches` (line 71) and `findFunctionTypeMatches` (line 108) use the `i` flag
on their regexes so the outer keyword match is case-insensitive (it fires for `var`, `Var`,
`VAR`). The type check then applies `isKnownType` + `!== toUpperCase()`. This means the
validators run on all casing variants of the `VAR`/`FUNCTION` keyword, which is correct
behaviour — but the interaction is implicit and easily forgotten when modifying these regexes.

---

## Summary table

| Severity | Count | Items |
|---|---|---|
| Critical | 6 | `require('vscode')` in all 6 files |
| Major | 1 | `typeKeywordCapsValidator` param `currentPos` not advanced |
| Minor | 7 | Dead variable, regex rebuild, full-line ranges, dead conditional, double-quote-only, silent no-op, scope lookup in inner loop |
| Observation | 4 | Count-based lifecycle checks, no naming tests, hardcoded delimiter, implicit regex flag interaction |
