diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml index 8be7c693..1a771c29 100644 --- a/.github/workflows/test-go.yml +++ b/.github/workflows/test-go.yml @@ -23,7 +23,7 @@ jobs: test-go: runs-on: ${{ matrix.os }} strategy: - fail-fast: false + fail-fast: true matrix: os: - ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index e6c19047..3f828e8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed +- Improve error message for missing operands ([#221](https://github.com/cucumber/tag-expressions/pull/221)) ## [8.0.0] - 2025-10-14 ### Fixed diff --git a/go/parser.go b/go/parser.go index 4ff46382..fe76fab4 100644 --- a/go/parser.go +++ b/go/parser.go @@ -42,7 +42,9 @@ func Parse(infix string) (Evaluatable, error) { isOp(operators.Peek()) && ((ASSOC[token] == "left" && PREC[token] <= PREC[operators.Peek()]) || (ASSOC[token] == "right" && PREC[token] < PREC[operators.Peek()])) { - pushExpr(operators.Pop(), expressions) + if err := pushExpr(infix, operators.Pop(), expressions); err != nil { + return nil, err + } } operators.Push(token) expectedTokenType = OPERAND @@ -57,7 +59,9 @@ func Parse(infix string) (Evaluatable, error) { return nil, err } for operators.Len() > 0 && operators.Peek() != "(" { - pushExpr(operators.Pop(), expressions) + if err := pushExpr(infix, operators.Pop(), expressions); err != nil { + return nil, err + } } if operators.Len() == 0 { return nil, fmt.Errorf("Tag expression \"%s\" could not be parsed because of syntax error: Unmatched ).", infix) @@ -70,7 +74,9 @@ func Parse(infix string) (Evaluatable, error) { if err := check(infix, expectedTokenType, OPERAND); err != nil { return nil, err } - pushExpr(token, expressions) + if err := pushExpr(infix, token, expressions); err != nil { + return nil, err + } expectedTokenType = OPERATOR } } @@ -79,7 +85,9 @@ func Parse(infix string) (Evaluatable, error) { if operators.Peek() == "(" { return nil, fmt.Errorf("Tag expression \"%s\" could not be parsed because of syntax error: Unmatched (.", infix) } - pushExpr(operators.Pop(), expressions) + if err := pushExpr(infix, operators.Pop(), expressions); err != nil { + return nil, err + } } return expressions.Pop(), nil @@ -153,24 +161,51 @@ func check(infix, expectedTokenType, tokenType string) error { return nil } -func pushExpr(token string, stack *EvaluatableStack) { +func pushExpr(infix string, token string, stack *EvaluatableStack) error { if token == "and" { - rightAndExpr := stack.Pop() + rightAndExpr, err := popOperand(infix, stack) + if err != nil { + return err + } + leftAndExpr, err := popOperand(infix, stack) + if err != nil { + return err + } stack.Push(&andExpr{ - leftExpr: stack.Pop(), + leftExpr: leftAndExpr, rightExpr: rightAndExpr, }) } else if token == "or" { - rightOrExpr := stack.Pop() + rightOrExpr, err := popOperand(infix, stack) + if err != nil { + return err + } + leftOrExpr, err := popOperand(infix, stack) + if err != nil { + return err + } stack.Push(&orExpr{ - leftExpr: stack.Pop(), + leftExpr: leftOrExpr, rightExpr: rightOrExpr, }) } else if token == "not" { - stack.Push(¬Expr{expr: stack.Pop()}) + expr, err := popOperand(infix, stack) + if err != nil { + return err + } + stack.Push(¬Expr{expr: expr}) } else { stack.Push(&literalExpr{value: token}) } + + return nil +} + +func popOperand(infix string, stack *EvaluatableStack) (Evaluatable, error) { + if stack.Len() > 0 { + return stack.Pop(), nil + } + return nil, fmt.Errorf("Tag expression \"%s\" could not be parsed because of syntax error: Expected operand.", infix) } type literalExpr struct { diff --git a/java/src/main/java/io/cucumber/tagexpressions/TagExpressionParser.java b/java/src/main/java/io/cucumber/tagexpressions/TagExpressionParser.java index e71d74f3..0000f341 100644 --- a/java/src/main/java/io/cucumber/tagexpressions/TagExpressionParser.java +++ b/java/src/main/java/io/cucumber/tagexpressions/TagExpressionParser.java @@ -52,7 +52,7 @@ private Expression parse() { || (ASSOC.get(token) == Assoc.RIGHT && PREC.get(token) < PREC.get(operators.peek()))) ) { - pushExpr(pop(operators), expressions); + pushExpr(operators.pop(), expressions); } operators.push(token); expectedTokenType = TokenType.OPERAND; @@ -63,13 +63,13 @@ private Expression parse() { } else if (")".equals(token)) { check(expectedTokenType, TokenType.OPERATOR); while (operators.size() > 0 && !"(".equals(operators.peek())) { - pushExpr(pop(operators), expressions); + pushExpr(operators.pop(), expressions); } if (operators.size() == 0) { throw new TagExpressionException("Tag expression \"%s\" could not be parsed because of syntax error: Unmatched ).", this.infix); } if ("(".equals(operators.peek())) { - pop(operators); + operators.pop(); } expectedTokenType = TokenType.OPERATOR; } else { @@ -83,7 +83,7 @@ private Expression parse() { if ("(".equals(operators.peek())) { throw new TagExpressionException("Tag expression \"%s\" could not be parsed because of syntax error: Unmatched (.", infix); } - pushExpr(pop(operators), expressions); + pushExpr(operators.pop(), expressions); } return expressions.pop(); @@ -128,31 +128,34 @@ private void check(TokenType expectedTokenType, TokenType tokenType) { } } - private T pop(Deque stack) { - if (stack.isEmpty()) - throw new TagExpressionException("Tag expression \"%s\" could not be parsed because of an empty stack", infix); - return stack.pop(); - } - - private void pushExpr(String token, Deque stack) { + private void pushExpr(String token, Deque expressions) { switch (token) { case "and": - Expression rightAndExpr = pop(stack); - stack.push(new And(pop(stack), rightAndExpr)); + Expression rightAndExpr = popOperand(expressions); + Expression leftAndExpr = popOperand(expressions); + expressions.push(new And(leftAndExpr, rightAndExpr)); break; case "or": - Expression rightOrExpr = pop(stack); - stack.push(new Or(pop(stack), rightOrExpr)); + Expression rightOrExpr = popOperand(expressions); + Expression leftOrExpr = popOperand(expressions); + expressions.push(new Or(leftOrExpr, rightOrExpr)); break; case "not": - stack.push(new Not(pop(stack))); + Expression expression = popOperand(expressions); + expressions.push(new Not(expression)); break; default: - stack.push(new Literal(token)); + expressions.push(new Literal(token)); break; } } + private T popOperand(Deque stack) { + if (stack.isEmpty()) + throw new TagExpressionException("Tag expression \"%s\" could not be parsed because of syntax error: Expected operand.", infix); + return stack.pop(); + } + private boolean isUnary(String token) { return "not".equals(token); } diff --git a/javascript/src/index.ts b/javascript/src/index.ts index 2d09f602..d6978d68 100644 --- a/javascript/src/index.ts +++ b/javascript/src/index.ts @@ -41,7 +41,7 @@ export default function parse(infix: string): Node { ((ASSOC[token] === 'left' && PREC[token] <= PREC[peek(operators)]) || (ASSOC[token] === 'right' && PREC[token] < PREC[peek(operators)])) ) { - pushExpr(pop(operators), expressions) + pushExpr(operators.pop() as string, expressions) } operators.push(token) expectedTokenType = OPERAND @@ -52,7 +52,7 @@ export default function parse(infix: string): Node { } else if (')' === token) { check(expectedTokenType, OPERATOR) while (operators.length > 0 && peek(operators) !== '(') { - pushExpr(pop(operators), expressions) + pushExpr(operators.pop() as string, expressions) } if (operators.length === 0) { throw new Error( @@ -60,7 +60,7 @@ export default function parse(infix: string): Node { ) } if (peek(operators) === '(') { - pop(operators) + operators.pop() } expectedTokenType = OPERATOR } else { @@ -76,10 +76,10 @@ export default function parse(infix: string): Node { `Tag expression "${infix}" could not be parsed because of syntax error: Unmatched (.` ) } - pushExpr(pop(operators), expressions) + pushExpr(operators.pop() as string, expressions) } - return pop(expressions) + return expressions.pop() as Node function check(expectedTokenType: string, tokenType: string) { if (expectedTokenType !== tokenType) { @@ -88,6 +88,29 @@ export default function parse(infix: string): Node { ) } } + + function pushExpr(token: string, stack: Node[]) { + if (token === 'and') { + const rightAndExpr = popOperand(stack) + stack.push(new And(popOperand(stack), rightAndExpr)) + } else if (token === 'or') { + const rightOrExpr = popOperand(stack) + stack.push(new Or(popOperand(stack), rightOrExpr)) + } else if (token === 'not') { + stack.push(new Not(popOperand(stack))) + } else { + stack.push(new Literal(token)) + } + } + + function popOperand(stack: T[]): T { + if (stack.length === 0) { + throw new Error( + `Tag expression "${infix}" could not be parsed because of syntax error: Expected operand.` + ) + } + return stack.pop() as T + } } function tokenize(expr: string): string[] { @@ -141,27 +164,6 @@ function peek(stack: string[]) { return stack[stack.length - 1] } -function pop(stack: T[]): T { - if (stack.length === 0) { - throw new Error('empty stack') - } - return stack.pop() as T -} - -function pushExpr(token: string, stack: Node[]) { - if (token === 'and') { - const rightAndExpr = pop(stack) - stack.push(new And(pop(stack), rightAndExpr)) - } else if (token === 'or') { - const rightOrExpr = pop(stack) - stack.push(new Or(pop(stack), rightOrExpr)) - } else if (token === 'not') { - stack.push(new Not(pop(stack))) - } else { - stack.push(new Literal(token)) - } -} - interface Node { evaluate(variables: string[]): boolean } diff --git a/perl/lib/Cucumber/TagExpressions.pm b/perl/lib/Cucumber/TagExpressions.pm index 936f1cd6..db48bfe7 100644 --- a/perl/lib/Cucumber/TagExpressions.pm +++ b/perl/lib/Cucumber/TagExpressions.pm @@ -104,7 +104,7 @@ sub _term_expr { my $token = _get_token( $state ); - die 'Unexpected end of input parsing tag expression' + die qq{Tag expression "$state->{text}" could not be parsed because of syntax error: Expected operand.} if not defined $token; if ( $token eq '(' ) { diff --git a/perl/t/02-evaluate.t b/perl/t/02-evaluate.t index 794ddc60..ce8fac4d 100644 --- a/perl/t/02-evaluate.t +++ b/perl/t/02-evaluate.t @@ -103,9 +103,9 @@ for my $ex (@good) { my %bad_syntax = ( '@a @b' => q{Expected operator.}, '@a not' => q{Expected operator.}, - '@a or' => 'Unexpected end of input parsing tag expression', + '@a or' => q{Expected operand.}, '@a not @b' => q{Expected operator.}, - '@a or (' => 'Unexpected end of input parsing tag expression', + '@a or (' => q{Expected operand.}, '@a and @b)' => q{Unmatched ).}, "\@a\\" => q{Illegal escape before ""}, ); diff --git a/php/src/TagExpressionParser.php b/php/src/TagExpressionParser.php index 095d0d55..f7fc809d 100644 --- a/php/src/TagExpressionParser.php +++ b/php/src/TagExpressionParser.php @@ -51,7 +51,7 @@ private function parseInfix(): Expression (Associativity::forOperator($token) === Associativity::LEFT && self::PREC[$token] <= self::PREC[self::peek($operators)]) || (Associativity::forOperator($token) === Associativity::RIGHT && self::PREC[$token] < self::PREC[self::peek($operators)]) )) { - $this->pushExpr($this->pop($operators), $expressions); + $this->pushExpr(array_pop($operators), $expressions); } // TODO check associativity $operators[] = $token; @@ -63,7 +63,7 @@ private function parseInfix(): Expression } elseif ($token === ')') { $this->check($expectedTokenType, TokenType::Operator); while (\count($operators) > 0 && self::peek($operators) !== '(') { - $this->pushExpr($this->pop($operators), $expressions); + $this->pushExpr(array_pop($operators), $expressions); } if (\count($operators) === 0) { @@ -71,7 +71,7 @@ private function parseInfix(): Expression } if (self::peek($operators) === '(') { - $this->pop($operators); + array_pop($operators); } $expectedTokenType = TokenType::Operator; @@ -87,10 +87,10 @@ private function parseInfix(): Expression throw new TagExpressionException(\sprintf('Tag expression "%s" could not be parsed because of syntax error: Unmatched (.', $this->infix)); } - $this->pushExpr($this->pop($operators), $expressions); + $this->pushExpr(array_pop($operators), $expressions); } - return $this->pop($expressions); + return $expressions[0]; } /** @@ -156,12 +156,12 @@ private static function peek(array $stack): string * * @return T */ - private function pop(array &$stack): mixed + private function popOperand(array &$stack): mixed { $value = array_pop($stack); if ($value === null) { - throw new TagExpressionException(\sprintf('Tag expression "%s" could not be parsed because of an empty stack.', $this->infix)); + throw new TagExpressionException(\sprintf('Tag expression "%s" could not be parsed because of syntax error: Expected operand.', $this->infix)); } return $value; @@ -174,17 +174,19 @@ private function pushExpr(string $token, array &$stack): void { switch ($token) { case 'and': - $rightAndExpr = $this->pop($stack); - $stack[] = new AndExpression($this->pop($stack), $rightAndExpr); + $rightAndExpr = $this->popOperand($stack); + $lefAndExpr = $this->popOperand($stack); + $stack[] = new AndExpression($lefAndExpr, $rightAndExpr); break; case 'or': - $rightOrExpr = $this->pop($stack); - $stack[] = new OrExpression($this->pop($stack), $rightOrExpr); + $rightOrExpr = $this->popOperand($stack); + $leftOrExpr = $this->popOperand($stack); + $stack[] = new OrExpression($leftOrExpr, $rightOrExpr); break; case 'not': - $stack[] = new NotExpression($this->pop($stack)); + $stack[] = new NotExpression($this->popOperand($stack)); break; default: diff --git a/ruby/lib/cucumber/tag_expressions/parser.rb b/ruby/lib/cucumber/tag_expressions/parser.rb index f579bef9..900feb89 100644 --- a/ruby/lib/cucumber/tag_expressions/parser.rb +++ b/ruby/lib/cucumber/tag_expressions/parser.rb @@ -18,10 +18,9 @@ def parse(infix_expression) tokens.each { |token| expected_token_type = handle_sequential_tokens(token, infix_expression, expected_token_type) } while @operators.any? raise "Tag expression \"#{infix_expression}\" could not be parsed because of syntax error: Unmatched (." if @operators.last == '(' - - push_expression(pop(@operators)) + push_expression(infix_expression, @operators.pop) end - pop(@expressions) + @expressions.pop end private @@ -67,11 +66,11 @@ def tokenize(infix_expression) tokens end - def push_expression(token) + def push_expression(infix_expression, token) case token - when 'and' then @expressions.push(And.new(*pop(@expressions, 2))) - when 'or' then @expressions.push(Or.new(*pop(@expressions, 2))) - when 'not' then @expressions.push(Not.new(pop(@expressions))) + when 'and' then @expressions.push(And.new(*popOperand(infix_expression, @expressions, 2))) + when 'or' then @expressions.push(Or.new(*popOperand(infix_expression, @expressions, 2))) + when 'not' then @expressions.push(Not.new(popOperand(infix_expression, @expressions))) else @expressions.push(Literal.new(token)) end end @@ -92,7 +91,7 @@ def handle_unary_operator(infix_expression, token, expected_token_type) def handle_binary_operator(infix_expression, token, expected_token_type) check(infix_expression, expected_token_type, :operator) - push_expression(pop(@operators)) while @operators.any? && operator?(@operators.last) && lower_precedence?(token) + push_expression(infix_expression, @operators.pop) while @operators.any? && operator?(@operators.last) && lower_precedence?(token) @operators.push(token) :operand end @@ -105,16 +104,16 @@ def handle_open_paren(infix_expression, token, expected_token_type) def handle_close_paren(infix_expression, _token, expected_token_type) check(infix_expression, expected_token_type, :operator) - push_expression(pop(@operators)) while @operators.any? && @operators.last != '(' + push_expression(infix_expression, @operators.pop) while @operators.any? && @operators.last != '(' raise "Tag expression \"#{infix_expression}\" could not be parsed because of syntax error: Unmatched )." if @operators.empty? - pop(@operators) if @operators.last == '(' + @operators.pop if @operators.last == '(' :operator end def handle_literal(infix_expression, token, expected_token_type) check(infix_expression, expected_token_type, :operand) - push_expression(token) + push_expression(infix_expression, token) :operator end @@ -124,9 +123,9 @@ def check(infix_expression, expected_token_type, token_type) raise "Tag expression \"#{infix_expression}\" could not be parsed because of syntax error: Expected #{expected_token_type}." end - def pop(array, amount = 1) + def popOperand(infix_expression, array, amount = 1) result = array.pop(amount) - raise('Empty stack') if result.length != amount + raise "Tag expression \"#{infix_expression}\" could not be parsed because of syntax error: Expected operand." if result.length != amount amount == 1 ? result.first : result end diff --git a/testdata/errors.yml b/testdata/errors.yml index 389e782d..bc1327c8 100644 --- a/testdata/errors.yml +++ b/testdata/errors.yml @@ -20,3 +20,11 @@ error: 'Tag expression "x or \y or z" could not be parsed because of syntax error: Illegal escape before "y".' - expression: 'x\ or y' error: 'Tag expression "x\ or y" could not be parsed because of syntax error: Expected operator.' +- expression: 'a and' + error: 'Tag expression "a and" could not be parsed because of syntax error: Expected operand.' +- expression: 'a or' + error: 'Tag expression "a or" could not be parsed because of syntax error: Expected operand.' +- expression: 'not' + error: 'Tag expression "not" could not be parsed because of syntax error: Expected operand.' +- expression: 'a and not' + error: 'Tag expression "a and not" could not be parsed because of syntax error: Expected operand.'