fix(rewrite): guard a chained comparison's explanation too

Short-circuiting the chain nested the statements that evaluate a link
past the first, but not the statements that explain it, so the failure
branch ran a skipped link's explanation against temporaries the link
never assigned:

    assert 1 < 0 < (a or b)

raised ``AttributeError: 'NoneType' object has no attribute 'append'``
instead of failing.  visit_BoolOp builds its explanation by creating a
list in the main body and appending to it from expl_stmts; nesting only
the body left the list None while the appends ran unconditionally.

SirHegel found this against the branch and named the fix -- nest
expl_stmts on the same condition, the way visit_BoolOp nests both -- in
pytest-dev#14822 (comment),
having reduced it from his own pytest-dev#14918.  What follows is his idea; two
details are worth recording.

Most links explain themselves in the format context alone and contribute
no statements, and an ``if`` with an empty body is not valid syntax, so
the guards are attached innermost first and the empty ones dropped --
attaching a child fills its parent, so a parent is only known to be empty
once its child has been placed.

The names to pre-bind now include the @py_format ones created inside
those guarded blocks, which the outer format context reads.  Collecting
them by walking the blocks is what makes them reachable at all, but the
walk must not take everything it finds: a walrus target inside a skipped
link belongs to the user, and Python leaves it unbound.  Binding it to
None to keep the explanation readable would be visible after the
assertion, so _rewriter_temporaries() takes only names the rewriter
itself makes.

That last point is a second failure mode, which the report did not
cover: the explanation of a walrus reads its target to decide how to show
it, and a skipped link never bound it.

    assert 1 < 0 < (w := 1)          # UnboundLocalError: 'w'
    assert 1 < 0 < identity(w := 1)  # likewise

Nesting does not reach it -- the read sits in the compare's own format
dict, which is built eagerly and belongs to no link -- and ``'w' in
locals()`` does not guard it, because the fallback hands the value to
_should_repr_global_name().  visit_NamedExpr now asks whether the target
is a global before reading it, and shows the bare name when it is
neither.  An undefined name inside a skipped operand failed the same way
with NameError, and is fixed by the nesting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: RonnyPfannschmidt

src/_pytest/assertion/rewrite.py

@@ -545,6 +545,25 @@ def _can_rebind(nodes: Iterable[ast.expr]) -> bool:
     )
 
 
+def _rewriter_temporaries(nodes: Iterable[ast.AST]) -> list[str]:
+    """Return the names the rewriter binds inside *nodes*, in creation order.
+
+    Only its own: a walrus target inside a conditional block belongs to the
+    user, and Python leaves it unbound when the block does not run.  Binding it
+    to None to make it readable would be visible after the assertion.
+    """
+    names: dict[str, None] = {}
+    for node in nodes:
+        for sub in ast.walk(node):
+            if (
+                isinstance(sub, ast.Name)
+                and isinstance(sub.ctx, ast.Store)
+                and sub.id.startswith("@py")
+            ):
+                names[sub.id] = None
+    return list(names)
+
+
 @functools.lru_cache(maxsize=1)
 def _get_assertion_exprs(src: bytes) -> dict[int, str]:
     """Return a mapping from {lineno: "assertion test expression"}."""
@@ -951,7 +970,20 @@ def visit_NamedExpr(self, name: ast.NamedExpr) -> tuple[ast.NamedExpr, str]:
         target_id = name.target.id
         target_name = ast.Name(target_id, ast.Load())
         inlocs = ast.Compare(ast.Constant(target_id), [ast.In()], [locs])
-        dorepr = self.helper("_should_repr_global_name", target_name)
+        # Unlike visit_Name, the target need not be bound when this runs: the
+        # walrus may sit in a branch that short-circuited away, and then the
+        # explanation is formatted for an assignment that never happened.
+        # Reading it to decide how to show it would raise instead -- so ask
+        # whether it exists as a global before passing it to a helper.
+        inglobals = ast.Compare(
+            ast.Constant(target_id),
+            [ast.In()],
+            [ast.Call(self.builtin("globals"), [], [])],
+        )
+        dorepr = ast.BoolOp(
+            ast.And(),
+            [inglobals, self.helper("_should_repr_global_name", target_name)],
+        )
         test = ast.BoolOp(ast.Or(), [inlocs, dorepr])
         expr = ast.IfExp(test, self.display(target_name), ast.Constant(target_id))
         return name, self.explanation_param(expr)
@@ -1183,20 +1215,32 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
         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.
