diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs index a1b56d8a18..1eb450071a 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs @@ -81,6 +81,21 @@ public static Templates ParseText( return ParseResource(resource, importResolver, expressionParser); } + /// + /// Parser to turn lg content into a . + /// + /// LG resource. + /// Resolver to resolve LG import id to template text. + /// Expression parser for parsing expressions. + /// new entity. + public static Templates ParseResource( + LGResource resource, + ImportResolverDelegate importResolver = null, + ExpressionParser expressionParser = null) + { + return InnerParseResource(resource, importResolver, expressionParser); + } + /// /// Parser to turn lg content into a based on the original LGFile. /// @@ -148,12 +163,14 @@ public static IParseTree AntlrParseTemplates(LGResource resource) /// Resolver to resolve LG import id to template text. /// Expression parser for parsing expressions. /// Give the file path and templates to avoid parsing and to improve performance. + /// Parent visited Templates. /// new entity. - public static Templates ParseResource( + private static Templates InnerParseResource( LGResource resource, ImportResolverDelegate importResolver = null, ExpressionParser expressionParser = null, - Dictionary cachedTemplates = null) + Dictionary cachedTemplates = null, + Stack parentTemplates = null) { if (resource == null) { @@ -161,6 +178,7 @@ public static Templates ParseResource( } cachedTemplates = cachedTemplates ?? new Dictionary(); + parentTemplates = parentTemplates ?? new Stack(); 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 GetReferences(Templates file, Dictionary cachedTemplates = null) + private static IList GetReferences(Templates file, Dictionary cachedTemplates = null, Stack parentTemplates = null) { var resourcesFound = new HashSet(); - ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary()); + ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary(), parentTemplates ?? new Stack()); resourcesFound.Remove(file); return resourcesFound.ToList(); } - private static void ResolveImportResources(Templates start, HashSet resourcesFound, Dictionary cachedTemplates) + private static void ResolveImportResources(Templates start, HashSet resourcesFound, Dictionary cachedTemplates, Stack 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 r throw new TemplateException(e.Message, new List() { 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 }); + } + if (resourcesFound.All(u => u.Id != resource.Id)) { Templates childResource; @@ -240,13 +267,15 @@ private static void ResolveImportResources(Templates start, HashSet 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(); } /// diff --git a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg new file mode 100644 index 0000000000..f257620d7a --- /dev/null +++ b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg @@ -0,0 +1,5 @@ +> Import CycleRef2.lg +[import](./ImportFile.lg) +[import](./CycleRef2.lg) +# a +- a \ No newline at end of file diff --git a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef2.lg b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef2.lg new file mode 100644 index 0000000000..245bec7962 --- /dev/null +++ b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef2.lg @@ -0,0 +1,4 @@ +> Import CycleRef1.lg +[import](./CycleRef1.lg) +# b +- b \ No newline at end of file diff --git a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/Microsoft.Bot.Builder.LanguageGeneration.Tests.csproj b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/Microsoft.Bot.Builder.LanguageGeneration.Tests.csproj index c8eb428b6f..15e191155b 100644 --- a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/Microsoft.Bot.Builder.LanguageGeneration.Tests.csproj +++ b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/Microsoft.Bot.Builder.LanguageGeneration.Tests.csproj @@ -92,6 +92,8 @@ + + @@ -334,6 +336,12 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest + PreserveNewest diff --git a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/TemplateDiagnosticTest.cs b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/TemplateDiagnosticTest.cs index f8353c05e1..eb350e1eff 100644 --- a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/TemplateDiagnosticTest.cs +++ b/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;