Report grammar rule undefined error and unreferenced warning.

ts/packages/actionGrammar/src/grammarCompiler.ts

@@ -11,40 +11,93 @@ import { Rule, RuleDefinition } from "./grammarRuleParser.js";
 
 type DefinitionMap = Map<
     string,
-    { rules: Rule[]; grammarRules?: GrammarRule[] }
+    { rules: Rule[]; pos: number | undefined; grammarRules?: GrammarRule[] }
 >;
 
-export function compileGrammar(definitions: RuleDefinition[]): Grammar {
+type GrammarCompileResult = {
+    grammar: Grammar;
+    errors: GrammarCompileError[];
+    warnings: GrammarCompileError[];
+};
+
+export type GrammarCompileError = {
+    message: string;
+    definition?: string | undefined;
+    pos?: number | undefined;
+};
+
+type CompileContext = {
+    ruleDefMap: DefinitionMap;
+    currentDefinition?: string | undefined;
+    errors: GrammarCompileError[];
+    warnings: GrammarCompileError[];
+};
+
+export function compileGrammar(
+    definitions: RuleDefinition[],
+    start: string,
+): GrammarCompileResult {
     const ruleDefMap: DefinitionMap = new Map();
+    const context: CompileContext = {
+        ruleDefMap,
+        errors: [],
+        warnings: [],
+    };
+
     for (const def of definitions) {
         const existing = ruleDefMap.get(def.name);
         if (existing === undefined) {
-            ruleDefMap.set(def.name, { rules: [...def.rules] });
+            ruleDefMap.set(def.name, { rules: [...def.rules], pos: def.pos });
         } else {
             existing.rules.push(...def.rules);
         }
     }
-    return { rules: createGrammarRules(ruleDefMap, "Start") };
+    const grammar = { rules: createGrammarRules(context, start) };
+
+    for (const [name, record] of ruleDefMap.entries()) {
+        if (record.grammarRules === undefined) {
+            context.warnings.push({
+                message: `Rule '<${name}>' is defined but never used.`,
+                pos: record.pos,
+            });
+        }
+    }
+    return {
+        grammar,
+        errors: context.errors,
+        warnings: context.warnings,
+    };
 }
 
+const emptyRecord = { rules: [], pos: undefined, grammarRules: [] };
 function createGrammarRules(
-    ruleDefMap: DefinitionMap,
+    context: CompileContext,
     name: string,
+    pos?: number,
 ): GrammarRule[] {
-    const record = ruleDefMap.get(name);
+    const record = context.ruleDefMap.get(name);
     if (record === undefined) {
-        throw new Error(`Missing rule definition for '<${name}>'`);
+        context.errors.push({
+            message: `Missing rule definition for '<${name}>'`,
+            definition: context.currentDefinition,
+            pos,
+        });
+        context.ruleDefMap.set(name, emptyRecord);
+        return emptyRecord.grammarRules;
     }
     if (record.grammarRules === undefined) {
         record.grammarRules = [];
+        const prev = context.currentDefinition;
+        context.currentDefinition = name;
         for (const r of record.rules) {
-            record.grammarRules.push(createGrammarRule(ruleDefMap, r));
+            record.grammarRules.push(createGrammarRule(context, r));
         }
+        context.currentDefinition = prev;
     }
     return record.grammarRules;
 }
 
-function createGrammarRule(ruleDefMap: DefinitionMap, rule: Rule): GrammarRule {
+function createGrammarRule(context: CompileContext, rule: Rule): GrammarRule {
     const { expressions, value } = rule;
     const parts: GrammarPart[] = [];
     for (const expr of expressions) {
@@ -59,15 +112,15 @@ function createGrammarRule(ruleDefMap: DefinitionMap, rule: Rule): GrammarRule {
                 break;
             }
             case "variable": {
-                const { name, typeName, ruleReference } = expr;
+                const { name, typeName, ruleReference, ruleRefPos } = expr;
                 if (ruleReference) {
-                    const rules = ruleDefMap.get(typeName);
-                    if (rules === undefined) {
-                        throw new Error(`No rule named ${typeName}`);
-                    }
                     parts.push({
                         type: "rules",
-                        rules: createGrammarRules(ruleDefMap, typeName),
+                        rules: createGrammarRules(
+                            context,
+                            typeName,
+                            ruleRefPos,
+                        ),
                         variable: name,
                         name: typeName,
                         optional: expr.optional,
@@ -91,23 +144,23 @@ function createGrammarRule(ruleDefMap: DefinitionMap, rule: Rule): GrammarRule {
             case "ruleReference":
                 parts.push({
                     type: "rules",
-                    rules: createGrammarRules(ruleDefMap, expr.name),
+                    rules: createGrammarRules(context, expr.name, expr.pos),
                     name: expr.name,
                 });
                 break;
             case "rules": {
                 const { rules, optional } = expr;
                 parts.push({
                     type: "rules",
-                    rules: rules.map((r) => createGrammarRule(ruleDefMap, r)),
+                    rules: rules.map((r) => createGrammarRule(context, r)),
                     optional,
                 });
 
                 break;
             }
             default:
                 throw new Error(
-                    `Unknown expression type ${(expr as any).type}`,
+                    `Internal Error: Unknown expression type ${(expr as any).type}`,
                 );
         }
     }

ts/packages/actionGrammar/src/grammarLoader.ts

@@ -1,12 +1,72 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { compileGrammar } from "./grammarCompiler.js";
+import { compileGrammar, GrammarCompileError } from "./grammarCompiler.js";
 import { parseGrammarRules } from "./grammarRuleParser.js";
 import { Grammar } from "./grammarTypes.js";
+import { getLineCol } from "./utils.js";
 
-export function loadGrammarRules(fileName: string, content: string): Grammar {
+// REVIEW: start symbol should be configurable
+const start = "Start";
+
+function convertCompileError(
+    fileName: string,
+    content: string,
+    type: "error" | "warning",
+    errors: GrammarCompileError[],
+) {
+    return errors.map((e) => {
+        const lineCol = getLineCol(content, e.pos ?? 0);
+        return `${fileName}(${lineCol.line},${lineCol.col}): ${type}: ${e.message}${e.definition ? ` in definition '<${e.definition}>'` : ""}`;
+    });
+}
+
+// Throw exception when error.
+export function loadGrammarRules(fileName: string, content: string): Grammar;
+// Return undefined when error if errors array provided.
+export function loadGrammarRules(
+    fileName: string,
+    content: string,
+    errors: string[],
+    warnings?: string[],
+): Grammar | undefined;
+export function loadGrammarRules(
+    fileName: string,
+    content: string,
+    errors?: string[],
+    warnings?: string[],
+): Grammar | undefined {
     const definitions = parseGrammarRules(fileName, content);
-    const grammar = compileGrammar(definitions);
-    return grammar;
+    const result = compileGrammar(definitions, start);
+
+    if (result.warnings.length > 0 && warnings !== undefined) {
+        warnings.push(
+            ...convertCompileError(
+                fileName,
+                content,
+                "warning",
+                result.warnings,
+            ),
+        );
+    }
+
+    if (result.errors.length === 0) {
+        return result.grammar;
+    }
+    const errorMessages = convertCompileError(
+        fileName,
+        content,
+        "error",
+        result.errors,
+    );
+    if (errors) {
+        errors.push(...errorMessages);
+        return undefined;
+    }
+
+    const errorStr = result.errors.length === 1 ? "error" : "errors";
+    errorMessages.unshift(
+        `Error detected in grammar compilation '${fileName}': ${result.errors.length} ${errorStr}.`,
+    );
+    throw new Error(errorMessages.join("\n"));
 }

ts/packages/actionGrammar/src/grammarRuleParser.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import registerDebug from "debug";
+import { getLineCol } from "./utils.js";
 
 const debugParse = registerDebug("typeagent:grammar:parse");
 /**
@@ -50,8 +51,9 @@ const debugParse = registerDebug("typeagent:grammar:parse");
 export function parseGrammarRules(
     fileName: string,
     content: string,
+    position: boolean = true,
 ): RuleDefinition[] {
-    const parser = new GrammarRuleParser(fileName, content);
+    const parser = new GrammarRuleParser(fileName, content, position);
     const definitions = parser.parse();
     debugParse(JSON.stringify(definitions, undefined, 2));
     return definitions;
@@ -67,6 +69,7 @@ type StrExpr = {
 type RuleRefExpr = {
     type: "ruleReference";
     name: string;
+    pos?: number | undefined;
 };
 
 type RulesExpr = {
@@ -80,6 +83,7 @@ type VarDefExpr = {
     name: string;
     typeName: string;
     ruleReference: boolean;
+    ruleRefPos?: number | undefined;
     optional?: boolean;
 };
 
@@ -113,6 +117,7 @@ export type Rule = {
 export type RuleDefinition = {
     name: string;
     rules: Rule[];
+    pos?: number | undefined;
 };
 
 export function isWhitespace(char: string) {
@@ -152,8 +157,13 @@ class GrammarRuleParser {
     constructor(
         private readonly fileName: string,
         private readonly content: string,
+        private readonly position: boolean = true,
     ) {}
 
+    private get pos(): number | undefined {
+        return this.position ? this.curr : undefined;
+    }
+
     private isAtWhiteSpace() {
         return !this.isAtEnd() && isWhitespace(this.content[this.curr]);
     }
@@ -319,14 +329,16 @@ class GrammarRuleParser {
         const id = this.parseId("Variable name");
         let typeName: string = "string";
         let ruleReference: boolean = false;
+        let ruleRefPos: number | undefined = undefined;
 
         if (this.isAt(":")) {
             // Consume colon
             this.skipWhitespace(1);
 
             if (this.isAt("<")) {
-                typeName = this.parseRuleName();
+                ruleRefPos = this.pos;
                 ruleReference = true;
+                typeName = this.parseRuleName();
             } else {
                 typeName = this.parseId("Type name");
             }
@@ -336,18 +348,17 @@ class GrammarRuleParser {
             name: id,
             typeName,
             ruleReference,
+            ruleRefPos,
         };
     }
 
     private parseExpression(): Expr[] {
         const expNodes: Expr[] = [];
         do {
             if (this.isAt("<")) {
-                const n = this.parseRuleName();
-                expNodes.push({
-                    type: "ruleReference",
-                    name: n,
-                });
+                const pos = this.pos;
+                const name = this.parseRuleName();
+                expNodes.push({ type: "ruleReference", name, pos });
                 continue;
             }
             if (this.isAt("$(")) {
@@ -575,13 +586,11 @@ class GrammarRuleParser {
 
     private parseRuleDefinition(): RuleDefinition {
         this.consume("@", "start of rule");
-        const n = this.parseRuleName();
+        const pos = this.pos;
+        const name = this.parseRuleName();
         this.consume("=", "after rule identifier");
-        const r = this.parseRules();
-        return {
-            name: n,
-            rules: r,
-        };
+        const rules = this.parseRules();
+        return { name, rules, pos };
     }
 
     private consume(expected: string, reason?: string) {
@@ -594,18 +603,7 @@ class GrammarRuleParser {
     }
 
     private getLineCol(pos: number) {
-        let line = 1;
-        let col = 1;
-        const content = this.content;
-        for (let i = 0; i < pos && i < content.length; i++) {
-            if (content[i] === "\n") {
-                line++;
-                col = 1;
-            } else {
-                col++;
-            }
-        }
-        return { line, col };
+        return getLineCol(this.content, pos);
     }
 
     private throwError(message: string, pos: number = this.curr): never {

ts/packages/actionGrammar/src/utils.ts

@@ -0,0 +1,16 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+export function getLineCol(content: string, pos: number) {
+    let line = 1;
+    let col = 1;
+    for (let i = 0; i < pos && i < content.length; i++) {
+        if (content[i] === "\n") {
+            line++;
+            col = 1;
+        } else {
+            col++;
+        }
+    }
+    return { line, col };
+}