Throw exception when circular reference is detected in LG imports

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

@@ -81,6 +81,21 @@ public static Templates ParseText(
             return ParseResource(resource, importResolver, expressionParser);
         }
 
+        /// <summary>
+        /// Parser to turn lg content into a <see cref="Templates"/>.
+        /// </summary>
+        /// <param name="resource">LG resource.</param>
+        /// <param name="importResolver">Resolver to resolve LG import id to template text.</param>
+        /// <param name="expressionParser">Expression parser for parsing expressions.</param>
+        /// <returns>new <see cref="Templates"/> entity.</returns>
+        public static Templates ParseResource(
+            LGResource resource,
+            ImportResolverDelegate importResolver = null,
+            ExpressionParser expressionParser = null)
+        {
+            return InnerParseResource(resource, importResolver, expressionParser);
+        }
+
         /// <summary>
         /// Parser to turn lg content into a <see cref="Templates"/> based on the original LGFile.
         /// </summary>
@@ -148,19 +163,22 @@ public static IParseTree AntlrParseTemplates(LGResource resource)
         /// <param name="importResolver">Resolver to resolve LG import id to template text.</param>
         /// <param name="expressionParser">Expression parser for parsing expressions.</param>
         /// <param name="cachedTemplates">Give the file path and templates to avoid parsing and to improve performance.</param>
+        /// <param name="parentTemplates">Parent visited Templates.</param>
         /// <returns>new <see cref="Templates"/> entity.</returns>
-        public static Templates ParseResource(
+        private static Templates InnerParseResource(
             LGResource resource,
             ImportResolverDelegate importResolver = null,
             ExpressionParser expressionParser = null,
-            Dictionary<string, Templates> cachedTemplates = null)
+            Dictionary<string, Templates> cachedTemplates = null,
+            Stack<Templates> parentTemplates = null)
         {
             if (resource == null)
             {
                 throw new ArgumentNullException(nameof(resource));
             }
 
             cachedTemplates = cachedTemplates ?? new Dictionary<string, Templates>();
+            parentTemplates = parentTemplates ?? new Stack<Templates>();
             if (cachedTemplates.ContainsKey(resource.Id))
             {
                 return cachedTemplates[resource.Id];
@@ -172,7 +190,7 @@ public static Templates ParseResource(
             try
             {
                 lg = new TemplatesTransformer(lg).Transform(AntlrParseTemplates(resource));
-                lg.References = GetReferences(lg, cachedTemplates);
+                lg.References = GetReferences(lg, cachedTemplates, parentTemplates);
                 new StaticChecker(lg).Check().ForEach(u => lg.Diagnostics.Add(u));
             }
             catch (TemplateException ex)
@@ -204,19 +222,19 @@ private static LGResource DefaultFileResolver(LGResource resource, string resour
             return new LGResource(importPath, importPath, File.ReadAllText(importPath));
         }
 
-        private static IList<Templates> GetReferences(Templates file, Dictionary<string, Templates> cachedTemplates = null)
+        private static IList<Templates> GetReferences(Templates file, Dictionary<string, Templates> cachedTemplates = null, Stack<Templates> parentTemplates = null)
         {
             var resourcesFound = new HashSet<Templates>();
-            ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary<string, Templates>());
+            ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary<string, Templates>(), parentTemplates ?? new Stack<Templates>());
 
             resourcesFound.Remove(file);
             return resourcesFound.ToList();
         }
 
-        private static void ResolveImportResources(Templates start, HashSet<Templates> resourcesFound, Dictionary<string, Templates> cachedTemplates)
+        private static void ResolveImportResources(Templates start, HashSet<Templates> resourcesFound, Dictionary<string, Templates> cachedTemplates, Stack<Templates> parentTemplates)
         {
             resourcesFound.Add(start);
-
+            parentTemplates.Push(start);
             foreach (var import in start.Imports)
             {
                 LGResource resource;
@@ -231,6 +249,15 @@ private static void ResolveImportResources(Templates start, HashSet<Templates> r
                     throw new TemplateException(e.Message, new List<Diagnostic>() { diagnostic });
                 }
 
+                // Cycle reference would throw exception to avoid infinite Loop.
+                // Import self is allowed, and would ignore it.
+                if (parentTemplates.Peek().Id != resource.Id && parentTemplates.Any(u => u.Id == resource.Id))
+                {
+                    var errorMsg = $"{TemplateErrors.LoopDetected} {resource.Id} => {start.Id}";
+                    var diagnostic = new Diagnostic(import.SourceRange.Range, errorMsg, DiagnosticSeverity.Error, start.Source);
+                    throw new TemplateException(errorMsg, new List<Diagnostic>() { diagnostic });
+                }
+
                 if (resourcesFound.All(u => u.Id != resource.Id))
                 {
                     Templates childResource;
@@ -240,13 +267,15 @@ private static void ResolveImportResources(Templates start, HashSet<Templates> r
                     }
                     else
                     {
-                        childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates);
+                        childResource = InnerParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, parentTemplates);
                         cachedTemplates.Add(resource.Id, childResource);
                     }
 
-                    ResolveImportResources(childResource, resourcesFound, cachedTemplates);
+                    ResolveImportResources(childResource, resourcesFound, cachedTemplates, parentTemplates);
                 }
             }
+
+            parentTemplates.Pop();
         }
 
         /// <summary>

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

@@ -0,0 +1,5 @@
+> Import CycleRef2.lg
+[import](./ImportFile.lg)
+[import](./CycleRef2.lg)
+# a
+- a

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

@@ -0,0 +1,4 @@
+> Import CycleRef1.lg
+[import](./CycleRef1.lg)
+# b
+- b

tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/Microsoft.Bot.Builder.LanguageGeneration.Tests.csproj

@@ -92,6 +92,8 @@
     <None Remove="ExceptionExamples\RunTimeErrors.lg" />
     <None Remove="ExceptionExamples\ExpressionFormatError.lg" />
     <None Remove="ExceptionExamples\ErrorLine.lg" />
+    <None Remove="ExceptionExamples\CycleRef1.lg" />
+    <None Remove="ExceptionExamples\CycleRef2.lg" />
     <None Remove="MultiLanguage\a.lg" />
     <None Remove="MultiLanguage\a.en.lg" />
   </ItemGroup>
@@ -334,6 +336,12 @@
     <Content Include="ExceptionExamples\ErrorLine.lg">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
+    <Content Include="ExceptionExamples\CycleRef1.lg">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
+    <Content Include="ExceptionExamples\CycleRef2.lg">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
     <Content Include="MultiLanguage\a.lg">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>

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

@@ -354,6 +354,14 @@ public void TestExpressionFormatError()
             Assert.Contains("Close } is missing in Expression", diagnostics[0].Message);
         }
 
+        [Fact]
+        public void TestLoopReference()
+        {
+            var diagnostics = GetDiagnostics("CycleRef1.lg");
+            Assert.Equal(1, diagnostics.Count);
+            Assert.StartsWith(TemplateErrors.LoopDetected, diagnostics[0].Message);
+        }
+
         private string GetExceptionExampleFilePath(string fileName)
         {
             return AppContext.BaseDirectory.Substring(0, AppContext.BaseDirectory.IndexOf("bin")) + "ExceptionExamples" + Path.DirectorySeparatorChar + fileName;