Skip to content

Commit

Permalink
Re-arranging some of the Try statements to ensure we get more meani…
Browse files Browse the repository at this point in the history
…ngful error messages when parsing `Expr` (this is the main entry point for almost anyone trying to parse expressions).
  • Loading branch information
djluck committed Nov 23, 2021
1 parent 3d415ad commit 4b786c0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
14 changes: 7 additions & 7 deletions src/PromQL.Parser/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,10 @@ from e in Parse.Ref(() => Expr).Between(Token.EqualTo(PromToken.LEFT_PAREN), Tok
select new ParenExpression(e);

public static TokenListParser<PromToken, Expr[]> FunctionArgs = Parse.Ref(() => Expr).ManyDelimitedBy(Token.EqualTo(PromToken.COMMA))
.Between(Token.EqualTo(PromToken.LEFT_PAREN), Token.EqualTo(PromToken.RIGHT_PAREN));
.Between(Token.EqualTo(PromToken.LEFT_PAREN).Try(), Token.EqualTo(PromToken.RIGHT_PAREN));

public static TokenListParser<PromToken, FunctionCall> FunctionCall =
from id in Token.EqualTo(PromToken.IDENTIFIER).Where(x => Functions.Names.Contains(x.ToStringValue()))
from id in Token.EqualTo(PromToken.IDENTIFIER).Where(x => Functions.Names.Contains(x.ToStringValue())).Try()
from args in FunctionArgs
select new FunctionCall(id.ToStringValue(), args.ToImmutableArray());

Expand Down Expand Up @@ -308,13 +308,13 @@ from rhs in Parse.Ref(() => Expr)
select new BinaryExpr(lhs, rhs, BinaryOperatorMap[op.Kind], vm);

public static TokenListParser<PromToken, (bool without, ImmutableArray<string> labels)> AggregateModifier =
from kind in Token.EqualTo(PromToken.BY)
.Or(Token.EqualTo(PromToken.WITHOUT))
from kind in Token.EqualTo(PromToken.BY).Try()
.Or(Token.EqualTo(PromToken.WITHOUT).Try())
from labels in GroupingLabels
select (kind.Kind == PromToken.WITHOUT, labels);

public static TokenListParser<PromToken, AggregateExpr> AggregateExpr =
from op in Token.EqualTo(PromToken.AGGREGATE_OP).Where(x => Operators.Aggregates.Contains(x.Span.ToStringValue()))
from op in Token.EqualTo(PromToken.AGGREGATE_OP).Where(x => Operators.Aggregates.Contains(x.Span.ToStringValue())).Try()
from argsAndMod in (
from args in FunctionArgs
from mod in AggregateModifier.OptionalOrDefault((without: false, labels: ImmutableArray<string>.Empty))
Expand All @@ -332,8 +332,8 @@ from args in FunctionArgs
from head in OneOf(
// TODO can we optimize order here?
Parse.Ref(() => ParenExpression).Cast<PromToken, ParenExpression, Expr>(),
Parse.Ref(() => AggregateExpr).Cast<PromToken, AggregateExpr, Expr>().Try(),
Parse.Ref(() => FunctionCall).Cast<PromToken, FunctionCall, Expr>().Try(),
Parse.Ref(() => AggregateExpr).Cast<PromToken, AggregateExpr, Expr>(),
Parse.Ref(() => FunctionCall).Cast<PromToken, FunctionCall, Expr>(),
Number.Cast<PromToken, NumberLiteral, Expr>().Try(),
Parse.Ref(() => UnaryExpr).Cast<PromToken, UnaryExpr, Expr>(),
MatrixSelector.Cast<PromToken, MatrixSelector, Expr>().Try(),
Expand Down
9 changes: 9 additions & 0 deletions tests/PromQL.Parser.Tests/ParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,15 @@ public void Invalid_Expr()
Assert.Throws<ParseException>(() => Parse(Parser.Expr, "!= 'blah'"));
}

[Test]
public void Invalid_DurationExpr()
{
// Prior to this test, a very unhelpful error message would be returned (parsing would complain about an invalid
// opening parenthesis)
Assert.Throws<ParseException>(() => Parse(Parser.Expr, "sum(my_metric[window])")).Message
.Should().Be("Syntax error (line 1, column 15): unexpected identifier `window`, expected duration.");
}

[Test]
public void ParseExpression_With_Comments()
{
Expand Down

0 comments on commit 4b786c0

Please sign in to comment.