Sfoglia il codice sorgente

Diagnostics: record-field types end to end

- parser captures each field's declared type (outline/hover show it)
- checker resolves r.f, r.a.b, p^.f, a[i].f and F().f to real types,
  so field reads, writes and comparisons are type-checked
- new genuine diagnostics (both gm2-probed): selection on a known
  non-record (no implicit pointer dereference) and unknown fields
- hover/go-to-definition/rename land on field declarations
- post-call postfixes (F().x), guarded field-lookup recursion
Eric Streit 1 settimana fa
parent
commit
dc8afc8b31

+ 10 - 0
extensions/modula2-language/DIAGNOSTICS.md

@@ -80,6 +80,16 @@ issues on all six. Getting there exposed and fixed:
   procedure variables (calls and `P := Q` assignment), the pervasive
   `LENGTH`/`ORDL`, arithmetic that preserves `CARDINAL` (`c + 1`).
 
+## Record-field types
+
+Record declarations capture each field's type, so `r.f`, `r.a.b`,
+`p^.f`, `a[i].f` and `F().f` resolve to real types instead of `unknown`:
+field reads, writes and comparisons are checked, and hover/go-to-definition
+land on the field declaration. Genuine errors are reported too: selecting
+a field of a known non-record (`f.x` on a `POINTER` — Modula-2 does not
+implicitly dereference) and unknown fields (`r.nope`). Anything
+unresolvable stays silent.
+
 ## Circular dependencies (no use-before-declaration)
 
 gm2 is multi-pass, so declaration order never matters — there is

+ 135 - 48
extensions/modula2-language/src/m2/checker.ts

@@ -16,7 +16,8 @@ import { Token } from './lexer';
 import { M2Symbol, M2Unit } from './parser';
 import { resolveName } from './resolve';
 import {
-  M2Type, UNKNOWN, arithKind, arithResultType, displayType, isArrayArgCompatible, isAssignable,
+  M2Type, UNKNOWN, arithKind, arithResultType, displayType, findFieldInType,
+  findRecordField, isArrayArgCompatible, isAssignable,
   isComparable, isOrdinal, numberLiteralType, parseTypeText, resolveDeclaredType, resolveQualified,
   resolveType, returnTypeOf, unitOfFile,
 } from './types';
