Skip to content

Fix wildcard semantics; fix like clause anchoring #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for wildcard translation - stop searching upon reaching another comparison.


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
20 changes: 20 additions & 0 deletions test/Serilog.Expressions.Tests/Cases/translation-cases.asv
Original file line number Diff line number Diff line change
@@ -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')})))
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