Merge pull request #26 from nblumhardt/wildcard-assoc-fix

Fix wildcard semantics; fix like clause anchoring

Author: nblumhardt

src/Serilog.Expressions/Expressions/Ast/Expression.cs

@@ -2,5 +2,7 @@
 {
     abstract class Expression
     {
+        // Used only as an enabler for testing and debugging.
+        public abstract override string ToString();
     }
 }

src/Serilog.Expressions/Expressions/Ast/IndexOfMatchExpression.cs

@@ -13,5 +13,10 @@ public IndexOfMatchExpression(Expression corpus, Regex regex)
             Corpus = corpus ?? throw new ArgumentNullException(nameof(corpus));
             Regex = regex ?? throw new ArgumentNullException(nameof(regex));
         }
+
+        public override string ToString()
+        {
+            return $"_Internal_IndexOfMatch({Corpus}, '{Regex.ToString().Replace("'", "''")}')";
+        }
     }
 }

src/Serilog.Expressions/Expressions/Compilation/ExpressionCompiler.cs

@@ -10,7 +10,7 @@ namespace Serilog.Expressions.Compilation
 {
     static class ExpressionCompiler
     {
-        public static CompiledExpression Compile(Expression expression, NameResolver nameResolver)
+        public static Expression Translate(Expression expression)
         {
             var actual = expression;
             actual = VariadicCallRewriter.Rewrite(actual);
@@ -19,7 +19,12 @@ public static CompiledExpression Compile(Expression expression, NameResolver nam
             actual = PropertiesObjectAccessorTransformer.Rewrite(actual);
             actual = ConstantArrayEvaluator.Evaluate(actual);
             actual = WildcardComprehensionTransformer.Expand(actual);
-
+            return actual;
+        }
+        
+        public static CompiledExpression Compile(Expression expression, NameResolver nameResolver)
+        {
+            var actual = Translate(expression);
             return LinqExpressionCompiler.Compile(actual, nameResolver);
         }
     }

src/Serilog.Expressions/Expressions/Compilation/Text/LikeSyntaxTransformer.cs

@@ -54,7 +54,10 @@ cx.Constant is ScalarValue scalar &&
 
         static string LikeToRegex(string like)
         {
+            var begin = "^";
             var regex = "";
+            var end = "$";
+            
             for (var i = 0; i < like.Length; ++i)
             {
                 var ch = like[i];
@@ -68,7 +71,17 @@ static string LikeToRegex(string like)
                     }
                     else
                     {
-                        regex += "(?:.|\\r|\\n)*"; // ~= RegexOptions.Singleline
+                        if (i == 0)
+                            begin = "";
+                        
+                        if (i == like.Length - 1)
+                            end = "";
+                        
+                        if (i == 0 && i == like.Length - 1)
+                            regex += ".*";
+                        
+                        if (i != 0 && i != like.Length - 1)
+                            regex += "(?:.|\\r|\\n)*"; // ~= RegexOptions.Singleline
                     }
                 }
                 else if (ch == '_')
@@ -87,7 +100,7 @@ static string LikeToRegex(string like)
                     regex += Regex.Escape(ch.ToString());
             }
 
-            return regex;
+            return begin + regex + end;
         }
     }
 }

src/Serilog.Expressions/Expressions/Compilation/Wildcards/WildcardComprehensionTransformer.cs

@@ -14,6 +14,15 @@ public static Expression Expand(Expression root)
             return wc.Transform(root);
         }
 
