diff --git a/src/celpy/evaluation.py b/src/celpy/evaluation.py index 47caf9b..5e97e3b 100644 --- a/src/celpy/evaluation.py +++ b/src/celpy/evaluation.py @@ -1314,6 +1314,13 @@ def expr(self, tree: lark.Tree) -> Result: The default implementation short-circuits and can ignore an CELEvalError in a sub-expression. + + See https://github.com/google/cel-spec/blob/master/doc/langdef.md#logical-operators + + > To get traditional left-to-right short-circuiting evaluation of logical operators, + as in C or other languages (also called "McCarthy Evaluation"), + the expression e1 && e2 can be rewritten `e1 ? e2 : false`. + Similarly, `e1 || e2` can be rewritten `e1 ? true : e2`. """ if len(tree.children) == 1: # expr is a single conditionalor. @@ -1322,8 +1329,13 @@ def expr(self, tree: lark.Tree) -> Result: elif len(tree.children) == 3: # full conditionalor "?" conditionalor ":" expr. func = self.functions["_?_:_"] - cond_value, left, right = cast(Tuple[Result, Result, Result], self.visit_children(tree)) + cond_value = self.visit(cast(lark.Tree, tree.children[0])) + left = right = cast(Result, celpy.celtypes.BoolType(False)) try: + if cond_value: + left = self.visit(cast(lark.Tree, tree.children[1])) + else: + right = self.visit(cast(lark.Tree, tree.children[2])) return func(cond_value, left, right) except TypeError as ex: self.logger.debug("%s(%s, %s) --> %s", func.__name__, left, right, ex) diff --git a/tests/test_evaluation.py b/tests/test_evaluation.py index 0d361fc..76e99f1 100644 --- a/tests/test_evaluation.py +++ b/tests/test_evaluation.py @@ -485,7 +485,7 @@ def test_eval_expr_1(): assert evaluator.evaluate() == celtypes.IntType(42) @fixture -def mock_expr_tree(): +def mock_left_expr_tree(): tree = lark.Tree( data='expr', children=[ @@ -501,39 +501,70 @@ def mock_expr_tree(): lark.Token(type_="INT_LIT", value="6"), ] ), - lark.Tree( - data='literal', - children=[ - lark.Token(type_="INT_LIT", value="7"), - ] - ), + sentinel.DO_NOT_EVALUATE # Test will crash if this is evaluated ], meta=Mock(line=1, column=1) ) return tree -def test_eval_expr_3_good(mock_expr_tree): +def test_eval_expr_3_left_good(mock_left_expr_tree): + """Assert ``true ? 6 : invalid`` does not execute the invalid expression.""" activation = Mock() evaluator = Evaluator( - mock_expr_tree, + mock_left_expr_tree, activation ) assert evaluator.evaluate() == celtypes.IntType(6) + # assert did not crash; therefore, invalid node not touched -def test_eval_expr_3_bad_override(mock_expr_tree): +def test_eval_expr_3_bad_override(mock_left_expr_tree): def bad_condition(a, b, c): raise TypeError activation = Mock() evaluator = Evaluator( - mock_expr_tree, + mock_left_expr_tree, activation, functions={"_?_:_": bad_condition} ) with raises(celpy.evaluation.CELEvalError): evaluator.evaluate() +@fixture +def mock_right_expr_tree(): + tree = lark.Tree( + data='expr', + children=[ + lark.Tree( + data='literal', + children=[ + lark.Token(type_="BOOL_LIT", value="false"), + ] + ), + sentinel.DO_NOT_EVALUATE, # Test will crash if this is evaluated + lark.Tree( + data='literal', + children=[ + lark.Token(type_="INT_LIT", value="7"), + ] + ), + ], + meta=Mock(line=1, column=1) + ) + return tree + +def test_eval_expr_3_right_good(mock_right_expr_tree): + """Assert ``false ? invalid : 7`` does not execute the invalid expression.""" + activation = Mock() + evaluator = Evaluator( + mock_right_expr_tree, + activation + ) + assert evaluator.evaluate() == celtypes.IntType(7) + # assert did not crash; therefore, invalid node not touched + + def test_eval_expr_0(): tree = lark.Tree( data='expr', diff --git a/type_check/lineprecision.txt b/type_check/lineprecision.txt index b80b663..de264b1 100644 --- a/type_check/lineprecision.txt +++ b/type_check/lineprecision.txt @@ -6,6 +6,6 @@ celpy.adapter 137 35 3 9 85 5 celpy.c7nlib 1582 340 15 154 1073 0 celpy.celparser 402 208 2 23 169 0 celpy.celtypes 1495 430 17 221 788 39 -celpy.evaluation 2434 824 33 172 1390 15 +celpy.evaluation 2446 827 33 173 1398 15 xlate 0 0 0 0 0 0 xlate.c7n_to_cel 1730 384 105 145 1091 5