fix - conditional evaluation (#50) - #63

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)

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',

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