Add ThrowOnRecursive to LG EvaluationOptions (#5872)

* Add ThrowOnRecursive to LG EvaluationOptions

* Address ThrowOnRecursive feedback

* Do not pass LgOptions to Analyzer constructor (they are available through the Templates instance)

* Add AnalyzerOptions

libraries/Microsoft.Bot.Builder.LanguageGeneration/Analyzer.cs

@@ -19,16 +19,20 @@ internal class Analyzer : LGTemplateParserBaseVisitor<AnalyzerResult>
         private readonly IExpressionParser _expressionParser;
 
         private readonly Stack<EvaluationTarget> _evaluationTargetStack = new Stack<EvaluationTarget>();
+
+        private readonly AnalyzerOptions _analyzerOptions;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Analyzer"/> class.
         /// </summary>
         /// <param name="templates">Templates.</param>
         /// <param name="opt">Options for LG. </param>
-        public Analyzer(Templates templates, EvaluationOptions opt = null)
+        /// <param name="analyzerOptions">Options for the analyzer.</param>
+        public Analyzer(Templates templates, EvaluationOptions opt = null, AnalyzerOptions analyzerOptions = null)
         {
             Templates = templates;
             _templateMap = templates.ToDictionary(t => t.Name);
+            _analyzerOptions = analyzerOptions;
 
             // create an evaluator to leverage it's customized function look up for checking
             var evaluator = new Evaluator(Templates, opt);
@@ -50,7 +54,23 @@ public Analyzer(Templates templates, EvaluationOptions opt = null)
         /// <returns>Analyze result including variables and template references.</returns>
         public AnalyzerResult AnalyzeTemplate(string templateName)
         {
-            if (!_templateMap.ContainsKey(templateName) || _evaluationTargetStack.Any(e => e.TemplateName == templateName))
+            var missingName = !_templateMap.ContainsKey(templateName);
+            var stackHasName = _evaluationTargetStack.Any(e => e.TemplateName == templateName);
+
+            if (_analyzerOptions?.ThrowOnRecursive == true)
+            {
+                if (missingName)
+                {
+                    throw new ArgumentException(TemplateErrors.TemplateNotExist(templateName));
+                }
+
+                if (stackHasName)
+                {
+                    throw new InvalidOperationException($"{TemplateErrors.LoopDetected} {string.Join(" => ", _evaluationTargetStack.Reverse().Select(e => e.TemplateName))} => {templateName}");
+                }
+            }
+
+            if (missingName || stackHasName)
             {
                 return new AnalyzerResult();
             }
@@ -210,7 +230,7 @@ private AnalyzerResult AnalyzeExpressionDirectly(Expression exp)
                 }
                 else
                 {
-                    if (!result.TemplateReferences.Contains(templateName))
+                    if (_analyzerOptions?.ThrowOnRecursive == true || !result.TemplateReferences.Contains(templateName))
                     {
                         // if template has parameters, just get the template ref without variables.
                         result.Union(new AnalyzerResult(templateReferences: this.AnalyzeTemplate(templateName).TemplateReferences));

libraries/Microsoft.Bot.Builder.LanguageGeneration/AnalyzerOptions.cs

@@ -0,0 +1,89 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+
+namespace Microsoft.Bot.Builder.LanguageGeneration
+{
+    /// <summary>
+    /// Options for analyzing LG templates.
+    /// </summary>
+    public class AnalyzerOptions
+    {
+        private readonly string _throwOnRecursive = "@throwOnRecursive";
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AnalyzerOptions"/> class.
+        /// </summary>
+        public AnalyzerOptions()
+        {
+            ThrowOnRecursive = null;
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AnalyzerOptions"/> class.
+        /// </summary>
+        /// <param name="opt">Instance to copy analyzer settings from.</param>
+        public AnalyzerOptions(AnalyzerOptions opt)
+        {
+            ThrowOnRecursive = opt.ThrowOnRecursive;
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AnalyzerOptions"/> class.
+        /// </summary>
+        /// <param name="optionsList">List of strings containing the options from an LG file.</param>
+        public AnalyzerOptions(IList<string> optionsList)
+        {
+            if (optionsList != null)
+            {
+                foreach (var option in optionsList)
+                {
+                    if (!string.IsNullOrWhiteSpace(option) && option.Contains("="))
+                    {
+                        var index = option.IndexOf('=');
+                        var key = option.Substring(0, index).Trim();
+                        var value = option.Substring(index + 1).Trim();
+
+                        if (key.Equals(_throwOnRecursive, StringComparison.OrdinalIgnoreCase))
+                        {
+                            if (value.ToLowerInvariant() == "true")
+                            {
+                                ThrowOnRecursive = true;
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Gets or sets a value determining if recursive calls throw an exception.
+        /// </summary>
+        /// <value>
+        /// When true, throw an exception if a recursive call is detected.
+        /// </value>
+        public bool? ThrowOnRecursive { get; set; } = null;
+
+        /// <summary>
+        /// Merge a incoming option to current option. If a property in incoming option is not null while it is null in current
+        /// option, then the value of this property will be overwritten.
+        /// </summary>
+        /// <param name="opt">Incoming option for merging.</param>
+        /// <returns>Result after merging.</returns>
+        public AnalyzerOptions Merge(AnalyzerOptions opt)
+        {
+            var properties = typeof(AnalyzerOptions).GetProperties();
+            foreach (var property in properties)
+            {
+                if (property.GetValue(this) == null && property.GetValue(opt) != null)
+                {
+                    property.SetValue(this, property.GetValue(opt));
+                }
+            }
+
+            return this;
+        }
+    }
+}

libraries/Microsoft.Bot.Builder.LanguageGeneration/Templates.cs

@@ -320,11 +320,13 @@ public IList<object> ExpandTemplate(string templateName, object scope = null, Ev
         /// Analyzes a template to get the static analyzer results including variables and template references.
         /// </summary>
         /// <param name="templateName">Template name to be evaluated.</param>
+        /// <param name="analyzerOptions ">Options used to determine behavior of the analyzer.</param>
         /// <returns>Analyzer result.</returns>
-        public AnalyzerResult AnalyzeTemplate(string templateName)
+        public AnalyzerResult AnalyzeTemplate(string templateName, AnalyzerOptions analyzerOptions = null)
         {
             CheckErrors();
-            var analyzer = new Analyzer(this);
+
+            var analyzer = new Analyzer(this, this.LgOptions, analyzerOptions);
             return analyzer.AnalyzeTemplate(templateName);
         }
 

tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/LoopDetected.lg

@@ -9,5 +9,5 @@
 - ${wPhrase()}
 
 > self loop
-# shouldFail(x)
-- ${shouldFail(x)}
+# selfLoop(x)
+- ${selfLoop(x)}

tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/TemplateDiagnosticTest.cs

@@ -289,6 +289,22 @@ public void TestLoopDetected()
             var lgFile = GetTemplates("LoopDetected.lg");
             var exception = Assert.Throws<InvalidOperationException>(() => lgFile.Evaluate("wPhrase"));
             Assert.Contains(TemplateErrors.LoopDetected, exception.Message);
+
+            // Without ThrowOnRecursive does not throw exception when loop is detected
+            var wPhraseResult = lgFile.AnalyzeTemplate("wPhrase");
+            Assert.Equal("welcome_user", wPhraseResult.TemplateReferences[0]);
+            Assert.Equal("wPhrase", wPhraseResult.TemplateReferences[1]);
+
+            var selfLoopResult = lgFile.AnalyzeTemplate("selfLoop");
+            Assert.Equal("selfLoop", selfLoopResult.TemplateReferences[0]);
+            Assert.Equal("x", selfLoopResult.Variables[0]);
+
+            // ThrowOnRecursive throws InvalidOperationException
+            exception = Assert.Throws<InvalidOperationException>(() => lgFile.AnalyzeTemplate("wPhrase", new AnalyzerOptions() { ThrowOnRecursive = true }));
+            Assert.Contains(TemplateErrors.LoopDetected, exception.Message);            
+
+            exception = Assert.Throws<InvalidOperationException>(() => lgFile.AnalyzeTemplate("selfLoop", new AnalyzerOptions() { ThrowOnRecursive = true }));
+            Assert.Contains(TemplateErrors.LoopDetected, exception.Message);
         }
 
         [Fact]