Skip to content

Commit fcf1cff

Browse files
nblumhardtnblumhardt
committed
Template parser error reporting
1 parent 327885b commit fcf1cff

File tree

5 files changed

+121
-41
lines changed

5 files changed

+121
-41
lines changed

‎src/Serilog.Expressions/Expressions/Compilation/Linq/LinqExpressionCompiler.cs

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,32 +61,25 @@ protected override ExpressionBody Transform(CallExpression lx)
6161

6262
// `and` and `or` short-circuit to save execution time; unlike the earlier Serilog.Filters.Expressions, nothing else does.
6363
if (Operators.SameOperator(lx.OperatorName, Operators.OpAnd))
64-
{
65-
return
66-
LX.Convert(
67-
LX.New(
68-
typeof(ScalarValue).GetConstructor(new[]{typeof(object)})!,
69-
LX.Convert(LX.AndAlso(
70-
LX.Call(CoerceToScalarBooleanMethod, operands[0]),
71-
LX.Call(CoerceToScalarBooleanMethod, operands[1])), typeof(object))),
72-
typeof(LogEventPropertyValue));
73-
}
64+
return CompileLogical(LX.AndAlso, operands[0], operands[1]);
7465

7566
if (Operators.SameOperator(lx.OperatorName, Operators.OpOr))
76-
{
77-
return
78-
LX.Convert(
79-
LX.New(
80-
typeof(ScalarValue).GetConstructor(new[]{typeof(object)})!,
81-
LX.Convert(LX.OrElse(
82-
LX.Call(CoerceToScalarBooleanMethod, operands[0]),
83-
LX.Call(CoerceToScalarBooleanMethod, operands[1])), typeof(object))),
84-
typeof(LogEventPropertyValue));
85-
}
67+
return CompileLogical(LX.OrElse, operands[0], operands[1]);
8668

8769
return LX.Call(m, operands);
8870
}
8971