+        // This matches expression fragments such as `A[?] = 'test'` and
+        // transforms them into `any(A, |p| p = 'test)`.
+        //
+        // As the comparand in such expressions can be complex, e.g.
+        // `Substring(A[?], 0, 4) = 'test')`, the search for `?` and `*` wildcards
+        // is deep, but, it terminates upon reaching any other wildcard-compatible
+        // comparison. Thus `(A[?] = 'test') = true` will result in `any(A, |p| p = 'test') = true` and
+        // not `any(A, |p| (p = 'test') = true)`, which is important because short-circuiting when the first
+        // argument to `any()` is undefined will change the semantics of the resulting expression, otherwise.
         protected override Expression Transform(CallExpression lx)
         {
             if (!Operators.WildcardComparators.Contains(lx.OperatorName))
@@ -24,7 +33,7 @@ protected override Expression Transform(CallExpression lx)
             var indexerOperand = -1;
             for (var i = 0; i < lx.Operands.Length; ++i)
             {
-                indexer = WildcardSearch.FindElementAtWildcard(lx.Operands[i]);
+                indexer = WildcardSearch.FindWildcardIndexer(lx.Operands[i]);
                 if (indexer != null)
                 {
                     indexerOperand = i;
@@ -52,5 +61,19 @@ protected override Expression Transform(CallExpression lx)
             var call = new CallExpression(false, op, coll, lambda);
             return Transform(call);
         }
+        
+        // Detects and transforms standalone `A[?]` fragments that are not part of a comparision; these
+        // are effectively Boolean tests.
+        protected override Expression Transform(IndexerExpression ix)
+        {
+            if (!(ix.Index is IndexerWildcardExpression wx))
+                return base.Transform(ix);
+
+            var px = new ParameterExpression("p" + _nextParameter++);
+            var coll = Transform(ix.Receiver);
+            var lambda = new LambdaExpression(new[] { px }, px);
+            var op = Operators.ToRuntimeWildcardOperator(wx.Wildcard);
+            return new CallExpression(false, op, coll, lambda);
+        }
     }
 }

src/Serilog.Expressions/Expressions/Compilation/Wildcards/WildcardSearch.cs

@@ -8,7 +8,7 @@ class WildcardSearch : SerilogExpressionTransformer<IndexerExpression?>
     {
         static readonly WildcardSearch Instance = new WildcardSearch();
 
-        public static IndexerExpression? FindElementAtWildcard(Expression fx)
+        public static IndexerExpression? FindWildcardIndexer(Expression fx)
         {
             return Instance.Transform(fx);
         }
@@ -59,6 +59,11 @@ class WildcardSearch : SerilogExpressionTransformer<IndexerExpression?>
 
         protected override IndexerExpression? Transform(CallExpression lx)
         {
+            // If we hit a wildcard-compatible operation, then any wildcards within its operands "belong" to
+            // it and can't be the result of this search.
+            if (Operators.WildcardComparators.Contains(lx.OperatorName))
+                return null;
+    
             return lx.Operands.Select(Transform).FirstOrDefault(e => e != null);
         }
 

src/Serilog.Expressions/Expressions/Parsing/ExpressionParser.cs

@@ -8,9 +8,9 @@ static class ExpressionParser
     {
         static ExpressionTokenizer Tokenizer { get; } = new ExpressionTokenizer();
 
-        public static Expression Parse(string filterExpression)
+        public static Expression Parse(string expression)
         {
-            if (!TryParse(filterExpression, out var root, out var error))
+            if (!TryParse(expression, out var root, out var error))
                 throw new ArgumentException(error);
 
             return root;

src/Serilog.Expressions/Expressions/SerilogExpression.cs

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 using System;
-using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Serilog.Expressions.Compilation;

test/Serilog.Expressions.Tests/Cases/expression-evaluation-cases.asv

@@ -267,3 +267,7 @@ undefined() = undefined() ci         ⇶ undefined()
 'test' like '%s_'                    ⇶ true
 'test' like '%'                      ⇶ true
 'test' like 't%s%'                   ⇶ true
+'test' like 'es'                     ⇶ false
+'test' like '%'                      ⇶ true
+'test' like ''                       ⇶ false
+'' like ''                           ⇶ true

test/Serilog.Expressions.Tests/Cases/translation-cases.asv

@@ -0,0 +1,20 @@
+// Like
+A like 'a'                                    ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, '^a$'), -1)
+A like 'a%'                                   ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, '^a'), -1)
+A like '%a%'                                  ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, 'a'), -1)
+A like '%a'                                   ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, 'a$'), -1)
+A like '%a_b%'                                ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, 'a.b'), -1)
+A like 'a%b%'                                 ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, '^a(?:.|\r|\n)*b'), -1)
+A like '%'                                    ⇶ _Internal_NotEqual(_Internal_IndexOfMatch(A, '.*'), -1)
+
+// Root properties
+@p['Test']                                    ⇶ Test
+@p[Test]                                      ⇶ @p[Test]
+
+// Variadics
+coalesce(A, B, C, D)                          ⇶ coalesce(A, coalesce(B, coalesce(C, D)))
+
+// Wildcards!
+A[?]                                          ⇶ _Internal_Any(A, |$$p0| {$$p0})
+A or B[*]                                     ⇶ _Internal_Or(A, _Internal_All(B, |$$p0| {$$p0}))
+not (A is not null) or not (A[?] = 'a')       ⇶ _Internal_Or(_Internal_Not(_Internal_IsNotNull(A)), _Internal_Not(_Internal_Any(A, |$$p0| {_Internal_Equal($$p0, 'a')})))

