Cherry pick changes for .NET Templating 1.3.0 (#7450)

* [.NET Templating] Update Boolean expression evaluation (#7223) * Update expansion of Boolean expressions * Add unit test * Add test for property that accepts boolean * [.NET Templating] Resolve commas added in wrong place (#7225) * Update expansion of Boolean expressions * Add unit test * Add test for property that accepts boolean * Resolve extra commas in temaplting output * [Templating] Realign platforms on invalid $when behavior (#7432) * Align templating platforms for invalid boolean behavior * Formatting * Add comment for exposing warnings to caller * [Templating] Expose warnings/errors to callers (#7437) * In progress: log warnings * Error log for .NET * Add error logging for NodeJS * Resolve comments on PR * Consolidate ArrayList declarations and fix testing nuget package * Rearrange Expand methods and add comments * Refactor to add the errors to template instance * Update warning message * Get warnings from last template expansion * Add markdown file changes Co-authored-by: Paul Campbell <paulcam@microsoft.com>
microsoft · May 13, 2022 · 6c7642f · 6c7642f
1 parent 34126f8
commit 6c7642f
Show file tree

Hide file tree

Showing 5 changed files with 351 additions and 14 deletions.
diff --git a/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardTemplate.cs b/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardTemplate.cs
@@ -4,6 +4,7 @@
 using Antlr4.Runtime.Tree;
 using Newtonsoft.Json;
 using System;
+using System.Collections;
 
 namespace AdaptiveCards.Templating
 {
@@ -21,6 +22,7 @@ public sealed class AdaptiveCardTemplate
     {
         private IParseTree parseTree;
         private string jsonTemplateString;
+        private ArrayList templateExpansionWarnings;
 
         /// <summary>
         /// <para>Creates an instance of AdaptiveCardTemplate</para>
@@ -109,7 +111,11 @@ public string Expand(EvaluationContext context, Func<string, object> nullSubstit
             }
 
             AdaptiveCardsTemplateVisitor eval = new AdaptiveCardsTemplateVisitor(nullSubstitutionOption, jsonData);
-            return eval.Visit(parseTree).ToString();
+            AdaptiveCardsTemplateResult result = eval.Visit(parseTree);
+
+            templateExpansionWarnings = eval.getTemplateVisitorWarnings();
+
+            return result.ToString();
         }
 
         /// <summary>
@@ -135,8 +141,20 @@ public string Expand(EvaluationContext context, Func<string, object> nullSubstit
         public string Expand(object rootData, Func<string, object> nullSubstitutionOption = null)
         {
             var context = new EvaluationContext(rootData);
-
             return Expand(context, nullSubstitutionOption);
         }
+
+        /// <summary>
+        /// Getter method for the array of warning strings from the last template expansion
+        /// </summary>
+        /// <returns>ArrayList</returns>
+        public ArrayList GetLastTemplateExpansionWarnings()
+        {
+            if (templateExpansionWarnings != null)
+            {
+                return templateExpansionWarnings;
+            }
+            return new ArrayList();
+        }
     }
 }
diff --git a/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardsTemplateResult.cs b/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardsTemplateResult.cs
@@ -37,7 +37,7 @@ public enum EvaluationResult
         /// <summary>
         /// Indicates that this instance captures the result of $when
         /// </summary>
-        public bool IsWhen { get; }
+        public bool IsWhen { get; set; }
         /// <summary>
         /// Predicate of $when expression
         /// </summary>

diff --git a/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardsTemplateVisitor.cs b/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardsTemplateVisitor.cs
@@ -8,6 +8,7 @@
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
 using System.Text;
@@ -23,6 +24,7 @@ public sealed class AdaptiveCardsTemplateVisitor : AdaptiveCardsTemplateParserBa
         private Stack<DataContext> dataContext = new Stack<DataContext>();
         private readonly JToken root;
         private readonly Options options;
+        private ArrayList templateVisitorWarnings;
 
         /// <summary>
         /// maintains data context
@@ -126,6 +128,8 @@ public AdaptiveCardsTemplateVisitor(Func<string, object> nullSubstitutionOption,
             {
                 NullSubstitution = nullSubstitutionOption != null? nullSubstitutionOption : (path) => $"${{{path}}}"
             };
+
+            templateVisitorWarnings = new ArrayList();
         }
 
         /// <summary>
@@ -198,6 +202,15 @@ private bool HasDataContext()
             return dataContext.Count != 0;
         }
 
+        /// <summary>
+        /// Getter for templateVisitorWarnings
+        /// </summary>
+        /// <returns>ArrayList</returns>
+        public ArrayList getTemplateVisitorWarnings()
+        {
+            return templateVisitorWarnings;
+        }
+
         /// <summary>
         /// antlr runtime wil call this method when parse tree's context is <see cref="AdaptiveCardsTemplateParser.TemplateDataContext"/>
         /// <para>It is used in parsing a pair that has $data as key</para>
@@ -336,8 +349,20 @@ public override AdaptiveCardsTemplateResult VisitValueTemplateExpression([NotNul
                 return result;
             }
 
+            bool isTrue = false;
+
+            try
+            {
+                isTrue = IsTrue(result.Predicate, dataContext.token);
+            }
+            catch (System.FormatException)
+            {
+                templateVisitorWarnings.Add($"WARN: Could not evaluate {result.Predicate} because it could not be found in the provided data. " +
+                                    "The condition has been set to false by default.");
+            }
+
             // evaluate $when
-            result.WhenEvaluationResult = IsTrue(result.Predicate, dataContext.token) ?
+            result.WhenEvaluationResult = isTrue ?
                 AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToTrue :
                 AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToFalse;
 
@@ -440,6 +465,7 @@ public override AdaptiveCardsTemplateResult VisitObj([NotNull] AdaptiveCardsTemp
             }
 
             int repeatsCounts = 1;
+            bool isObjAdded = false;
             var dataContext = GetCurrentDataContext();
 
             if (isArrayType && hasDataContext)
@@ -452,7 +478,7 @@ public override AdaptiveCardsTemplateResult VisitObj([NotNull] AdaptiveCardsTemp
             // indicates the number of removed json object(s)
             int removedCounts = 0;
             var comma = context.COMMA();
-            string jsonPairDelimieter = (comma != null && comma.Length > 0) ? comma[0].GetText() : "";
+            string jsonPairDelimiter = (comma != null && comma.Length > 0) ? comma[0].GetText() : "";
 
             // loop for repeating obj parsed in the inner loop
             for (int iObj = 0; iObj < repeatsCounts; iObj++)
@@ -487,14 +513,26 @@ public override AdaptiveCardsTemplateResult VisitObj([NotNull] AdaptiveCardsTemp
                         // a pair after first pair is added
                         if (isPairAdded && !returnedResult.IsWhen)
                         {
-                            result.Append(jsonPairDelimieter);
+                            result.Append(jsonPairDelimiter);
                         }
 
                         result.Append(returnedResult);
 
                         if (returnedResult.IsWhen)
                         {
-                            whenEvaluationResult = returnedResult.WhenEvaluationResult;
+                            if (returnedResult.WhenEvaluationResult == AdaptiveCardsTemplateResult.EvaluationResult.NotEvaluated)
+                            {
+                                // The when expression could not be evaluated, so we are defaulting the value to false
+                                whenEvaluationResult = AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToFalse;
+
+                                templateVisitorWarnings.Add($"WARN: Could not evaluate {returnedResult} because it is not an expression or the " +
+                                    $"expression is invalid. The $when condition has been set to false by default.");
+
+                            }
+                            else
+                            {
+                                whenEvaluationResult = returnedResult.WhenEvaluationResult;
+                            }
                         }
                         else
                         {
@@ -508,11 +546,14 @@ public override AdaptiveCardsTemplateResult VisitObj([NotNull] AdaptiveCardsTemp
 
                 if (whenEvaluationResult != AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToFalse)
                 {
-                    if (iObj != repeatsCounts - 1)
+                    if (isObjAdded)
                     {
-                        result.Append(jsonPairDelimieter);
+                        // add a delimiter, e.g ',' before appending
+                        // another object after first object is added
+                        combinedResult.Append(jsonPairDelimiter);
                     }
                     combinedResult.Append(result);
+                    isObjAdded = true;
                 }
                 else
                 {
@@ -630,7 +671,12 @@ public static string Expand(string unboundString, IMemory data, bool isTemplated
                     result.Append('"');
                     result.Append(value);
                     result.Append('"');
-                } else
+                }
+                else if (value is Boolean)
+                {
+                    result.Append(value.ToString().ToLower());
+                }
+                else
                 {
                     result.Append(value);
                 }
@@ -658,6 +704,15 @@ public override AdaptiveCardsTemplateResult VisitTemplateWhen([NotNull] Adaptive
             // this node is visited only when parsing was correctly done
             // [ '{', '$when', ':', ',', 'expression'] 
             var result = Visit(context.templateExpression());
+
+            if (!result.IsWhen)
+            {
+                // We know that this result was supposed to be IsWhen since it is called from VisitTemplateWhen
+                // We create a result with `IsWhen = false` if the expression is invalid
+                // Result will now correctly follow the rest of the IsWhen logic
+                result.IsWhen = true;
+            }
+
             return result;
         }
 
@@ -681,18 +736,26 @@ public override AdaptiveCardsTemplateResult VisitArray([NotNull] AdaptiveCardsTe
             AdaptiveCardsTemplateResult result = new AdaptiveCardsTemplateResult(context.LSB().GetText());
             var values = context.value();
             var arrayDelimiters = context.COMMA();
+            bool isValueAdded = false;
 
             // visit each json value in json array and integrate parsed result
             for (int i = 0; i < values.Length; i++)
             {
                 var value = context.value(i);
                 var parsedResult = Visit(value);
-                result.Append(parsedResult);
-                // only add delimiter when parsedResult has not been dropped, and delimiter is needed
-                if (!parsedResult.HasItBeenDropped && i != values.Length - 1 && arrayDelimiters.Length > 0)
+
+                // only add delimiter when parsedResult has not been dropped,
+                // and a value has already been added to the array
+                if (isValueAdded && !parsedResult.HasItBeenDropped && arrayDelimiters.Length > 0)
                 {
                     result.Append(arrayDelimiters[0].GetText());
                 }
+
+                if (!parsedResult.HasItBeenDropped)
+                {
+                    result.Append(parsedResult);
+                    isValueAdded = true;
+                }
             }
 
             result.Append(context.RSB().GetText());

diff --git a/source/dotnet/Library/AdaptiveCards.Templating/docs/AdaptiveCardsTemplate.md b/source/dotnet/Library/AdaptiveCards.Templating/docs/AdaptiveCardsTemplate.md
@@ -7,6 +7,7 @@
   - [#ctor(jsonTemplate)](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-#ctor-System-Object- 'AdaptiveCards.Templating.AdaptiveCardTemplate.#ctor(System.Object)')
   - [Expand(context,nullSubstitutionOption)](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-Expand-AdaptiveCards-Templating-EvaluationContext,System-Func{System-String,System-Object}- 'AdaptiveCards.Templating.AdaptiveCardTemplate.Expand(AdaptiveCards.Templating.EvaluationContext,System.Func{System.String,System.Object})')
   - [Expand(rootData,nullSubstitutionOption)](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-Expand-System-Object,System-Func{System-String,System-Object}- 'AdaptiveCards.Templating.AdaptiveCardTemplate.Expand(System.Object,System.Func{System.String,System.Object})')
+  - [GetLastTemplateExpansionWarnings()](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-GetLastTemplateExpansionWarnings 'AdaptiveCards.Templating.AdaptiveCardTemplate.GetLastTemplateExpansionWarnings')
 - [AdaptiveCardsTemplateParserBaseVisitor\`1](#T-AdaptiveCardsTemplateParserBaseVisitor`1 'AdaptiveCardsTemplateParserBaseVisitor`1')
   - [VisitArray(context)](#M-AdaptiveCardsTemplateParserBaseVisitor`1-VisitArray-AdaptiveCardsTemplateParser-ArrayContext- 'AdaptiveCardsTemplateParserBaseVisitor`1.VisitArray(AdaptiveCardsTemplateParser.ArrayContext)')
   - [VisitJson(context)](#M-AdaptiveCardsTemplateParserBaseVisitor`1-VisitJson-AdaptiveCardsTemplateParser-JsonContext- 'AdaptiveCardsTemplateParserBaseVisitor`1.VisitJson(AdaptiveCardsTemplateParser.JsonContext)')
@@ -65,6 +66,7 @@
   - [VisitValueObject(context)](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-VisitValueObject-AdaptiveCardsTemplateParser-ValueObjectContext- 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.VisitValueObject(AdaptiveCardsTemplateParser.ValueObjectContext)')
   - [VisitValueTemplateExpression(context)](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-VisitValueTemplateExpression-AdaptiveCardsTemplateParser-ValueTemplateExpressionContext- 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.VisitValueTemplateExpression(AdaptiveCardsTemplateParser.ValueTemplateExpressionContext)')
   - [VisitValueTemplateString(context)](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-VisitValueTemplateString-AdaptiveCardsTemplateParser-ValueTemplateStringContext- 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.VisitValueTemplateString(AdaptiveCardsTemplateParser.ValueTemplateStringContext)')
+  - [getTemplateVisitorWarnings()](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-getTemplateVisitorWarnings 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.getTemplateVisitorWarnings')
 - [AdaptiveTemplateException](#T-AdaptiveCards-Templating-AdaptiveTemplateException 'AdaptiveCards.Templating.AdaptiveTemplateException')
   - [#ctor()](#M-AdaptiveCards-Templating-AdaptiveTemplateException-#ctor 'AdaptiveCards.Templating.AdaptiveTemplateException.#ctor')
   - [#ctor(message)](#M-AdaptiveCards-Templating-AdaptiveTemplateException-#ctor-System-String- 'AdaptiveCards.Templating.AdaptiveTemplateException.#ctor(System.String)')
@@ -239,6 +241,21 @@ Default behavior is leaving templated string unchanged
 
 - [AdaptiveCards.Templating.EvaluationContext](#T-AdaptiveCards-Templating-EvaluationContext 'AdaptiveCards.Templating.EvaluationContext')
 
+<a name='M-AdaptiveCards-Templating-AdaptiveCardTemplate-GetLastTemplateExpansionWarnings'></a>
+### GetLastTemplateExpansionWarnings() `method`
+
+##### Summary
+
+Getter method for the array of warning strings from the last template expansion
+
+##### Returns
+
+ArrayList
+
+##### Parameters
+
+This method has no parameters.
+
 <a name='T-AdaptiveCardsTemplateParserBaseVisitor`1'></a>
 ## AdaptiveCardsTemplateParserBaseVisitor\`1 `type`
 
@@ -1131,6 +1148,21 @@ Visitor method for `valueTemplateString` grammar rule `AdaptiveCardsTemplatePars
 | ---- | ---- | ----------- |
 | context | [AdaptiveCardsTemplateParser.ValueTemplateStringContext](#T-AdaptiveCardsTemplateParser-ValueTemplateStringContext 'AdaptiveCardsTemplateParser.ValueTemplateStringContext') |  |
 
+<a name='M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-getTemplateVisitorWarnings'></a>
+### getTemplateVisitorWarnings() `method`
+
+##### Summary
+
+Getter for templateVisitorWarnings
+
+##### Returns
+
+ArrayList
+
+##### Parameters
+
+This method has no parameters.
+
 <a name='T-AdaptiveCards-Templating-AdaptiveTemplateException'></a>
 ## AdaptiveTemplateException `type`