72+
static ExpressionBody CompileLogical(Func<ExpressionBody, ExpressionBody, ExpressionBody> apply, ExpressionBody lhs, ExpressionBody rhs)
73+
{
74+
return LX.Convert(
75+
LX.New(
76+
typeof(ScalarValue).GetConstructor(new[]{typeof(object)})!,
77+
LX.Convert(apply(
78+
LX.Call(CoerceToScalarBooleanMethod, lhs),
79+
LX.Call(CoerceToScalarBooleanMethod, rhs)), typeof(object))),
80+
typeof(LogEventPropertyValue));
81+
}
82+
9083
protected override ExpressionBody Transform(AccessorExpression spx)
9184
{
9285
var recv = Transform(spx.Receiver);

‎src/Serilog.Expressions/Templates/OutputTemplate.cs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,42 @@ namespace Serilog.Templates
1212
/// </summary>
1313
public class OutputTemplate : ITextFormatter
1414
{
15+
/// <summary>
16+
/// Construct an <see cref="OutputTemplate"/>.
17+
/// </summary>
18+
/// <param name="template">The template text.</param>
19+
/// <param name="formatProvider">Optionally, an <see cref="IFormatProvider"/> to use when formatting
20+
/// embedded values.</param>
21+
/// <param name="result">The parsed template, if successful.</param>
22+
/// <param name="error">A description of the error, if unsuccessful.</param>
23+
/// <returns><c langword="true">true</c> if the template was well-formed.</returns>
24+
public static bool TryParse(
25+
string template,
26+
IFormatProvider formatProvider,
27+
out OutputTemplate result,
28+
out string error)
29+
{
30+
if (template == null) throw new ArgumentNullException(nameof(template));
31+
32+
if (!TemplateParser.TryParse(template, out var parsed, out error))
33+
{
34+
result = null;
35+
return false;
36+
}
37+
38+
result = new OutputTemplate(TemplateCompiler.Compile(parsed), formatProvider);
39+
return true;
40+
}
41+
1542
readonly IFormatProvider _formatProvider;
1643
readonly CompiledTemplate _compiled;
1744

45+
OutputTemplate(CompiledTemplate compiled, IFormatProvider formatProvider)
46+
{
47+
_compiled = compiled;
48+
_formatProvider = formatProvider;
49+
}
50+
1851
/// <summary>
1952
/// Construct an <see cref="OutputTemplate"/>.
2053
/// </summary>
@@ -23,9 +56,13 @@ public class OutputTemplate : ITextFormatter
2356
/// embedded values.</param>
2457
public OutputTemplate(string template, IFormatProvider formatProvider = null)
2558
{
26-
_formatProvider = formatProvider;
27-
var parsed = TemplateParser.Parse(template);
59+
if (template == null) throw new ArgumentNullException(nameof(template));
60+
61+
if (!TemplateParser.TryParse(template, out var parsed, out var error))
62+
throw new ArgumentException(error);
63+
2864
_compiled = TemplateCompiler.Compile(parsed);
65+
_formatProvider = formatProvider;
2966
}
3067

3168
/// <inheritdoc />

‎src/Serilog.Expressions/Templates/Parsing/TemplateParser.cs

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ static class TemplateParser
1212
{
1313
static ExpressionTokenizer Tokenizer { get; } = new ExpressionTokenizer();
1414

15-
public static Template Parse(string template)
15+
public static bool TryParse(string template, out Template parsed, out string error)
1616
{
1717
if (template == null) throw new ArgumentNullException(nameof(template));
18-
18+
19+
parsed = null;
1920
var elements = new List<Template>();
2021

2122
var i = 0;
@@ -27,32 +28,38 @@ public static Template Parse(string template)
2728
{
2829
i++;
2930
if (i == template.Length)
30-
throw new ArgumentException("Character `{` must be escaped by doubling in literal text.");
31+
{
32+
error = "Character `{` must be escaped by doubling in literal text.";
33+
return false;
34+
}
3135

3236
if (template[i] == '{')
3337
{
34-
elements.Add(new LiteralText("}"));
38+
elements.Add(new LiteralText("{"));
3539
i++;
3640
}
3741
else
3842
{
39-
// A lot of TODOs, here; exceptions thrown instead of error returns; positions of errors not
40-
// reported
41-
42-
// Not reporting line/column
43+
// No line/column tracking
4344
var tokens = Tokenizer.GreedyTokenize(new TextSpan(template, new Position(i, 0, 0), template.Length - i));
44-
// Dropping error info; may return a zero-length parse
4545
var expr = ExpressionTokenParsers.TryPartialParse(tokens);
4646
if (!expr.HasValue)
47-
throw new ArgumentException($"Invalid expression, {expr.FormatErrorMessageFragment()}.");
48-
47+
{
48+
// Error message accuracy is not great here
49+
error = $"Invalid expression, {expr.FormatErrorMessageFragment()}.";
50+
return false;
51+
}
52+
4953
if (expr.Remainder.Position == tokens.Count())
5054
i = tokens.Last().Position.Absolute + tokens.Last().Span.Length;
5155
else
5256
i = tokens.ElementAt(i).Position.Absolute;
5357

5458
if (i == template.Length)
55-
throw new ArgumentException("Un-closed hole, `}` expected.");
59+
{
60+
error = "Un-closed hole, `}` expected.";
61+
return false;
62+
}
5663

5764
string format = null;
5865
if (template[i] == ':')
@@ -68,9 +75,19 @@ public static Template Parse(string template)
6875

6976
format = formatBuilder.ToString();
7077
}
71-
72-
if (i == template.Length || template[i] != '}')
73-
throw new ArgumentException("Un-closed hole, `}` expected.");
78+
79+
if (i == template.Length)
80+
{
81+
error = "Un-closed hole, `}` expected.";
82+
return false;
83+
}
84+
85+
if (template[i] != '}')
86+
{
87+
error = $"Invalid expression, unexpected `{template[i]}`.";
88+
return false;
89+
}
90+
7491
i++;
7592

7693
elements.Add(new FormattedExpression(expr.Value, format));
@@ -80,7 +97,11 @@ public static Template Parse(string template)
8097
{
8198
i++;
8299
if (i == template.Length || template[i] != '}')
83-
throw new ArgumentException("Character `}` must be escaped by doubling in literal text.");
100+
{
101+
error = "Character `}` must be escaped by doubling in literal text.";
102+
return false;
103+
}
104+
84105
elements.Add(new LiteralText("}"));
85106
i++;
86107
}
@@ -97,9 +118,12 @@ public static Template Parse(string template)
97118
}
98119

99120
if (elements.Count == 1)
100-
return elements.Single();
101-
102-
return new TemplateBlock(elements.ToArray());
121+
parsed = elements.Single();
122+
else
123+
parsed = new TemplateBlock(elements.ToArray());
124+
125+
error = null;
126+
return true;
103127
}
104128
}
105129
}

‎test/Serilog.Expressions.Tests/Cases/template-evaluation-cases.asv

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,8 @@ Hello, {'world'}! ⇶ Hello, world!
33
{@l} ⇶ Information
44
Items are {[1, 2]} ⇶ Items are [1,2]
55
{@p} ⇶ {}
6+
Hello, {'my } brackety { {}} friends'}! ⇶ Hello, my } brackety { {}} friends!
7+
Text only ⇶ Text only
8+
{{ Escaped {{ left {{ ⇶ { Escaped { left {
9+
}} Escaped }} right }} ⇶ } Escaped } right }
10+
Formatted {42:0000} ⇶ Formatted 0042

‎test/Serilog.Expressions.Tests/TemplateParserTests.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using Serilog.Templates;
2+
using Xunit;
3+
4+
namespace Serilog.Expressions.Tests
5+
{
6+
public class TemplateParserTests
7+
{
8+
[Theory]
9+
[InlineData("Trailing {", "Character `{` must be escaped by doubling in literal text.")]
10+
[InlineData("Lonely } bracket", "Character `}` must be escaped by doubling in literal text.")]
11+
[InlineData("Trailing }", "Character `}` must be escaped by doubling in literal text.")]
12+
[InlineData("Unclosed {hole", "Un-closed hole, `}` expected.")]
13+
[InlineData("Syntax {+Err}or", "Invalid expression, unexpected operator `+`, expected expression.")]
14+
[InlineData("Syntax {1 + 2 and}or", "Invalid expression, unexpected end of input, expected expression.")]
15+
public void ErrorsAreReported(string input, string error)
16+
{
17+
Assert.False(OutputTemplate.TryParse(input, null, out _, out var actual));
18+
Assert.Equal(error, actual);
19+
}
20+
}
21+
}

0 commit comments

Comments
 (0)