Port: LG error related changes (#1785)

* port LG error centralization * merge changes * parity code * fix lg test bugs * fix bugs * fix * fix test cases * fix assert throw to verify error message * modify staticChecker * fix bugs * fix import * remove comments * add signature * remove unused file * retrigger Co-authored-by: Hongyang Du (hond) <[email protected]>
microsoft · Feb 26, 2020 · 25b1b53 · 25b1b53
1 parent b8711ea
commit 25b1b53
Show file tree

Hide file tree

Showing 32 changed files with 1,526 additions and 1,209 deletions.
diff --git a/libraries/adaptive-expressions/src/commonRegex.ts b/libraries/adaptive-expressions/src/commonRegex.ts
@@ -75,6 +75,7 @@ export class CommonRegex {
     private static antlrParse(pattern: string): ParseTree {
         const inputStream: ANTLRInputStream = new ANTLRInputStream(pattern);
         const lexer: CommonRegexLexer = new CommonRegexLexer(inputStream);
+        lexer.removeErrorListeners();
         const tokenStream: CommonTokenStream = new CommonTokenStream(lexer);
         const parser: CommonRegexParser = new CommonRegexParser(tokenStream);
         parser.removeErrorListeners();

diff --git a/libraries/adaptive-expressions/src/expressionFunctions.ts b/libraries/adaptive-expressions/src/expressionFunctions.ts
@@ -1011,6 +1011,9 @@ export class ExpressionFunctions {
         let instance: any;
 
         ({ value: instance, error } = expression.children[0].tryEvaluate(state));
+        if (!instance) {
+            error = `'${ expression.children[0] }' evaluated to null.`;
+        }
 
         if (!error) {
             // 2nd parameter has been rewrite to $local.item

diff --git a/libraries/adaptive-expressions/src/parser/expressionEngine.ts b/libraries/adaptive-expressions/src/parser/expressionEngine.ts
@@ -209,6 +209,7 @@ export class ExpressionEngine implements ExpressionParserInterface {
     protected static antlrParse(expression: string): ParseTree {
         const inputStream: ANTLRInputStream = new ANTLRInputStream(expression);
         const lexer: ExpressionLexer = new ExpressionLexer(inputStream);
+        lexer.removeErrorListeners();
         const tokenStream: CommonTokenStream = new CommonTokenStream(lexer);
         const parser: ExpressionParser = new ExpressionParser(tokenStream);
         parser.removeErrorListeners();

diff --git a/libraries/botbuilder-lg/src/LGFileLexer.g4 b/libraries/botbuilder-lg/src/LGFileLexer.g4
@@ -52,6 +52,10 @@ fragment ESCAPE_CHARACTER_FRAGMENT : '\\' ~[\r\n]?;
 
 
 // top level elements
+OPTIONS
+  : '>' WHITESPACE* '!#' ~('\r'|'\n')+
+  ;
+
 COMMENTS
   : '>' ~('\r'|'\n')+ -> skip
   ;

diff --git a/libraries/botbuilder-lg/src/LGFileParser.g4 b/libraries/botbuilder-lg/src/LGFileParser.g4
@@ -9,6 +9,7 @@ file
 paragraph
     : templateDefinition
     | importDefinition
+    | optionsDefinition
     | EOF
     | errorTemplate
     ;
@@ -125,4 +126,8 @@ switchCaseStat
 
 importDefinition
     : IMPORT
+    ;
+
+optionsDefinition
+    : OPTIONS
     ;
diff --git a/libraries/botbuilder-lg/src/README.md b/libraries/botbuilder-lg/src/README.md
diff --git a/libraries/botbuilder-lg/src/activityChecker.ts b/libraries/botbuilder-lg/src/activityChecker.ts
@@ -169,7 +169,7 @@ export class ActivityChecker {
 
         if (typeof value === 'object') {
             const type: string = this.getStructureType(value);
-            if (type !== 'cardaction'){
+            if (type !== 'cardaction') {
                 result.push(this.buildDiagnostic(`'${ type }' is not card action type.`, false));
             } else {
                 result.push(...this.checkCardActionPropertyName(value));
@@ -236,7 +236,7 @@ export class ActivityChecker {
         } else if (type === 'adaptivecard') {
             // TODO
             // check adaptivecard format
-        } else if(type === 'attachment') {
+        } else if (type === 'attachment') {
             // TODO
             // Check attachment format
         } else {
@@ -311,7 +311,7 @@ export class ActivityChecker {
     private static normalizedToList(item: any): any[] {
         if (item === undefined) {
             return [];
-        } else if (Array.isArray(item)){
+        } else if (Array.isArray(item)) {
             return item;
         } else {
             return [item];

diff --git a/libraries/botbuilder-lg/src/activityFactory.ts b/libraries/botbuilder-lg/src/activityFactory.ts
@@ -131,7 +131,7 @@ export class ActivityFactory {
                 value: ''
             };
 
-            if(type === 'cardaction') {
+            if (type === 'cardaction') {
                 for (const key of Object.keys(action)) {
                     const property: string = key.trim();
                     if (property === Evaluator.LGType) {
@@ -194,9 +194,9 @@ export class ActivityFactory {
         const type: string = this.getStructureType(input);
         if (ActivityChecker.genericCardTypeMapping.has(type)) {
             attachment = this.getCardAttachment(ActivityChecker.genericCardTypeMapping.get(type), input);
-        } else if(type === 'adaptivecard') {
+        } else if (type === 'adaptivecard') {
             attachment = CardFactory.adaptiveCard(input);
-        } else if(type === 'attachment') {
+        } else if (type === 'attachment') {
             attachment = this.getNormalAttachment(input);
         } else {
             attachment = {contentType: type, content: input};
@@ -319,7 +319,7 @@ export class ActivityFactory {
     private static normalizedToList(item: any): any[] {
         if (item === undefined) {
             return [];
-        } else if (Array.isArray(item)){
+        } else if (Array.isArray(item)) {
             return item;
         } else {
             return [item];
@@ -335,7 +335,7 @@ export class ActivityFactory {
 
         lgStringResult = lgStringResult.trim();
 
-        if (lgStringResult === '' || !lgStringResult.startsWith('{') || !lgStringResult.endsWith('}')){
+        if (lgStringResult === '' || !lgStringResult.startsWith('{') || !lgStringResult.endsWith('}')) {
             return undefined;
         }
 

diff --git a/libraries/botbuilder-lg/src/analyzer.ts b/libraries/botbuilder-lg/src/analyzer.ts
@@ -17,6 +17,7 @@ import { LGFileParserVisitor } from './generated/LGFileParserVisitor';
 import { LGTemplate } from './lgTemplate';
 import { LGExtensions } from './lgExtensions';
 import { AnalyzerResult } from './analyzerResult';
+import {LGErrors} from './lgErrors';
 
 // tslint:disable-next-line: max-classes-per-file
 /**
@@ -49,11 +50,11 @@ export class Analyzer extends AbstractParseTreeVisitor<AnalyzerResult> implement
      */
     public analyzeTemplate(templateName: string): AnalyzerResult {
         if (!(templateName in this.templateMap)) {
-            throw new Error(`No such template: ${ templateName }`);
+            throw new Error(LGErrors.templateNotExist(templateName));
         }
 
         if (this.evalutationTargetStack.find((u: EvaluationTarget): boolean => u.templateName === templateName) !== undefined) {
-            throw new Error(`Loop detected: ${ this.evalutationTargetStack.reverse()
+            throw new Error(`${ LGErrors.loopDetected } ${ this.evalutationTargetStack.reverse()
                 .map((u: EvaluationTarget): string => u.templateName)
                 .join(' => ') }`);
         }
@@ -79,7 +80,7 @@ export class Analyzer extends AbstractParseTreeVisitor<AnalyzerResult> implement
             }
         }
 
-        throw Error(`template name match failed`);
+        return new AnalyzerResult();
     }
 
     public visitNormalBody(ctx: lp.NormalBodyContext): AnalyzerResult {

diff --git a/libraries/botbuilder-lg/src/customizedMemory.ts b/libraries/botbuilder-lg/src/customizedMemory.ts
@@ -25,9 +25,9 @@ export class CustomizedMemory implements MemoryInterface {
      */
     public localMemory: MemoryInterface;
 
-    public constructor(scope?: any) {
+    public constructor(scope?: any, localMemory: MemoryInterface = undefined) {
         this.globalMemory = !scope ? undefined : SimpleObjectMemory.wrap(scope);
-        this.localMemory = undefined;
+        this.localMemory = localMemory;
     }
 
     /**

diff --git a/libraries/botbuilder-lg/src/diagnostic.ts b/libraries/botbuilder-lg/src/diagnostic.ts
@@ -43,8 +43,8 @@ export class Diagnostic {
 
     public toString(): string {
 
-        // ignore error range if source is "inline"
-        if (this.source === 'inline') {
+        // ignore error range if source is "inline content"
+        if (this.source === 'inline content') {
             return `[${ DiagnosticSeverity[this.severity] }] ${ this.message.toString() }`;
         } else {
             return `[${ DiagnosticSeverity[this.severity] }] ${ this.range.toString() }: ${ this.message.toString() }`;

diff --git a/libraries/botbuilder-lg/src/errorListener.ts b/libraries/botbuilder-lg/src/errorListener.ts
@@ -10,6 +10,7 @@ import { Diagnostic, DiagnosticSeverity } from './diagnostic';
 import { LGException } from './lgException';
 import { Position } from './position';
 import { Range } from './range';
+import { LGErrors } from './lgErrors';
 
 // tslint:disable-next-line: completed-docs
 /**
@@ -34,11 +35,7 @@ export class ErrorListener implements ANTLRErrorListener<any> {
         // tslint:disable-next-line: restrict-plus-operands
         const stopPosition: Position = new Position(line, charPositionInLine + offendingSymbol.stopIndex - offendingSymbol.startIndex + 1);
         const range: Range = new Range(startPosition, stopPosition);
-        msg = `syntax error message: ${ msg }`;
-        if (this.source !== undefined && this.source !== '') {
-            msg = `source: ${ this.source }, ${ msg }`;
-        }
-        const diagnostic: Diagnostic = new Diagnostic(range, msg, DiagnosticSeverity.Error, this.source);
+        const diagnostic: Diagnostic = new Diagnostic(range, LGErrors.syntaxError, DiagnosticSeverity.Error, this.source);
 
         throw new LGException(diagnostic.toString(), [diagnostic]);
     }

diff --git a/libraries/botbuilder-lg/src/evaluationTarget.ts b/libraries/botbuilder-lg/src/evaluationTarget.ts
@@ -45,14 +45,14 @@ export class EvaluationTarget {
         const memory = this.scope as CustomizedMemory;
         let result = this.templateName;
         if (memory) {
-            if (memory.globalMemory){
+            if (memory.globalMemory) {
                 const version = memory.globalMemory.version();
                 if (version) {
                     result = result.concat(version);
                 }
             }
 
-            if (memory.localMemory){
+            if (memory.localMemory) {
                 const localMemoryString = memory.localMemory.toString();
                 if (localMemoryString) {
                     result = result.concat(localMemoryString);