Browse Source

Semantic diagnostics: duplicates, unknown identifiers, enum literals

- new m2/analyse.ts: duplicate declarations per scope and unknown
  identifiers with conservative skips (qualified names, imports,
  WITH regions, unverifiable FROM-imports, builtins, END names)
- parser records import ranges and exposes (A, B, ...) literals as
  constants; resolveLocal/completion see them
- server validate() merges local modula2 diagnostics with Coco/R output
Eric Streit 1 tuần trước cách đây
mục cha
commit
329f229610

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

@@ -19,3 +19,20 @@ The diagnostics are published through the VS Code-compatible
 
 
 The next stage will replace the structural checks with diagnostics generated by
 The next stage will replace the structural checks with diagnostics generated by
 the Coco/R parser, while keeping the same editor diagnostic mechanism.
 the Coco/R parser, while keeping the same editor diagnostic mechanism.
+
+## Server-side semantic layer (`src/m2/analyse.ts`, source `modula2`)
+
+In parallel, the language server runs its own checks on every open
+document — always on, merged with Coco/R output when a validator is
+configured:
+
+- duplicate declarations in one scope (error, with first-declared line);
+- unknown identifiers (error), with conservative skips: qualified names,
+  import clauses, `WITH` regions, unverifiable FROM-imports, predefined
+  identifiers, closing `END Name`;
+- enumeration literals `(A, B, ...)` are treated as declared constants, so
+  they resolve, complete, and appear in the outline.
+
+Not covered (needs statement/expression parsing): assignment type
+compatibility, call arity, use-before-declaration order, FROM-list membership
+against the defining `.def`.

+ 146 - 0
extensions/modula2-language/src/m2/analyse.ts

