From 2b95161d7f49cb1eb516980c36b9dc7b18725293 Mon Sep 17 00:00:00 2001 From: hond Date: Sat, 10 Oct 2020 17:30:10 +0800 Subject: [PATCH 1/5] throw exception when occurs loop reference --- .../TemplatesParser.cs | 30 ++++++++++++++----- .../ExceptionExamples/CycleRef1.lg | 4 +++ .../ExceptionExamples/CycleRef2.lg | 4 +++ ...ot.Builder.LanguageGeneration.Tests.csproj | 8 +++++ .../TemplateDiagnosticTest.cs | 8 +++++ 5 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg create mode 100644 tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef2.lg diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs index a1b56d8a18..03faf4207e 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs @@ -148,12 +148,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. + /// Traceability chain of the visited templates. /// new entity. public static Templates ParseResource( LGResource resource, ImportResolverDelegate importResolver = null, ExpressionParser expressionParser = null, - Dictionary cachedTemplates = null) + Dictionary cachedTemplates = null, + Stack history = null) { if (resource == null) { @@ -161,6 +163,7 @@ public static Templates ParseResource( } cachedTemplates = cachedTemplates ?? new Dictionary(); + history = history ?? new Stack(); if (cachedTemplates.ContainsKey(resource.Id)) { return cachedTemplates[resource.Id]; @@ -172,7 +175,7 @@ public static Templates ParseResource( try { lg = new TemplatesTransformer(lg).Transform(AntlrParseTemplates(resource)); - lg.References = GetReferences(lg, cachedTemplates); + lg.References = GetReferences(lg, cachedTemplates, history); new StaticChecker(lg).Check().ForEach(u => lg.Diagnostics.Add(u)); } catch (TemplateException ex) @@ -204,19 +207,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 history = null) { var resourcesFound = new HashSet(); - ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary()); + ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary(), history ?? 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 history) { resourcesFound.Add(start); - + history.Push(start); foreach (var import in start.Imports) { LGResource resource; @@ -231,6 +234,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 (history.Peek().Id != resource.Id && history.Any(u => u.Id == resource.Id)) + { + var errorMsg = TemplateErrors.LoopDetected + resource.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 +252,15 @@ private static void ResolveImportResources(Templates start, HashSet r } else { - childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates); + childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, history); cachedTemplates.Add(resource.Id, childResource); } - ResolveImportResources(childResource, resourcesFound, cachedTemplates); + ResolveImportResources(childResource, resourcesFound, cachedTemplates, history); } } + + history.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..f47c56f89b --- /dev/null +++ b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg @@ -0,0 +1,4 @@ +> Import CycleRef2.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; From fecc71fa8b022958d372472f5b5d84d3391ef3f0 Mon Sep 17 00:00:00 2001 From: hond Date: Sat, 10 Oct 2020 17:51:50 +0800 Subject: [PATCH 2/5] refine error code --- .../Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs | 2 +- .../ExceptionExamples/CycleRef1.lg | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs index 03faf4207e..d692e2045a 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs @@ -238,7 +238,7 @@ private static void ResolveImportResources(Templates start, HashSet r // Import self is allowed, and would ignore it. if (history.Peek().Id != resource.Id && history.Any(u => u.Id == resource.Id)) { - var errorMsg = TemplateErrors.LoopDetected + 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 }); } diff --git a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg index f47c56f89b..f257620d7a 100644 --- a/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg +++ b/tests/Microsoft.Bot.Builder.LanguageGeneration.Tests/ExceptionExamples/CycleRef1.lg @@ -1,4 +1,5 @@ > Import CycleRef2.lg +[import](./ImportFile.lg) [import](./CycleRef2.lg) # a - a \ No newline at end of file From e78655c922a046918345731709d41555ea22f3e5 Mon Sep 17 00:00:00 2001 From: hond Date: Sat, 10 Oct 2020 17:57:25 +0800 Subject: [PATCH 3/5] rename --- .../TemplatesParser.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs index d692e2045a..9f24257215 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs @@ -148,14 +148,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. - /// Traceability chain of the visited templates. + /// Ancestor visited Templates. /// new entity. public static Templates ParseResource( LGResource resource, ImportResolverDelegate importResolver = null, ExpressionParser expressionParser = null, Dictionary cachedTemplates = null, - Stack history = null) + Stack ancestorTemplates = null) { if (resource == null) { @@ -163,7 +163,7 @@ public static Templates ParseResource( } cachedTemplates = cachedTemplates ?? new Dictionary(); - history = history ?? new Stack(); + ancestorTemplates = ancestorTemplates ?? new Stack(); if (cachedTemplates.ContainsKey(resource.Id)) { return cachedTemplates[resource.Id]; @@ -175,7 +175,7 @@ public static Templates ParseResource( try { lg = new TemplatesTransformer(lg).Transform(AntlrParseTemplates(resource)); - lg.References = GetReferences(lg, cachedTemplates, history); + lg.References = GetReferences(lg, cachedTemplates, ancestorTemplates); new StaticChecker(lg).Check().ForEach(u => lg.Diagnostics.Add(u)); } catch (TemplateException ex) @@ -207,19 +207,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, Stack history = null) + private static IList GetReferences(Templates file, Dictionary cachedTemplates = null, Stack ancestorTemplates = null) { var resourcesFound = new HashSet(); - ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary(), history ?? new Stack()); + ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary(), ancestorTemplates ?? new Stack()); resourcesFound.Remove(file); return resourcesFound.ToList(); } - private static void ResolveImportResources(Templates start, HashSet resourcesFound, Dictionary cachedTemplates, Stack history) + private static void ResolveImportResources(Templates start, HashSet resourcesFound, Dictionary cachedTemplates, Stack ancestorTemplates) { resourcesFound.Add(start); - history.Push(start); + ancestorTemplates.Push(start); foreach (var import in start.Imports) { LGResource resource; @@ -236,7 +236,7 @@ private static void ResolveImportResources(Templates start, HashSet r // Cycle reference would throw exception to avoid infinite Loop. // Import self is allowed, and would ignore it. - if (history.Peek().Id != resource.Id && history.Any(u => u.Id == resource.Id)) + if (ancestorTemplates.Peek().Id != resource.Id && ancestorTemplates.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); @@ -252,15 +252,15 @@ private static void ResolveImportResources(Templates start, HashSet r } else { - childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, history); + childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, ancestorTemplates); cachedTemplates.Add(resource.Id, childResource); } - ResolveImportResources(childResource, resourcesFound, cachedTemplates, history); + ResolveImportResources(childResource, resourcesFound, cachedTemplates, ancestorTemplates); } } - history.Pop(); + ancestorTemplates.Pop(); } /// From 358ffdbe94e82fc370cd619002667357a0ebfbd8 Mon Sep 17 00:00:00 2001 From: hond Date: Mon, 12 Oct 2020 17:38:02 +0800 Subject: [PATCH 4/5] rename --- .../TemplatesParser.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs index 9f24257215..93d996d185 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs @@ -148,14 +148,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. - /// Ancestor visited Templates. + /// Parent visited Templates. /// new entity. public static Templates ParseResource( LGResource resource, ImportResolverDelegate importResolver = null, ExpressionParser expressionParser = null, Dictionary cachedTemplates = null, - Stack ancestorTemplates = null) + Stack parentTemplates = null) { if (resource == null) { @@ -163,7 +163,7 @@ public static Templates ParseResource( } cachedTemplates = cachedTemplates ?? new Dictionary(); - ancestorTemplates = ancestorTemplates ?? new Stack(); + parentTemplates = parentTemplates ?? new Stack(); if (cachedTemplates.ContainsKey(resource.Id)) { return cachedTemplates[resource.Id]; @@ -175,7 +175,7 @@ public static Templates ParseResource( try { lg = new TemplatesTransformer(lg).Transform(AntlrParseTemplates(resource)); - lg.References = GetReferences(lg, cachedTemplates, ancestorTemplates); + lg.References = GetReferences(lg, cachedTemplates, parentTemplates); new StaticChecker(lg).Check().ForEach(u => lg.Diagnostics.Add(u)); } catch (TemplateException ex) @@ -207,19 +207,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, Stack ancestorTemplates = null) + private static IList GetReferences(Templates file, Dictionary cachedTemplates = null, Stack parentTemplates = null) { var resourcesFound = new HashSet(); - ResolveImportResources(file, resourcesFound, cachedTemplates ?? new Dictionary(), ancestorTemplates ?? new Stack()); + 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, Stack ancestorTemplates) + private static void ResolveImportResources(Templates start, HashSet resourcesFound, Dictionary cachedTemplates, Stack parentTemplates) { resourcesFound.Add(start); - ancestorTemplates.Push(start); + parentTemplates.Push(start); foreach (var import in start.Imports) { LGResource resource; @@ -236,7 +236,7 @@ private static void ResolveImportResources(Templates start, HashSet r // Cycle reference would throw exception to avoid infinite Loop. // Import self is allowed, and would ignore it. - if (ancestorTemplates.Peek().Id != resource.Id && ancestorTemplates.Any(u => u.Id == resource.Id)) + 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); @@ -252,15 +252,15 @@ private static void ResolveImportResources(Templates start, HashSet r } else { - childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, ancestorTemplates); + childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, parentTemplates); cachedTemplates.Add(resource.Id, childResource); } - ResolveImportResources(childResource, resourcesFound, cachedTemplates, ancestorTemplates); + ResolveImportResources(childResource, resourcesFound, cachedTemplates, parentTemplates); } } - ancestorTemplates.Pop(); + parentTemplates.Pop(); } /// From 8eb8e187b1bb6f084865b0a818de775be97f8b9a Mon Sep 17 00:00:00 2001 From: hond Date: Wed, 14 Oct 2020 16:41:39 +0800 Subject: [PATCH 5/5] Add InnerParseResource --- .../TemplatesParser.cs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/TemplatesParser.cs index 93d996d185..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. /// @@ -150,7 +165,7 @@ public static IParseTree AntlrParseTemplates(LGResource resource) /// 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, @@ -252,7 +267,7 @@ private static void ResolveImportResources(Templates start, HashSet r } else { - childResource = ParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, parentTemplates); + childResource = InnerParseResource(resource, start.ImportResolver, start.ExpressionParser, cachedTemplates, parentTemplates); cachedTemplates.Add(resource.Id, childResource); }