Skip to content

Commit

Permalink
Fix wildcard semantics; fix like clause anchoring
Browse files Browse the repository at this point in the history
  • Loading branch information
nblumhardt committed Mar 7, 2021
1 parent e10eae6 commit 6ad474f
Show file tree
Hide file tree
Showing 14 changed files with 138 additions and 35 deletions.
2 changes: 2 additions & 0 deletions src/Serilog.Expressions/Expressions/Ast/Expression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@
{
abstract class Expression
{
// Used only as an enabler for testing and debugging.
public abstract override string ToString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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("'", "''")}')";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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 == '_')
Expand All @@ -87,7 +100,7 @@ static string LikeToRegex(string like)
regex += Regex.Escape(ch.ToString());
}

return regex;
return begin + regex + end;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/Serilog.Expressions/Expressions/SerilogExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 16 additions & 0 deletions test/Serilog.Expressions.Tests/Cases/translation-cases.asv
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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)

// 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')})))
14 changes: 1 addition & 13 deletions test/Serilog.Expressions.Tests/ExpressionEvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
29 changes: 29 additions & 0 deletions test/Serilog.Expressions.Tests/ExpressionTranslationTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
26 changes: 26 additions & 0 deletions test/Serilog.Expressions.Tests/Support/AsvCases.cs
Original file line number Diff line number Diff line change
@@ -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>();
}
}
}
14 changes: 1 addition & 13 deletions test/Serilog.Expressions.Tests/TemplateEvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down

0 comments on commit 6ad474f

Please sign in to comment.