test/Serilog.Expressions.Tests/ExpressionEvaluationTests.cs

@@ -11,20 +11,8 @@ namespace Serilog.Expressions.Tests
 {
     public class ExpressionEvaluationTests
     {
-        static readonly string CasesPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory!, "Cases");
-
-        static IEnumerable<object[]> ReadCases(string filename)
-        {
-            foreach (var line in File.ReadLines(Path.Combine(CasesPath, filename)))
-            {
-                var cols = line.Split("⇶", StringSplitOptions.RemoveEmptyEntries);
-                if (cols.Length == 2)
-                    yield return cols.Select(c => c.Trim()).ToArray<object>();
-            }
-        }
-
         public static IEnumerable<object[]> ExpressionEvaluationCases =>
-            ReadCases("expression-evaluation-cases.asv");
+            AsvCases.ReadCases("expression-evaluation-cases.asv");
 
         [Theory]
         [MemberData(nameof(ExpressionEvaluationCases))]

test/Serilog.Expressions.Tests/ExpressionTranslationTests.cs

@@ -0,0 +1,29 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using Serilog.Events;
+using Serilog.Expressions.Compilation;
+using Serilog.Expressions.Parsing;
+using Serilog.Expressions.Runtime;
+using Serilog.Expressions.Tests.Support;
+using Xunit;
+
+namespace Serilog.Expressions.Tests
+{
+    public class ExpressionTranslationTests
+    {
+        public static IEnumerable<object[]> ExpressionEvaluationCases =>
+            AsvCases.ReadCases("translation-cases.asv");
+
+        [Theory]
+        [MemberData(nameof(ExpressionEvaluationCases))]
+        public void ExpressionsAreCorrectlyTranslated(string expr, string expected)
+        {
+            var parsed = ExpressionParser.Parse(expr);
+            var translated = ExpressionCompiler.Translate(parsed);
+            var actual = translated.ToString();
+            Assert.Equal(expected, actual);
+        }
+    }
+}

test/Serilog.Expressions.Tests/Support/AsvCases.cs

@@ -0,0 +1,26 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+
+namespace Serilog.Expressions.Tests.Support
+{
+    // "Arrow-separated values ;-) ... convenient because the Unicode `⇶` character doesn't appear in
+    // any of the cases themselves, and so we ignore any need for special character escaping (which is
+    // troublesome when the language the cases are written in uses just about all special characters somehow
+    // or other!).
+    //
+    // The ASV format informally supports `//` comment lines, as long as they don't contain the arrow character.
+    static class AsvCases
+    {
+        static readonly string CasesPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory!, "Cases");
+
+        public static IEnumerable<object[]> ReadCases(string filename)
+        {
+            return from line in File.ReadLines(Path.Combine(CasesPath, filename))
+                select line.Split("⇶", StringSplitOptions.RemoveEmptyEntries) into cols
+                where cols.Length == 2
+                select cols.Select(c => c.Trim()).ToArray<object>();
+        }
+    }
+}

test/Serilog.Expressions.Tests/TemplateEvaluationTests.cs

@@ -10,20 +10,8 @@ namespace Serilog.Expressions.Tests
 {
     public class TemplateEvaluationTests
     {
-        static readonly string CasesPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory!, "Cases");
-
-        static IEnumerable<object[]> ReadCases(string filename)
-        {
-            foreach (var line in File.ReadLines(Path.Combine(CasesPath, filename)))
-            {
-                var cols = line.Split("⇶", StringSplitOptions.RemoveEmptyEntries);
-                if (cols.Length == 2)
-                    yield return cols.Select(c => c.Trim()).ToArray<object>();
-            }
-        }
-
         public static IEnumerable<object[]> TemplateEvaluationCases =>
-            ReadCases("template-evaluation-cases.asv");
+            AsvCases.ReadCases("template-evaluation-cases.asv");
 
         [Theory]
         [MemberData(nameof(TemplateEvaluationCases))]