Improve error message for missing operands (#221)

mpkorstanje · web-flow · commit eb1e717bda27 · 2025-11-09T17:37:36.000+01:00
Improved the error message for cases where an operand is missing.
Unfortunately the parsing algorithm can't detect which operator is
missing operands so the error message must remain fairly generic.
diff --git 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
@@ -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
@@ -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(&notExpr{expr: stack.Pop()})
+		expr, err := popOperand(infix, stack)
+		if err != nil {
+			return err
+		}
+		stack.Push(&notExpr{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
@@ -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> T pop(Deque<T> 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<Expression> stack) {
+    private void pushExpr(String token, Deque<Expression> 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> T popOperand(Deque<T> 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
@@ -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,15 +52,15 @@ 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(
           `Tag expression "${infix}" could not be parsed because of syntax error: Unmatched ).`
         )
       }
       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<T>(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<T>(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
@@ -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
@@ -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 "<end-of-input>"},
     );
diff --git a/php/src/TagExpressionParser.php b/php/src/TagExpressionParser.php
diff --git a/ruby/lib/cucumber/tag_expressions/parser.rb b/ruby/lib/cucumber/tag_expressions/parser.rb
diff --git a/testdata/errors.yml b/testdata/errors.yml