+        # ``if`` on the link before it -- both what evaluates the link and what
+        # explains it, because the explanation reads what the evaluation bound.
+        # The temporaries a skipped link would have produced are set to None up
+        # front, so the failure path can still read them: it builds a tuple of
+        # every link's result, and formats every link's explanation.  None is
+        # also falsey, so _call_reprcompare still stops at the link that
+        # actually failed, and the entries behind it are never rendered.
         body = self.statements
-        deferred_at = deferred_from = None
+        fail_save = self.expl_stmts
+        deferred_at = None
+        deferred_stmt_ifs: list[ast.If] = []
+        deferred_expl_ifs: list[tuple[list[ast.stmt], ast.If]] = []
         for i, op, next_operand in it:
             if i:
                 if deferred_at is None:
-                    deferred_at, deferred_from = len(body), len(self.variables)
+                    deferred_at = len(body)
                 inner: list[ast.stmt] = []
-                self.statements.append(ast.If(load_names[i - 1], inner, []))
+                stmt_if = ast.If(load_names[i - 1], inner, [])
+                deferred_stmt_ifs.append(stmt_if)
+                self.statements.append(stmt_if)
                 self.statements = inner
+                fail_inner: list[ast.stmt] = []
+                deferred_expl_ifs.append(
+                    (self.expl_stmts, ast.If(load_names[i - 1], fail_inner, []))
+                )
+                self.expl_stmts = fail_inner
             next_res, next_expl = self.visit_operand(
                 next_operand, comp.comparators[i + 1 :]
             )
@@ -1213,9 +1257,24 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
             self.statements.append(ast.Assign([store_names[i]], res_expr))
             left_res, left_expl = next_res, next_expl
         self.statements = body
+        self.expl_stmts = fail_save
+        # Attach the explanation guards innermost first, dropping the ones that
+        # stayed empty -- most links explain themselves in the format context
+        # alone and contribute no statements, and an ``if`` with an empty body
+        # is not valid syntax.  Attaching a child fills its parent, so the
+        # parent is only known to be empty once the child has been placed.
+        for parent, fail_if in reversed(deferred_expl_ifs):
+            if fail_if.body:
+                parent.append(fail_if)
         if deferred_at is not None:
-            assert deferred_from is not None
-            deferred = [*res_variables[1:], *self.variables[deferred_from:]]
+            deferred = dict.fromkeys(
+                [
+                    *res_variables[1:],
+                    *_rewriter_temporaries(
+                        [*deferred_stmt_ifs, *(f for _, f in deferred_expl_ifs)]
+                    ),
+                ]
+            )
             body.insert(
                 deferred_at,
                 ast.Assign(

testing/test_assertrewrite_coverage.py

@@ -1277,6 +1277,73 @@ def check():
                 return "passed", None
             """)
 
+    def test_chained_compare_skipped_boolop_operand(self) -> None:
+        """A boolop in a skipped link builds its explanation in that link."""
+        assert_evaluation_order("""
+            def check():
+                a = b = 0
+                try:
+                    assert 1 < 0 < (a or b)
+                except AssertionError:
+                    return "raised", None
+                return "passed", None
+            """)
+
+    def test_chained_compare_skipped_operand_reads_no_name(self) -> None:
+        """Nor may it read the names that operand would have read."""
+        assert_evaluation_order("""
+            def check():
+                try:
+                    assert 1 < 0 < (missing or 0)
+                except AssertionError:
+                    return "raised", None
+                return "passed", None
+            """)
+
+    def test_chained_compare_skipped_walrus_stays_unbound(self) -> None:
+        """A skipped walrus assigns nothing -- the explanation may not either."""
+        assert_evaluation_order("""
+            def check():
+                try:
+                    assert 1 < 0 < (w := 1)
+                except AssertionError:
+                    return "raised", "w" in locals()
+                return "passed", "w" in locals()
+            """)
+
+    def test_chained_compare_skipped_walrus_inside_call(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                try:
+                    assert 1 < 0 < identity(w := 1)
+                except AssertionError:
+                    return "raised", "w" in locals()
+                return "passed", "w" in locals()
+            """)
+
+    def test_chained_compare_reports_the_link_that_failed(self) -> None:
+        """The skipped link contributes nothing to the message either."""
+        assert_introspects(
+            """
+            def check():
+                a = b = 0
+                assert 1 < 0 < (a or b)
+            """,
+            must_contain=["assert 1 < 0"],
+            must_not_contain=["None"],
+        )
+
+    def test_chained_compare_reports_a_taken_walrus_link(self) -> None:
+        assert_introspects(
+            """
+            def check():
+                assert 0 < 1 < (w := 5) < 2
+            """,
+            must_contain=["5"],
+        )
+
     def test_method_lookup_precedes_arguments(self) -> None:
         """Guard: the bound method is looked up before the arguments run."""
         assert_evaluation_order("""