@@ -0,0 +1,146 @@
+/** Best-effort semantic checks over parsed Modula-2 units.
+ *
+ *  Works on the lenient declaration parser: statement and expression bodies
+ *  are not parsed, so checks are limited to declaration structure and
+ *  identifier usage. The guiding rule is zero false positives — uncertain
+ *  cases are skipped, never flagged:
+ *  - qualified names (`M.X`, `r.field`) are never flagged;
+ *  - identifiers inside `WITH` regions are skipped (unqualified field access);
+ *  - FROM-imported names are skipped when the defining `.def` is unavailable;
+ *  - predefined identifiers (`INTEGER`, `INC`, `NIL`, ...) are always accepted.
+ *
+ *  Full type checking (assignment compatibility, call arity) needs statement
+ *  and expression parsing and is deliberately out of scope here.
+ */
+
+import { lex, Token } from './lexer';
+import { M2Symbol, M2Unit, parseUnitText, rangeContains } from './parser';
+import { flattenUnit, resolveName } from './resolve';
+
+export interface M2Issue {
+  line: number; ch: number; endLine: number; endCh: number;
+  message: string;
+  severity: 'error' | 'warning';
+}
+
+/** Predefined identifiers available without declaration (GNU/PIM/ISO common core). */
+const BUILTINS = new Set([
+  // Types
+  'INTEGER', 'CARDINAL', 'LONGINT', 'SHORTINT', 'LONGCARD', 'SHORTCARD',
+  'REAL', 'LONGREAL', 'CHAR', 'BOOLEAN', 'BITSET', 'ADDRESS', 'WORD', 'BYTE',
+  'OCTET',
+  // Constants
+  'NIL', 'TRUE', 'FALSE',
+  // Procedures and functions
+  'ABS', 'ADR', 'CAP', 'CHR', 'DEC', 'DISPOSE', 'EXCL', 'FLOAT', 'HALT',
+  'HIGH', 'INC', 'INCL', 'LFLOAT', 'MAX', 'MIN', 'NEW', 'ODD', 'ORD',
+  'SIZE', 'TRUNC', 'TSIZE', 'VAL', 'CODE',
+]);
+
+const OPENERS = new Set(['BEGIN', 'IF', 'CASE', 'LOOP', 'WHILE', 'FOR', 'WITH', 'RECORD', 'MODULE']);
+
+export function analyseUnit(text: string, filePath: string, docDir: string): M2Issue[] {
+  const unit = parseUnitText(text, filePath);
+  return [
+    ...duplicateDeclarations(unit),
+    ...unknownIdentifiers(text, unit, docDir),
+  ];
+}
+
+/** Same name declared twice in one scope (Modula-2 has no overloading). */
+function duplicateDeclarations(unit: M2Unit): M2Issue[] {
+  const issues: M2Issue[] = [];
+  const scopes = new Map<M2Symbol | null, Map<string, M2Symbol>>();
+  for (const { sym, chain } of flattenUnit(unit)) {
+    if (!sym.name) continue;
+    // The file's own module symbol is the root scope, not a declaration.
+    if (sym.kind === 'module' && chain.length === 0) continue;
+    const parent = chain[chain.length - 1] ?? null;
+    // Enumeration literals live in the scope enclosing their type.
+    const scope = sym.kind === 'constant' && parent?.kind === 'type'
+      ? chain[chain.length - 2] ?? null
+      : parent;
+    let seen = scopes.get(scope);
+    if (!seen) { seen = new Map(); scopes.set(scope, seen); }
+    const first = seen.get(sym.name);
+    if (first) {
+      issues.push({
+        line: sym.nameRange.startLine, ch: sym.nameRange.startCh,
+        endLine: sym.nameRange.endLine, endCh: sym.nameRange.endCh,
+        message: `Duplicate declaration "${sym.name}" (first declared at line ${first.nameRange.startLine + 1})`,
+        severity: 'error',
+      });
+    } else {
+      seen.set(sym.name, sym);
+    }
+  }
+  return issues;
+}
+
+/** Identifier uses that resolve to no visible declaration. */
+function unknownIdentifiers(text: string, unit: M2Unit, docDir: string): M2Issue[] {
+  const issues: M2Issue[] = [];
+  const tokens = lex(text);
+  const declared = new Set<string>();
+  for (const { sym } of flattenUnit(unit)) {
+    declared.add(`${sym.nameRange.startLine}:${sym.nameRange.startCh}`);
+  }
+  const fromNames = new Set<string>();
+  for (const imp of unit.imports) for (const n of imp.names) fromNames.add(n);
+  const withRegions = withRegionsOf(tokens);
+  for (let k = 0; k < tokens.length; k++) {
+    const t = tokens[k];
+    if (t.kind !== 'ident') continue;
+    const prev = tokens[k - 1];
+    const next = tokens[k + 1];
+    // Qualified access (`M.X`, `r.field`): unverifiable without deeper analysis.
+    if (prev?.text === '.' || next?.text === '.') continue;
+    // Declaration sites.
+    if (declared.has(`${t.line}:${t.ch}`)) continue;
+    // Import clauses.
+    if (unit.imports.some(imp => rangeContains(imp.range, t.line, t.ch))) continue;
+    // WITH regions (unqualified record field access).
+    if (withRegions.some(([a, b]) => a <= t.offset && t.endOffset <= b)) continue;
+    // FROM-imported: unverifiable when the defining `.def` is unavailable.
+    if (fromNames.has(t.text)) continue;
+    if (BUILTINS.has(t.text)) continue;
+    // Closing `END Name`.
+    if (prev?.kind === 'keyword' && prev.text === 'END') continue;
+    const resolved = resolveName(unit, docDir, t.line, t.ch, null, t.text);
+    if (!resolved) {
+      issues.push({
+        line: t.line, ch: t.ch, endLine: t.endLine, endCh: t.endCh,
+        message: `Unknown identifier "${t.text}"`,
+        severity: 'error',
+      });
+    }
+  }
+  return issues;
+}
+
+/** Offset spans of `WITH ... DO ... END` blocks (conservative bracket matching). */
+function withRegionsOf(tokens: Token[]): Array<[number, number]> {
+  const regions: Array<[number, number]> = [];
+  for (let k = 0; k < tokens.length; k++) {
+    const t = tokens[k];
+    if (t.kind !== 'keyword' || t.text !== 'WITH') continue;
+    let doIdx = -1;
+    for (let j = k + 1; j < Math.min(k + 13, tokens.length); j++) {
+      const u = tokens[j];
+      if (u.kind === 'keyword' && u.text === 'DO') { doIdx = j; break; }
+      if ((u.kind === 'symbol' && u.text === ';') ||
+          (u.kind === 'keyword' && (u.text === 'BEGIN' || u.text === 'END'))) break;
+    }
+    if (doIdx < 0) continue;
+    let depth = 1;
+    for (let m = doIdx + 1; m < tokens.length; m++) {
+      const u = tokens[m];
+      if (u.kind === 'keyword' && OPENERS.has(u.text)) depth++;
+      else if (u.kind === 'keyword' && u.text === 'END') {
+        depth--;
+        if (depth === 0) { regions.push([t.offset, u.endOffset]); break; }
+      }
+    }
+  }
+  return regions;
+}