@@ -441,7 +442,12 @@ class Walker {
   private parseDesignatorRest(head: Token, mode: 'expr' | 'stmt'): Value {
     let info = this.resolveHead(head);
     let tok = head;
-    // Qualifier chain: M.X or r.field (unknown-typed beyond the head).
+    // Declaration behind the current value (for field selection).
+    let cur: { sym: M2Symbol; unit: M2Unit; dir: string } | null =
+      (info.kind === 'var' || info.kind === 'const')
+        ? { sym: info.sym, unit: info.host, dir: info.dir } : null;
+    let type = this.valueType(info, tok);
+    // Qualifier chain: M.X or r.field.
     while (this.atSym('.')) {
       this.next(); // .
       const nm = this.peek();
@@ -456,6 +462,8 @@ class Walker {
           info = PROC_BUILTINS.has(nm.text)
             ? { kind: 'builtinProc', name: nm.text }
             : { kind: 'unknown' };
+          cur = null;
+          type = UNKNOWN;
           continue;
         }
         switch (q.sym.kind) {
@@ -465,54 +473,18 @@ class Walker {
           case 'type': info = { kind: 'type' }; break;
           default: info = { kind: 'unknown' }; break;
         }
+        cur = (info.kind === 'var' || info.kind === 'const')
+          ? { sym: info.sym, unit: info.host, dir: info.dir } : null;
+        type = this.valueType(info, nm);
       } else {
+        const selected = this.selectField(cur, type, nm);
+        cur = selected.decl;
+        type = selected.type;
         info = { kind: 'field' };
       }
     }
-    // Postfix: indexing and dereferencing.
-    let type = this.valueType(info, tok);
-    for (;;) {
-      if (this.atSym('[')) {
-        const open = this.next()!;
-        const idxTypes = this.parseExprList();
-        if (this.atSym(']')) this.next();
-        // Consume one dimension per index: gm2 flattens `a[i, j]` over
-        // nested arrays (`symSet[0, i]` means `symSet[0][i]`).
-        let badIndex = false;
-        for (const it of idxTypes) {
-          if (type.kind === 'array') {
-            if (!badIndex && it.kind !== 'unknown' && !isOrdinal(it)) {
-              this.issue(open, `Array index must have an ordinal type, found ${displayType(it)}`);
-              badIndex = true;
-            }
-            type = type.element;
-          } else if (type.kind !== 'unknown') {
-            this.issue(open, `Cannot index ${displayType(type)}`);
-            type = UNKNOWN;
-            break;
-          }
-        }
-      } else if (this.atSym('^')) {
-        const op = this.next()!;
-        if (type.kind === 'pointer') {
-          type = type.target;
-        } else if (type.kind !== 'unknown') {
-          this.issue(op, `Cannot dereference ${displayType(type)}`);
-          type = UNKNOWN;
-        }
-      } else if (this.atSym('.')) {
-        // Field selection after indexing or dereferencing (`f^.x`, `a[i].x`).
-        // (Direct `r.x` is consumed by the qualifier chain above.)
-        this.next(); // .
-        const nm = this.peek();
-        if (nm?.kind === 'ident') this.next();
-        // Field types are untracked: any structured value yields unknown.
-        type = UNKNOWN;
-        info = { kind: 'field' };
-      } else {
-        break;
-      }
-    }
+    // Postfix: indexing, dereferencing and field selection.
+    ({ info, tok, type } = this.parsePostfix(info, tok, type));
     // Set constructor with explicit type (`BITSET{...}`, `MySet{...}`).
     if ((info.kind === 'type' || info.kind === 'builtinType') && this.atSym('{')) {
       this.next();
@@ -523,9 +495,16 @@ class Walker {
     if (this.atSym('(')) {
       const call = this.parseCall(info, tok);
       if (mode === 'stmt') {
-        if (call.returnsValue) {
+        // `F().x := ...` assigns into a call temporary (accepted by gm2):
+        // only a bare `F();` ignores its result.
+        if (call.returnsValue && !this.atPostfixStart()) {
           this.issue(tok, `Return value of function "${nameOf(info, tok)}" is ignored`);
         }
+        if (this.atPostfixStart()) {
+          const rest = this.parsePostfix(info, tok, UNKNOWN);
+          info = rest.info;
+          tok = rest.tok;
+        }
         return { type: UNKNOWN, info, tok };
       }
       if (call.isVoid) {
@@ -533,6 +512,9 @@ class Walker {
         return { type: UNKNOWN, info, tok };
       }
       type = call.type;
+      // Postfix after a call result (`F().x`, `a[i].f` already handled above).
+      ({ info, tok, type } = this.parsePostfix(info, tok, type));
+      return { type, info, tok };
     } else if (mode === 'expr') {
       if (info.kind === 'proc') {
         // A bare procedure name on the right of a `P := Q` assignment
@@ -563,6 +545,110 @@ class Walker {
     return { type, info, tok };
   }
 
+  /** Type of `.name` selected on a value, with declaration tracking.
+   *
+   *  `decl` carries the declaration behind the base value when known (a
+   *  variable/constant, or a previously selected field); `base` is its
+   *  computed type. Returns the field type (or UNKNOWN) plus the field's
+   *  own declaration for chained selection. Genuine errors are reported:
+   *  selection on a known non-record, or an absent field of a known
+   *  record (gm2 rejects both); anything unknowable stays silent.
+   */
+  private selectField(
+    decl: { sym: M2Symbol; unit: M2Unit; dir: string } | null,
+    base: M2Type, nm: Token,
+  ): { type: M2Type; decl: { sym: M2Symbol; unit: M2Unit; dir: string } | null } {
+    const stripped = base.kind === 'subrange' ? base.base : base;
+    if (decl) {
+      const found = findRecordField(decl.unit, decl.dir, decl.sym, nm.text, nm.line, nm.ch);
+      if (found) {
+        return {
+          type: resolveDeclaredType(found.sym, found.unit, found.dir, nm.line, nm.ch),
+          decl: { sym: found.sym, unit: found.unit, dir: found.dir },
+        };
+      }
+      if (stripped.kind === 'record') {
+        this.issue(nm, `Record "${stripped.name ?? '?'}" has no field "${nm.text}"`);
+        return { type: UNKNOWN, decl: null };
+      }
+      if (stripped.kind !== 'unknown') {
+        this.issue(nm, `Type ${displayType(stripped)} is not a record`);
+        return { type: UNKNOWN, decl: null };
+      }
+      return { type: UNKNOWN, decl: null };
+    }
+    const found = findFieldInType(this.ctx.unit, this.ctx.docDir, base, nm.text, nm.line, nm.ch);
+    if (found) {
+      return {
+        type: resolveDeclaredType(found.sym, found.unit, found.dir, nm.line, nm.ch),
+        decl: { sym: found.sym, unit: found.unit, dir: found.dir },
+      };
+    }
+    if (stripped.kind !== 'unknown' && stripped.kind !== 'record') {
+      this.issue(nm, `Type ${displayType(stripped)} is not a record`);
+      return { type: UNKNOWN, decl: null };
+    }
+    if (stripped.kind === 'record') {
+      this.issue(nm, `Record "${stripped.name ?? '?'}" has no field "${nm.text}"`);
+    }
+    return { type: UNKNOWN, decl: null };
+  }
+
+  private atPostfixStart(): boolean {
+    return this.atSym('[') || this.atSym('^') || this.atSym('.');
+  }
+
+  /** Indexing, dereferencing and field selection after `^`/`[]`
+   *  (direct `r.x` is consumed by the qualifier chain instead). */
+  private parsePostfix(
+    info: HeadInfo, tok: Token, type: M2Type,
+  ): { info: HeadInfo; tok: Token; type: M2Type } {
+    for (;;) {
+      if (this.atSym('[')) {
+        const open = this.next()!;
+        const idxTypes = this.parseExprList();
+        if (this.atSym(']')) this.next();
+        // Consume one dimension per index: gm2 flattens `a[i, j]` over
+        // nested arrays (`symSet[0, i]` means `symSet[0][i]`).
+        let badIndex = false;
+        for (const it of idxTypes) {
+          if (type.kind === 'array') {
+            if (!badIndex && it.kind !== 'unknown' && !isOrdinal(it)) {
+              this.issue(open, `Array index must have an ordinal type, found ${displayType(it)}`);
+              badIndex = true;
+            }
+            type = type.element;
+          } else if (type.kind !== 'unknown') {
+            this.issue(open, `Cannot index ${displayType(type)}`);
+            type = UNKNOWN;
+            break;
+          }
+        }
+      } else if (this.atSym('^')) {
+        const op = this.next()!;
+        if (type.kind === 'pointer') {
+          type = type.target;
+        } else if (type.kind !== 'unknown') {
+          this.issue(op, `Cannot dereference ${displayType(type)}`);
+          type = UNKNOWN;
+        }
+      } else if (this.atSym('.')) {
+        this.next(); // .
+        const nm = this.peek();
+        if (!nm || nm.kind !== 'ident') break;
+        this.next();
+        tok = nm;
+        // Only the computed type is known here (no declaration).
+        const selected = this.selectField(null, type, nm);
+        type = selected.type;
+        info = { kind: 'field' };
+      } else {
+        break;
+      }
+    }
+    return { info, tok, type };
+  }
+
   private valueType(info: HeadInfo, tok: Token): M2Type {
     switch (info.kind) {
       case 'var': return resolveDeclaredType(info.sym, info.host, info.dir, tok.line, tok.ch);
@@ -672,7 +758,8 @@ class Walker {
       this.issue(d.tok, `"${d.tok.text}" is a module and cannot be assigned to`);
       return;
     }
-    if (info.kind === 'unknown' || info.kind === 'field' || info.kind === 'builtinProc') return;
+    if (info.kind === 'unknown' || info.kind === 'builtinProc') return;
+    // Variables and record fields check against their declared types.
     const lhs = d.type;
     if (lhs.kind !== 'unknown' && rhs.type.kind !== 'unknown' && !isAssignable(lhs, rhs.type)) {
       this.issue(op, `Cannot assign ${displayType(rhs.type)} to ${displayType(lhs)}`);

+ 53 - 6
extensions/modula2-language/src/m2/parser.ts

@@ -305,7 +305,7 @@ class Parser {
     let recDepth = 0;    // RECORD..END nesting
     let roundDepth = 0;  // ( ) only, for enumeration detection
     let prevSignificant: Token | null = null;
-    const fieldCands: Token[] = [];
+    const fieldCands: Array<{ tok: Token; idx: number }> = [];
     const topIdents: Token[] = [];
     let enumOk = true;
     let firstTok: Token | null = null;
@@ -346,7 +346,7 @@ class Parser {
         const afterCase = prevSignificant !== null && prevSignificant.kind === 'keyword' &&
           (prevSignificant.text === 'CASE' || prevSignificant.text === 'OF');
         if (!afterCase && nx.kind === 'symbol' && (nx.text === ',' || nx.text === ':')) {
-          fieldCands.push(t);
+          fieldCands.push({ tok: t, idx: this.pos });
         }
       } else if (t.kind === 'ident') {
         if (roundDepth === 1 && recDepth === 0 && depth <= 1) topIdents.push(t);
@@ -360,11 +360,12 @@ class Parser {
       if (t.kind !== 'eof') prevSignificant = t;
       this.next();
     }
-    // Promote candidates that precede ':' (name lists) to fields.
-    for (let k = 0; k < fieldCands.length; k++) {
-      const t = fieldCands[k];
+    // Promote candidates that precede ':' (name lists) to fields, with types.
+    for (const { tok: t, idx } of fieldCands) {
+      const typeText = this.fieldTypeText(idx);
       fields.push({
-        name: t.text, kind: 'field', detail: t.text,
+        name: t.text, kind: 'field',
+        detail: typeText ? `${t.text} : ${typeText}` : t.text,
         nameRange: rangeOf(t), extent: rangeOf(t), children: [],
       });
     }
@@ -375,6 +376,52 @@ class Parser {
     return { text, fields, enumLiterals: isEnum ? topIdents : [] };
   }
 
+  /** Type text of a record field (`x` in `x, y: INTEGER;`), or '' when unclear.
+   *
+   *  Bails (returns '') on a second top-level colon, which marks variant
+   *  case labels (`OF red: x: INTEGER`) rather than real fields.
+   */
+  private fieldTypeText(fieldIdx: number): string {
+    let j = fieldIdx;
+    for (;;) {
+      const comma = this.tokens[j + 1];
+      const after = this.tokens[j + 2];
+      if (comma?.kind === 'symbol' && comma.text === ',' && after?.kind === 'ident') {
+        j += 2;
+        continue;
+      }
+      break;
+    }
+    const colon = this.tokens[j + 1];
+    if (!colon || colon.kind !== 'symbol' || colon.text !== ':') return '';
+    let depth = 0;
+    let k = j + 2;
+    const start = k;
+    while (k < this.tokens.length) {
+      const u = this.tokens[k];
+      if (u.kind === 'eof') break;
+      if (u.kind === 'symbol') {
+        if (u.text === '(' || u.text === '[' || u.text === '{') depth++;
+        else if (u.text === ')' || u.text === ']' || u.text === '}') {
+          if (depth === 0) break;
+          depth--;
+        } else if (u.text === ';' || u.text === '|') {
+          if (depth === 0) break;
+        } else if (u.text === ':' && depth === 0) {
+          return ''; // variant label or nested declaration, not a plain field
+        }
+      } else if (u.kind === 'keyword') {
+        // NOTE: no OF here — it belongs to ARRAY OF / SET OF / CASE OF.
+        if (depth === 0 && (u.text === 'END' || u.text === 'ELSE' || u.text === 'CASE' ||
+            u.text === 'CONST' || u.text === 'TYPE' ||
+            u.text === 'VAR' || u.text === 'PROCEDURE' || u.text === 'MODULE' ||
+            u.text === 'BEGIN' || u.text === 'FINALLY')) break;
+      }
+      k++;
+    }
+    return this.rawRange(start, k).trim().replace(/\s+/g, ' ');
+  }
+
   private skipAlignment(): void {
     if (this.atSym('<*')) {
       this.next();

+ 7 - 0
extensions/modula2-language/src/m2/resolve.ts

@@ -8,6 +8,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import { lex } from './lexer';
 import { M2Import, M2Symbol, M2Unit, parseUnitText, rangeContains } from './parser';
+import { findRecordField } from './types';
 
 export interface M2Location {
   filePath: string;
@@ -217,6 +218,12 @@ export function resolveName(
   qualifier: string | null, name: string,
 ): Resolved | null {
   if (qualifier) {
+    // Record field on a local variable (`r.f`): enables hover/definition.
+    const head = resolveLocal(unit, line, ch, qualifier);
+    if (head && (head.kind === 'variable' || head.kind === 'parameter' || head.kind === 'constant')) {
+      const f = findRecordField(unit, docDir, head, name, line, ch);
+      if (f) return { sym: f.sym, filePath: f.unit.filePath };
+    }
     const imp: M2Import | undefined = unit.imports.find(i => i.module === qualifier);
     const fromFile = imp ? imp.module : qualifier;
     const def = loadDefModule(docDir, fromFile);

+ 85 - 2
extensions/modula2-language/src/m2/types.ts

@@ -20,7 +20,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import { lex, Token } from './lexer';
 import { M2Symbol, M2Unit, parseUnitText } from './parser';
-import { loadDefModule, resolveName, topLevel } from './resolve';
+import { loadDefModule, ownDefUnit, resolveName, topLevel } from './resolve';
 
 export type M2Type =
   | { kind: 'unknown' }
@@ -284,7 +284,7 @@ export function resolveDeclaredType(
     // Enumeration literal recorded as `Type.Literal`.
     return { kind: 'enum', name: detail.slice(0, detail.indexOf('.')) };
   }
-  if (sym.kind === 'variable' || sym.kind === 'parameter') {
+  if (sym.kind === 'variable' || sym.kind === 'parameter' || sym.kind === 'field') {
     const colon = detail.indexOf(':');
     if (colon < 0) return UNKNOWN;
     return resolveT(parseTypeText(detail.slice(colon + 1)), hostUnit, hostDir, line, ch, new Set());
@@ -581,6 +581,89 @@ export function displayType(t: M2Type): string {
   }
 }
 
+/** A record field's declaration: `{ sym, unit, dir }` for typing and navigation.
+ *
+ *  Resolves `r.f` where `r` is a variable/parameter/constant of record type.
+ *  Returns null when the base is not a record or the field is absent
+ *  (callers decide whether that is an error).
+ */
+/** Re-entrancy guard: type qualifiers naming variables (`VAR r: r.S`)
+ *  are pathological; resolveName can otherwise loop back in here forever.
+ */
+const activeFieldLookups = new Set<string>();
+
+export function findRecordField(
+  hostUnit: M2Unit, hostDir: string, hostSym: M2Symbol,
+  fieldName: string, line: number, ch: number,
+): { sym: M2Symbol; unit: M2Unit; dir: string } | null {
+  const key = `${hostUnit.filePath}:${hostSym.nameRange.startLine}:${hostSym.nameRange.startCh}.${fieldName}`;
+  if (activeFieldLookups.has(key)) return null;
+  activeFieldLookups.add(key);
+  try {
+    return findRecordFieldInner(hostUnit, hostDir, hostSym, fieldName, line, ch);
+  } finally {
+    activeFieldLookups.delete(key);
+  }
+}
+
+function findRecordFieldInner(
+  hostUnit: M2Unit, hostDir: string, hostSym: M2Symbol,
+  fieldName: string, line: number, ch: number,
+): { sym: M2Symbol; unit: M2Unit; dir: string } | null {
+  const base = resolveDeclaredType(hostSym, hostUnit, hostDir, line, ch);
+  if (base.kind === 'unknown') return null;
+  if (base.kind === 'record' && !base.name) {
+    // Anonymous inline record: fields attached to the host symbol itself.
+    const own = hostSym.children.find(c => c.name === fieldName);
+    return own ? { sym: own, unit: hostUnit, dir: hostDir } : null;
+  }
+  return findFieldInType(hostUnit, hostDir, base, fieldName, line, ch);
+}
+
+/** Field lookup by an already-computed base type (no host symbol needed).
+ *
+ *  The record is resolved scope-correctly (nested and local-module types
+ *  included), so positions always belong to the analysed document.
+ */
+/** Maximum nested field-type resolutions (pathological `r.T` qualifiers). */
+let fieldDepth = 0;
+
+export function findFieldInType(
+  hostUnit: M2Unit, hostDir: string, base: M2Type, fieldName: string,
+  line: number, ch: number,
+): { sym: M2Symbol; unit: M2Unit; dir: string } | null {
+  const t = base.kind === 'subrange' ? base.base : base;
+  if (t.kind !== 'record' || !t.name) return null;
+  if (++fieldDepth > 25) {
+    fieldDepth--;
+    return null;
+  }
+  const key = `${hostUnit.filePath}:${t.name}.${fieldName}`;
+  if (activeFieldLookups.has(key)) {
+    fieldDepth--;
+    return null;
+  }
+  activeFieldLookups.add(key);
+  try {
+    const dot = t.name.lastIndexOf('.');
+    const qualifier = dot >= 0 ? t.name.slice(0, dot) : null;
+    const typeName = dot >= 0 ? t.name.slice(dot + 1) : t.name;
+    const r = resolveName(hostUnit, hostDir, line, ch, qualifier, typeName);
+    if (!r || r.sym.kind !== 'type') return null;
+    const declUnit = r.filePath === hostUnit.filePath ? hostUnit : unitOfFile(r.filePath);
+    if (!declUnit) return null;
+    const f = r.sym.children.find(c => c.name === fieldName);
+    if (!f) return null;
+    return {
+      sym: f, unit: declUnit,
+      dir: r.filePath === hostUnit.filePath ? hostDir : path.dirname(r.filePath),
+    };
+  } finally {
+    activeFieldLookups.delete(key);
+    fieldDepth--;
+  }
+}
+
 /** Resolve an imported qualified name (`M.X`) to its declaration for typing. */
 export function resolveQualified(
   unit: M2Unit, docDir: string, module: string, name: string,