Skip to content

Commit dd44579

Browse files
fix(rewrite): short-circuit chained comparisons (pytest-dev#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. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f41bb7f commit dd44579

3 files changed

Lines changed: 51 additions & 0 deletions

File tree

changelog/14822.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,7 +1181,22 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
11811181
expls: list[ast.expr] = []
11821182
syms: list[ast.expr] = []
11831183
results = [left_res]
1184+
# A chain short-circuits: ``a < b < c`` leaves c unevaluated when a < b
1185+
# is false. Everything past the first link therefore goes inside an
1186+
# ``if`` on the link before it, and the temporaries it would have
1187+
# produced are set to None up front -- the failure path builds a tuple
1188+
# of all of them, and the ones that never ran must still be readable.
1189+
# None is also falsey, so _call_reprcompare still stops at the link
1190+
# that actually failed.
1191+
body = self.statements
1192+
deferred_at = deferred_from = None
11841193
for i, op, next_operand in it:
1194+
if i:
1195+
if deferred_at is None:
1196+
deferred_at, deferred_from = len(body), len(self.variables)
1197+
inner: list[ast.stmt] = []
1198+
self.statements.append(ast.If(load_names[i - 1], inner, []))
1199+
self.statements = inner
11851200
next_res, next_expl = self.visit_operand(
11861201
next_operand, comp.comparators[i + 1 :]
11871202
)
@@ -1197,6 +1212,17 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
11971212
res_expr = ast.copy_location(ast.Compare(left_res, [op], [next_res]), comp)
11981213
self.statements.append(ast.Assign([store_names[i]], res_expr))
11991214
left_res, left_expl = next_res, next_expl
1215+
self.statements = body
1216+
if deferred_at is not None:
1217+
assert deferred_from is not None
1218+
deferred = [*res_variables[1:], *self.variables[deferred_from:]]
1219+
body.insert(
1220+
deferred_at,
1221+
ast.Assign(
1222+
[ast.Name(name, ast.Store()) for name in deferred],
1223+
ast.Constant(None),
1224+
),
1225+
)
12001226
# Use pytest.assertion.util._reprcompare if that's available.
12011227
expl_call = self.helper(
12021228
"_call_reprcompare",

testing/test_assertrewrite_coverage.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,6 +1253,30 @@ def check():
12531253
return "passed", value
12541254
""")
12551255

1256+
def test_chained_compare_stops_at_the_first_false(self) -> None:
1257+
assert_evaluation_order("""
1258+
def check():
1259+
trace = []
1260+
def rec(label, value):
1261+
trace.append(label)
1262+
return value
1263+
try:
1264+
assert rec("a", 1) < rec("b", 0) < rec("c", 5)
1265+
except AssertionError:
1266+
return "raised", trace
1267+
return "passed", trace
1268+
""")
1269+
1270+
def test_chained_compare_unreached_operand_does_not_raise(self) -> None:
1271+
assert_evaluation_order("""
1272+
def check():
1273+
try:
1274+
assert 1 < 0 < 1 / 0
1275+
except AssertionError:
1276+
return "raised", None
1277+
return "passed", None
1278+
""")
1279+
12561280
def test_method_lookup_precedes_arguments(self) -> None:
12571281
"""Guard: the bound method is looked up before the arguments run."""
12581282
assert_evaluation_order("""

0 commit comments

Comments
 (0)