# F-02 — returnTypeValidator.js: RETURN detection by regex + duplicated type inference

**Severity:** Major  
**File:** `core/src/validators/returnTypeValidator.js`  
**Category:** Validator compensating for incomplete AST use; duplicated type inference  
**Remediation appropriate:** Yes — Tier 1

---

## Context

`returnTypeValidator.js` validates that RETURN expressions match the declared return type of their
enclosing function, and that typed functions contain at least one RETURN statement. Both checks are
currently driven by regex on raw line text rather than AST nodes.

---

## Finding 1 — `RETURN_RE` compensates for AST `ReturnStatement` nodes

**Lines:** 20, 85–90

```javascript
const RETURN_RE = /^\s*#?\s*RETURN\b(.*)/i;

// In the validation loop:
for (let lineIndex = startLine + 1; lineIndex < endLine; lineIndex++) {
    const lineText = document.lineAt(lineIndex).text;
    const effective = stripTrailingComment(lineText).trim();
    const match = effective.match(RETURN_RE);
    if (!match) continue;

    hasReturn = true;
    const exprText = match[1].trim();
```

**What the AST already provides:**
The AST parser produces `ReturnStatement` nodes:
```javascript
{
    type: NodeType.ReturnStatement,
    value: TokenSpan | null,   // null = bare RETURN
    line: number
}
```

These are present in the body of `FunctionDeclaration` nodes. The `functionScopes` built by
`documentFacts.js` already have `startLine` and `endLine`. What is missing is that the
function body statements are not surfaced through facts — they remain in the AST which is
accessible via `buildDocumentAst(document)`.

**The fix:** The validator already imports nothing from the document-as-text for finding RETURN
positions — it only needs the AST. The simplest change:

```javascript
const { buildDocumentAst } = require("../ast/documentAst");
const { NodeType } = require("../ast/nodes");

// In validateReturnTypes:
const { ast } = buildDocumentAst(document);
// ... walk function body statements looking for ReturnStatement nodes
```

Because `buildDocumentAst` is memoised, this adds no I/O cost — the cache is already warm
from `documentFacts.js` having called it first.

The AST `ReturnStatement.line` gives the exact line number. `ReturnStatement.value` is the
expression TokenSpan. No regex, no raw line access, no `stripTrailingComment`.

---

## Finding 2 — `inferReturnExpressionType()` duplicates the expression parser's type inference

**Lines:** 31–61

```javascript
function inferReturnExpressionType(exprText, params, locals) {
    // String literal
    if (/^"[^"]*"$/.test(exprText) || /^'[^']*'$/.test(exprText)) return "STRING";
    // Decimal → DOUBLE
    if (/^-?\d+\.\d+$/.test(exprText)) return "DOUBLE";
    // Integer → SHORT
    if (/^-?\d+$/.test(exprText)) return "SHORT";
    // Simple variable — look up in params/locals
    if (/^[A-Za-z_][A-Za-z0-9_]*$/.test(exprText)) { ... }
    return null;
}
```

This is a manual reimplementation of what `expressionParser.js` already provides:

| Construct | Regex pattern | Expression parser equivalent |
|---|---|---|
| String literal | `/^"[^"]*"$/` | `makeLiteral(tok, AcurityTypes.STRING)` |
| Decimal | `/^-?\d+\.\d+$/` | `inferNumericType(value)` → `AcurityTypes.DOUBLE` |
| Integer | `/^-?\d+$/` | `inferNumericType(value)` → `AcurityTypes.SHORT` |
| Identifier | `/^[A-Za-z_][A-Za-z0-9_]*$/` | `makeIdentifier(tok)` → `ExprNodeType.Identifier` |

**Problem with the regex approach:** The regex patterns are incomplete and slightly incorrect:
- `/^-?\d+$/` matches `SHORT` but the actual type should be `SHORT`, `LONG`, or `BIGINT`
  depending on magnitude (per `inferNumericType` in expressionParser). The validator always
  returns `SHORT` for any integer, which will produce incorrect type-mismatch diagnostics for
  functions that return `LONG x = 100000`.
- The patterns do not handle compound expressions (e.g. `a + 1`) or function calls — these
  return `null` (skip), which is correct in intent but fragile.

**The fix:**

```javascript
const { parseExpression } = require("../ast/expressionParser");
const { ExprNodeType } = require("../ast/expressionNodes");
const { AcurityTypes } = require("../types/acurityTypes");

function getReturnExpressionType(returnStmt, params, locals) {
    if (!returnStmt.value) return null;
    const expr = parseExpression(returnStmt.value);
    if (!expr) return null;

    if (expr.type === ExprNodeType.Literal) return expr.inferredType;

    if (expr.type === ExprNodeType.Unary && expr.operator === "-") {
        // -literal: propagate inner type
        if (expr.operand?.type === ExprNodeType.Literal) return expr.operand.inferredType;
    }

    if (expr.type === ExprNodeType.Identifier) {
        const upper = expr.name.toUpperCase();
        for (const [k, v] of params) { if (k.toUpperCase() === upper) return v.toUpperCase(); }
        for (const [k, v] of locals) { if (k.toUpperCase() === upper) return v.toUpperCase(); }
    }

    return null;
}
```

This eliminates all regex patterns, correctly handles `LONG`/`BIGINT` integer magnitudes, and
is consistent with how the rest of the expression pipeline works.

---

## Finding 3 — Column position via `lineText.search()` (minor, in same file)

**Lines:** 93, 111**

```javascript
const col = lineText.search(/\bRETURN\b/i);
// and
const col = lineText.search(/\bENDFUNCTION\b/i);
```

These use regex on raw line text to find the column of the keyword. With AST nodes:
- `ReturnStatement.line` is already known; the column is the char position of the first
  token in `ReturnStatement.value.start.char` or, for the RETURN keyword itself, the position
  of the RETURN token in the token stream.
- `EndFunctionStatement.line` is known; its column can be found from the token stream.

This is minor because `lineText.search(/\bRETURN\b/i)` is accurate for normal code (RETURN
won't appear in string content on the same line, and the `search` finds the first match which
is the keyword). But it is inconsistent with the rest of the codebase's move toward token-based
positions.

**Recommendation:** Accept as-is for now; fix when the RETURN_RE migration is done
(the token position will naturally be available from the AST node).

---

## Summary

| Finding | Current approach | Better approach | Tier |
|---|---|---|---|
| RETURN detection | `RETURN_RE` regex on raw line text | AST `ReturnStatement` nodes | 1 |
| Type inference | Regex patterns in `inferReturnExpressionType` | `parseExpression` → ExprNode | 1 |
| Column positions | `lineText.search(/\bRETURN\b/)` | Token positions from AST nodes | follows from above |