+ 45 - 10
extensions/modula2-language/src/m2/parser.ts

@@ -37,6 +37,8 @@ export interface M2Import {
   names: string[];
   names: string[];
   /** True for plain `IMPORT m` (all exports visible as m.X). */
   /** True for plain `IMPORT m` (all exports visible as m.X). */
   all: boolean;
   all: boolean;
+  /** Source range of the whole IMPORT/FROM clause. */
+  range: M2Range;
 }
 }
 
 
 export interface M2Unit {
 export interface M2Unit {
@@ -153,23 +155,27 @@ class Parser {
   private parseImportSeq(unit: M2Unit): void {
   private parseImportSeq(unit: M2Unit): void {
     for (;;) {
     for (;;) {
       if (this.atKw('FROM')) {
       if (this.atKw('FROM')) {
-        this.next();
+        const kw = this.next();
         const m = this.expectIdent();
         const m = this.expectIdent();
         if (!m || !this.eatKw('IMPORT')) { this.syncDecl(); continue; }
         if (!m || !this.eatKw('IMPORT')) { this.syncDecl(); continue; }
         const names = this.parseIdentList();
         const names = this.parseIdentList();
         this.eatSym(';');
         this.eatSym(';');
-        if (m) unit.imports.push({ module: m.text, names, all: false });
+        if (m) unit.imports.push({ module: m.text, names, all: false, range: this.span(kw, this.prevTok()) });
       } else if (this.atKw('IMPORT')) {
       } else if (this.atKw('IMPORT')) {
-        this.next();
+        const kw = this.next();
         const names = this.parseIdentList();
         const names = this.parseIdentList();
         this.eatSym(';');
         this.eatSym(';');
-        for (const n of names) unit.imports.push({ module: n, names: [], all: true });
+        for (const n of names) unit.imports.push({ module: n, names: [], all: true, range: this.span(kw, this.prevTok()) });
       } else {
       } else {
         return;
         return;
       }
       }
     }
     }
   }
   }
 
 
+  private span(a: Token, b: Token): M2Range {
+    return { startLine: a.line, startCh: a.ch, endLine: b.endLine, endCh: b.endCh };
+  }
+
   private parseIdentList(): string[] {
   private parseIdentList(): string[] {
     const names: string[] = [];
     const names: string[] = [];
     for (;;) {
     for (;;) {
@@ -230,12 +236,21 @@ class Parser {
     while (this.peek().kind === 'ident') {
     while (this.peek().kind === 'ident') {
       const nm = this.next();
       const nm = this.next();
       if (this.eatSym('=')) {
       if (this.eatSym('=')) {
-        const { text, fields } = this.captureType();
+        const { text, fields, enumLiterals } = this.captureType();
         this.skipAlignment();
         this.skipAlignment();
-        parent.children.push({
+        const typeSym: M2Symbol = {
           name: nm.text, kind: 'type', detail: `${nm.text} = ${text}`,
           name: nm.text, kind: 'type', detail: `${nm.text} = ${text}`,
           nameRange: rangeOf(nm), extent: rangeOf(nm), children: fields,
           nameRange: rangeOf(nm), extent: rangeOf(nm), children: fields,
-        });
+        };
+        // Enumeration literals live in the type's scope: expose them so
+        // completion/definition/references and the analyser can see them.
+        for (const lit of enumLiterals) {
+          typeSym.children.push({
+            name: lit.text, kind: 'constant', detail: `${nm.text}.${lit.text}`,
+            nameRange: rangeOf(lit), extent: rangeOf(lit), children: [],
+          });
+        }
+        parent.children.push(typeSym);
       }
       }
       if (!this.eatSym(';')) this.syncDecl();
       if (!this.eatSym(';')) this.syncDecl();
     }
     }
@@ -268,26 +283,37 @@ class Parser {
     }
     }
   }
   }
 
 
-  /** Capture a type expression up to ';' at depth 0; collect RECORD fields. */
-  private captureType(): { text: string; fields: M2Symbol[] } {
+  /** Capture a type expression up to ';' at depth 0; collect RECORD fields.
+   *  A type consisting solely of `(A, B, ...)` additionally yields the
+   *  enumeration literals so they can be treated as declared constants. */
+  private captureType(): { text: string; fields: M2Symbol[]; enumLiterals: Token[] } {
     const startIdx = this.pos;
     const startIdx = this.pos;
     const fields: M2Symbol[] = [];
     const fields: M2Symbol[] = [];
     let depth = 0;       // ( [ {
     let depth = 0;       // ( [ {
     let recDepth = 0;    // RECORD..END nesting
     let recDepth = 0;    // RECORD..END nesting
+    let roundDepth = 0;  // ( ) only, for enumeration detection
     let prevSignificant: Token | null = null;
     let prevSignificant: Token | null = null;
     const fieldCands: Token[] = [];
     const fieldCands: Token[] = [];
+    const topIdents: Token[] = [];
+    let enumOk = true;
+    let firstTok: Token | null = null;
+    let lastTok: Token | null = null;
     while (!this.atEof()) {
     while (!this.atEof()) {
       const t = this.peek();
       const t = this.peek();
       if (t.kind === 'symbol') {
       if (t.kind === 'symbol') {
         if (t.text === '(' || t.text === '[' || t.text === '{') depth++;
         if (t.text === '(' || t.text === '[' || t.text === '{') depth++;
         else if (t.text === ')' || t.text === ']' || t.text === '}') depth = Math.max(0, depth - 1);
         else if (t.text === ')' || t.text === ']' || t.text === '}') depth = Math.max(0, depth - 1);
         else if (t.text === ';' && depth === 0 && recDepth === 0) break;
         else if (t.text === ';' && depth === 0 && recDepth === 0) break;
+        if (t.text === '(') roundDepth++;
+        else if (t.text === ')') roundDepth = Math.max(0, roundDepth - 1);
+        if (t.text !== '(' && t.text !== ')' && t.text !== ',') enumOk = false;
       } else if (t.kind === 'keyword') {
       } else if (t.kind === 'keyword') {
         if (t.text === 'RECORD' && depth === 0) recDepth++;
         if (t.text === 'RECORD' && depth === 0) recDepth++;
         else if (t.text === 'END' && depth === 0 && recDepth > 0) recDepth--;
         else if (t.text === 'END' && depth === 0 && recDepth > 0) recDepth--;
         else if ((t.text === 'CONST' || t.text === 'TYPE' || t.text === 'VAR' ||
         else if ((t.text === 'CONST' || t.text === 'TYPE' || t.text === 'VAR' ||
                   t.text === 'PROCEDURE' || t.text === 'MODULE' || t.text === 'BEGIN' ||
                   t.text === 'PROCEDURE' || t.text === 'MODULE' || t.text === 'BEGIN' ||
                   t.text === 'FINALLY') && depth === 0 && recDepth === 0) break;
                   t.text === 'FINALLY') && depth === 0 && recDepth === 0) break;
+        enumOk = false;
       } else if (t.kind === 'ident' && recDepth > 0 && depth === 0) {
       } else if (t.kind === 'ident' && recDepth > 0 && depth === 0) {
         // Candidate record field: ident followed by ',' or ':' at field level.
         // Candidate record field: ident followed by ',' or ':' at field level.
         const nx = this.peek(1);
         const nx = this.peek(1);
@@ -296,7 +322,15 @@ class Parser {
         if (!afterCase && nx.kind === 'symbol' && (nx.text === ',' || nx.text === ':')) {
         if (!afterCase && nx.kind === 'symbol' && (nx.text === ',' || nx.text === ':')) {
           fieldCands.push(t);
           fieldCands.push(t);
         }
         }
+      } else if (t.kind === 'ident') {
+        if (roundDepth === 1 && recDepth === 0 && depth <= 1) topIdents.push(t);
+        else enumOk = false;
+      } else {
+        // Numbers, strings and anything else disqualify enumerations.
+        enumOk = false;
       }
       }
+      if (!firstTok) firstTok = t;
+      lastTok = t;
       if (t.kind !== 'eof') prevSignificant = t;
       if (t.kind !== 'eof') prevSignificant = t;
       this.next();
       this.next();
     }
     }
@@ -311,7 +345,8 @@ class Parser {
     const endTok = this.peek();
     const endTok = this.peek();
     const text = this.rawRange(startIdx, this.pos).trim().replace(/\s+/g, ' ');
     const text = this.rawRange(startIdx, this.pos).trim().replace(/\s+/g, ' ');
     void endTok;
     void endTok;
-    return { text, fields };
+    const isEnum = enumOk && topIdents.length > 0 && firstTok?.text === '(' && lastTok?.text === ')';
+    return { text, fields, enumLiterals: isEnum ? topIdents : [] };
   }
   }
 
 
   private skipAlignment(): void {
   private skipAlignment(): void {

+ 22 - 2
extensions/modula2-language/src/m2/resolve.ts

@@ -59,6 +59,12 @@ export function visibleSymbols(unit: M2Unit, line: number, ch: number): M2Symbol
   for (let k = chain.length - 1; k >= 0; k--) {
   for (let k = chain.length - 1; k >= 0; k--) {
     for (const c of chain[k].children) {
     for (const c of chain[k].children) {
       if (!seen.has(c.name)) { seen.add(c.name); out.push(c); }
       if (!seen.has(c.name)) { seen.add(c.name); out.push(c); }
+      // Enumeration literals are visible wherever their type is.
+      if (c.kind === 'type') {
+        for (const g of c.children) {
+          if (g.kind === 'constant' && !seen.has(g.name)) { seen.add(g.name); out.push(g); }
+        }
+      }
     }
     }
   }
   }
   // Imported names.
   // Imported names.
@@ -121,16 +127,30 @@ function topLevel(unit: M2Unit, name: string): M2Symbol | null {
   return null;
   return null;
 }
 }
 
 
+/** Find a name among scope children, including enumeration literals
+ *  (constants declared inside a TYPE's `(A, B, ...)` list). */
+function findInScope(children: M2Symbol[], name: string): M2Symbol | null {
+  const direct = children.find(c => c.name === name);
+  if (direct) return direct;
+  for (const c of children) {
+    if (c.kind === 'type') {
+      const lit = c.children.find(g => g.kind === 'constant' && g.name === name);
+      if (lit) return lit;
+    }
+  }
+  return null;
+}
+
 /** Resolve a same-file visible name to its declaration. */
 /** Resolve a same-file visible name to its declaration. */
 export function resolveLocal(unit: M2Unit, line: number, ch: number, name: string): M2Symbol | null {
 export function resolveLocal(unit: M2Unit, line: number, ch: number, name: string): M2Symbol | null {
   const chain = scopeChainAt(unit, line, ch);
   const chain = scopeChainAt(unit, line, ch);
   for (let k = chain.length - 1; k >= 0; k--) {
   for (let k = chain.length - 1; k >= 0; k--) {
-    const found = chain[k].children.find(c => c.name === name);
+    const found = findInScope(chain[k].children, name);
     if (found) return found;
     if (found) return found;
   }
   }
   // Module-level fallback.
   // Module-level fallback.
   for (const s of unit.symbols) {
   for (const s of unit.symbols) {
-    const found = s.children.find(c => c.name === name);
+    const found = findInScope(s.children, name);
     if (found) return found;
     if (found) return found;
   }
   }
   return null;
   return null;

+ 20 - 3
extensions/modula2-language/src/server.ts

@@ -12,6 +12,7 @@ import { spawn } from 'child_process';
 import { pathToFileURL } from 'url';
 import { pathToFileURL } from 'url';
 import { lex, Token } from './m2/lexer';
 import { lex, Token } from './m2/lexer';
 import { M2Symbol, M2Unit, parseUnitText } from './m2/parser';
 import { M2Symbol, M2Unit, parseUnitText } from './m2/parser';
+import { analyseUnit } from './m2/analyse';
 import {
 import {
   moduleExports, recordFieldsOf, resolveName, toLocation, visibleSymbols,
   moduleExports, recordFieldsOf, resolveName, toLocation, visibleSymbols,
   findReferencesInText, LocatedOccurrence,
   findReferencesInText, LocatedOccurrence,
@@ -50,8 +51,9 @@ documents.onDidChangeContent(e => validate(e.document));
 documents.onDidClose(e => connection.sendDiagnostics({ uri: e.document.uri, diagnostics: [] }));
 documents.onDidClose(e => connection.sendDiagnostics({ uri: e.document.uri, diagnostics: [] }));
 
 
 async function validate(document: TextDocument): Promise<void> {
 async function validate(document: TextDocument): Promise<void> {
+  const local = analyseDocument(document);
   if (!validatorCommand) {
   if (!validatorCommand) {
-    connection.sendDiagnostics({ uri: document.uri, diagnostics: [] });
+    connection.sendDiagnostics({ uri: document.uri, diagnostics: local });
     return;
     return;
   }
   }
 
 
@@ -64,9 +66,9 @@ async function validate(document: TextDocument): Promise<void> {
     await fs.writeFile(tempFile, document.getText(), 'utf8');
     await fs.writeFile(tempFile, document.getText(), 'utf8');
     const args = validatorArguments.map(a => a.replace(/\{file\}/g, tempFile));
     const args = validatorArguments.map(a => a.replace(/\{file\}/g, tempFile));
     const output = await execute(validatorCommand, args, path.dirname(filePath));
     const output = await execute(validatorCommand, args, path.dirname(filePath));
-    connection.sendDiagnostics({ uri: document.uri, diagnostics: parseDiagnostics(output) });
+    connection.sendDiagnostics({ uri: document.uri, diagnostics: [...local, ...parseDiagnostics(output)] });
   } catch (error) {
   } catch (error) {
-    connection.sendDiagnostics({ uri: document.uri, diagnostics: [{
+    connection.sendDiagnostics({ uri: document.uri, diagnostics: [...local, {
       severity: DiagnosticSeverity.Warning,
       severity: DiagnosticSeverity.Warning,
       range: { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } },
       range: { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } },
       message: `Coco/R validator: ${error instanceof Error ? error.message : String(error)}`,
       message: `Coco/R validator: ${error instanceof Error ? error.message : String(error)}`,
@@ -110,6 +112,21 @@ function uriToFilePath(uri: string): string {
   return uri;
   return uri;
 }
 }
 
 
+/** Local semantic diagnostics (always on; merged with Coco/R output when configured). */
+function analyseDocument(document: TextDocument): Diagnostic[] {
+  const text = document.getText();
+  const filePath = uriToFilePath(document.uri);
+  return analyseUnit(text, filePath, path.dirname(filePath)).map(issue => ({
+    severity: issue.severity === 'error' ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning,
+    range: {
+      start: { line: issue.line, character: issue.ch },
+      end: { line: issue.endLine, character: issue.endCh },
+    },
+    message: issue.message,
+    source: 'modula2',
+  }));
+}
+
 // ---- Phase 2: AST-backed language intelligence ----
 // ---- Phase 2: AST-backed language intelligence ----
 
 
 const SERVER_KEYWORDS = [
 const SERVER_KEYWORDS = [