expressions: Short circuit logic operators

Our logic operators 'and' and 'or' would not short-circuit if the result
was already conclusive. This would lead to unnecessary evaluation and
broke user assumption that they could be used to run additional tests
only if required. We fix that by introducing explicit short-circuiting
and adding tests to document the problem and catch regressions.

We also fix over-eager short-circuiting for compound 'or' operations by
excluding both 'or' and 'and' from post-evaluation short-circuiting and
deferring their short-circuiting to the next iteration based on the
then-first operand.

Finally, we explicitly document the distinction between operand passing
within compound operations (such as 1 < 2 < 2) and their evaluation into
boolean results. We opt (as we did implictly before) to always return
booleans even for 'and' and 'or' which in python return the determing
operand (1 or 2 -> 1 instead of True).

In order to determine what operation we're looking at, we switch 'and'
and 'or' from lambda functions to static methods.

This now also allows tests to avoid error conditions like: foo is not
None and "bar" in foo (NoneType not iterable in this case).

Partially addresses scVENUS#176.

peekaboo/ruleset/expressions.py

@@ -443,8 +443,8 @@ def __init__(self, tokens):
             "isdisjoint": lambda a, b: a.isdisjoint(b),
             # beware of operator.and_ and operator.or_: these are bitwise not
             # logic
-            "and": lambda a, b: a and b,
-            "or": lambda a, b: a or b,
+            "and": EvalLogic.and_,
+            "or": EvalLogic.or_,
         }
 
     @staticmethod
@@ -459,6 +459,16 @@ def not_in(a, b):
         """ Naively implement non-membership test. """
         return a not in b
 
+    @staticmethod
+    def and_(op1, op2):
+        """ Naively implement logic and. """
+        return op1 and op2
+
+    @staticmethod
+    def or_(op1, op2):
+        """ Naively implement logic or. """
+        return op1 or op2
+
     @staticmethod
     def handle_regexes(function, val1, val2):
         """ Special handling of equality and membership checks for regular
@@ -486,14 +496,36 @@ def handle_regexes(function, val1, val2):
 
     def eval(self, context):
         val1 = self.value[0].eval(context)
+        result = False
         for op, parseobj in operator_operands(self.value[1:]):
+            function = self.operator_map[op]
+
+            # short-circuiting on first operand already: do not evaluate
+            # further operands if evaluation result is already conclusive
+            if function is EvalLogic.and_ and not val1:
+                return False
+            if function is EvalLogic.or_ and val1:
+                return True
+
             val2 = parseobj.eval(context)
             logger.debug("Comparison: %s %s %s", val1, op, val2)
-            function = self.operator_map[op]
-            if not self.handle_regexes(function, val1, val2):
-                break
+            result = self.handle_regexes(function, val1, val2)
+
+            # short-circuiting compound operations on two operands such as
+            # comparison: Do not evaluate further operations if this one has
+            # already turned false.
+            if function not in [EvalLogic.and_, EvalLogic.or_] and not result:
+                return False
+
+            # pass second operand as first operand to next part of compound
+            # operation
             val1 = val2
-        else:
+
+        # very explicit evaluation as boolean to make distinction clear: some
+        # python operations such as 'and' and 'or' return the operand which
+        # caused the operation to terminate, *not* it's boolean value. Others
+        # such as comparisons do. We always return booleans here.
+        if result:
             return True
 
         return False

tests/test.py

@@ -1915,6 +1915,27 @@ def test_basic_expressions(self):
             # these two would break if bitwise operators were used
             ["True and 2", True],
             ["True or 2", True],
+            ["False and True", False],
+            ["True or False", True],
+            ["True and False", False],
+            ["False or True", True],
+            ["1 < 2 < 3", True],
+            ["1 < 2 < 2", False],
+            ["1 == 2 == 2", False],
+            ["1 == 1 == 2", False],
+            ["1 != 2 != 2", False],
+            ["1 != 1 != 2", False],
+            ["'a' in 'aa' in 'aaa'", True],
+            ["True and True and False", False],
+            ["True and False and True", False],
+            ["False or False or True", True],
+            ["1 and 2 and 0", False],
+            ["1 and 0 and 2", False],
+            ["0 or 0 or 2", True],
+            # this would raise a NoneType exception if the second operand was
+            # evaluated
+            ["False and 'foo' in None", False],
+            ["True or 'foo' in None", True],
             ["5", 5],
             ["5 == 5", True],
             ["5 == 7", False],