fix(rewrite): short-circuit chained comparisons (#14819)

Python evaluates a comparison chain lazily -- in `a < b < c`, c is never
evaluated when a < b is false.  visit_Compare walked the comparators in
a loop and only combined the results with `and` afterwards, by which
time everything had already run:

    assert 1 < 0 < 1 / 0    # ZeroDivisionError, not AssertionError

Each link past the first now goes inside an `if` on the link before it,
the same shape visit_BoolOp has used since #57 was fixed for and/or.

The failure path builds a tuple of every link's result and every
operand, so the temporaries belonging to links that never ran are set to
None ahead of the chain rather than left unbound.  None is falsey, which
is also what _call_reprcompare wants: it stops at the first falsey
result, and that is the link that actually failed.

Closes the order-chained-compare-lazy group in the coverage matrix.

Author: RonnyPfannschmidt

changelog/14822.bugfix.rst

@@ -0,0 +1 @@
+Chained comparisons in an ``assert`` now short-circuit the way Python does: in ``assert a < b < c``, ``c`` is no longer evaluated when ``a < b`` is false. Previously ``assert 1 < 0 < 1 / 0`` raised ``ZeroDivisionError`` instead of ``AssertionError``, and a call in an unreached position ran anyway.

src/_pytest/assertion/rewrite.py

@@ -1191,7 +1191,22 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
         expls: list[ast.expr] = []
         syms: list[ast.expr] = []
         results = [left_res]
+        # A chain short-circuits: ``a < b < c`` leaves c unevaluated when a < b
+        # is false.  Everything past the first link therefore goes inside an
+        # ``if`` on the link before it, and the temporaries it would have
+        # produced are set to None up front -- the failure path builds a tuple
+        # of all of them, and the ones that never ran must still be readable.
+        # None is also falsey, so _call_reprcompare still stops at the link
+        # that actually failed.
+        body = self.statements
+        deferred_at = deferred_from = None
         for i, op, next_operand in it:
+            if i:
+                if deferred_at is None:
+                    deferred_at, deferred_from = len(body), len(self.variables)
+                inner: list[ast.stmt] = []
+                self.statements.append(ast.If(load_names[i - 1], inner, []))
+                self.statements = inner
             next_res, next_expl = self.visit_operand(
                 next_operand, comp.comparators[i + 1 :]
             )
@@ -1207,6 +1222,17 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
             res_expr = ast.copy_location(ast.Compare(left_res, [op], [next_res]), comp)
             self.statements.append(ast.Assign([store_names[i]], res_expr))
             left_res, left_expl = next_res, next_expl
+        self.statements = body
+        if deferred_at is not None:
+            assert deferred_from is not None
+            deferred = [*res_variables[1:], *self.variables[deferred_from:]]
+            body.insert(
+                deferred_at,
+                ast.Assign(
+                    [ast.Name(name, ast.Store()) for name in deferred],
+                    ast.Constant(None),
+                ),
+            )
         # Use pytest.assertion.util._reprcompare if that's available.
         expl_call = self.helper(
             "_call_reprcompare",

testing/test_assertrewrite_coverage.py

@@ -21,7 +21,6 @@
     order-call-argument           an earlier argument is read too late
     order-binop-left              a binary operator's left operand is read too late
     order-starred-argument        a starred argument is read too late
-    order-chained-compare-lazy    a chained comparison evaluates every comparator
 
 The xfails are strict on purpose: fixing the behaviour without flipping the
 marker fails the suite.  A change may remove entries from that list, never add
@@ -1297,10 +1296,6 @@ def check():
     return "passed", value
 """)
 
-    @pytest.mark.xfail(
-        strict=True,
-        reason="order-chained-compare-lazy: #14819, every comparator is evaluated",
-    )
     def test_chained_compare_stops_at_the_first_false(self) -> None:
         assert_evaluation_order("""
 def check():
@@ -1315,10 +1310,6 @@ def rec(label, value):
     return "passed", trace
 """)
 
-    @pytest.mark.xfail(
-        strict=True,
-        reason="order-chained-compare-lazy: #14819, an unreached comparator still raises",
-    )
     def test_chained_compare_unreached_operand_does_not_raise(self) -> None:
         assert_evaluation_order("""
 def check():