fix(rewrite): freeze operands a later walrus would clobber

The rewriter hoists each operand into its own statement, but a plain
name is left as a bare load evaluated when the enclosing expression is
assembled -- after the statements of the operands that follow it.  A
walrus operator in a later operand rebinds the name in between, so both
the value used and the value reported were the post-walrus one, while
Python evaluates the earlier operand first:

    assert value != identity(value := value.lower())

visit_BoolOp already guarded against this; extract its pre-scan as
_walrus_targets() and add visit_operand() to apply the same freeze in
visit_Compare, visit_Call and visit_BinOp.  visit_Compare previously
matched only a comparator that *was* a NamedExpr, missing walrus
operators nested inside it; visit_Call did not guard at all, so an
earlier argument saw a later argument's assignment.

These cases predate the walrus rework -- they fail on main too.

Closes the single-eval-walrus, order-compare-left, order-call-argument
and order-binop-left groups in the coverage matrix.  order-call-argument
keeps one entry: a bare walrus argument is still substituted into a
later one, which visit_operand does not yet see because the operand is a
NamedExpr rather than a Name.

Reported-by: Denis Scapin

Author: RonnyPfannschmidt

changelog/14445.bugfix.rst

@@ -1 +1,3 @@
 Fixed assertion rewriting evaluating walrus operator (``:=``) expressions multiple times, causing incorrect test results when the expression had side effects (e.g., incrementing a counter or calling a function).
+
+Operands preceding a walrus operator are now evaluated -- and reported -- before it rebinds their name, so ``assert value != identity(value := value.lower())`` keeps Python's left-to-right evaluation order.

src/_pytest/assertion/rewrite.py

@@ -530,6 +530,16 @@ def traverse_node(node: ast.AST) -> Iterator[ast.AST]:
         yield from traverse_node(child)
 
 
+def _walrus_targets(nodes: Iterable[ast.expr]) -> set[str]:
+    """Return the names any walrus operator in *nodes* rebinds."""
+    return {
+        sub.target.id
+        for node in nodes
+        for sub in ast.walk(node)
+        if isinstance(sub, ast.NamedExpr)
+    }
+
+
 @functools.lru_cache(maxsize=1)
 def _get_assertion_exprs(src: bytes) -> dict[int, str]:
     """Return a mapping from {lineno: "assertion test expression"}."""
@@ -951,6 +961,27 @@ def visit_Name(self, name: ast.Name) -> tuple[ast.Name, str]:
         expr = ast.IfExp(test, self.display(name), ast.Constant(name.id))
         return name, self.explanation_param(expr)
 
+    def visit_operand(
+        self, operand: ast.expr, later: Sequence[ast.expr]
+    ) -> tuple[ast.expr, str]:
+        """Visit an operand, freezing it against walrus operators in *later*.
+
+        Operands are rewritten into statements that run in source order, but
+        a plain name is left as a bare load evaluated at the very end, when
+        the enclosing expression is assembled.  A walrus operator in a later
+        operand rebinds that name in between, so both the value used and the
+        value reported would be the post-walrus one -- Python evaluates the
+        earlier operand first.  Copy the value into a temporary instead.
+        """
+        specifiers = set(self.explanation_specifiers)
+        res, expl = self.visit(operand)
+        if isinstance(res, ast.Name) and res.id in _walrus_targets(later):
+            snapshot = self.assign(res)
+            for key in set(self.explanation_specifiers) - specifiers:
+                self.explanation_specifiers[key] = self.display(snapshot)
+            res = snapshot
+        return res, expl
+
     def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]:
         res_var = self.variable()
         expl_list = self.assign(ast.List([], ast.Load()))
@@ -959,18 +990,10 @@ def visit_BoolOp(self, boolop: ast.BoolOp) -> tuple[ast.Name, str]:
         body = save = self.statements
         fail_save = self.expl_stmts
         levels = len(boolop.values) - 1
-        # Pre-scan: for each operand position, collect the set of variable
-        # names that a *later* operand's walrus operator will overwrite.
-        # An operand needs a snapshot only when its value references a name
-        # in this set (otherwise the explanation would show the post-walrus
-        # value instead of the value at evaluation time).
-        later_walrus_targets: list[set[str]] = [set() for _ in boolop.values]
-        seen: set[str] = set()
-        for idx in range(len(boolop.values) - 1, -1, -1):
-            later_walrus_targets[idx] = set(seen)
-            for node in ast.walk(boolop.values[idx]):
-                if isinstance(node, ast.NamedExpr):
-                    seen.add(node.target.id)
+        later_walrus_targets = [
+            _walrus_targets(boolop.values[idx + 1 :])
+            for idx in range(len(boolop.values))
+        ]
         self.push_format_context()
         # Process each operand, short-circuiting as needed.
         for i, v in enumerate(boolop.values):
@@ -1024,7 +1047,7 @@ def visit_UnaryOp(self, unary: ast.UnaryOp) -> tuple[ast.Name, str]:
 
     def visit_BinOp(self, binop: ast.BinOp) -> tuple[ast.Name, str]:
         symbol = BINOP_MAP[binop.op.__class__]
-        left_expr, left_expl = self.visit(binop.left)
+        left_expr, left_expl = self.visit_operand(binop.left, [binop.right])
         right_expr, right_expl = self.visit(binop.right)
         explanation = f"({left_expl} {symbol} {right_expl})"
         res = self.assign(
@@ -1033,16 +1056,19 @@ def visit_BinOp(self, binop: ast.BinOp) -> tuple[ast.Name, str]:
         return res, explanation
 
     def visit_Call(self, call: ast.Call) -> tuple[ast.Name, str]:
-        new_func, func_expl = self.visit(call.func)
+        # The callee and every argument are evaluated left to right, so each of
+        # them has to be frozen against walrus operators in what follows.
+        operands = [*call.args, *(keyword.value for keyword in call.keywords)]
+        new_func, func_expl = self.visit_operand(call.func, operands)
         arg_expls = []
         new_args = []
         new_kwargs = []
-        for arg in call.args:
-            res, expl = self.visit(arg)
+        for i, arg in enumerate(call.args):
+            res, expl = self.visit_operand(arg, operands[i + 1 :])
             arg_expls.append(expl)
             new_args.append(res)
-        for keyword in call.keywords:
-            res, expl = self.visit(keyword.value)
+        for i, keyword in enumerate(call.keywords, start=len(call.args)):
+            res, expl = self.visit_operand(keyword.value, operands[i + 1 :])
             new_kwargs.append(ast.keyword(keyword.arg, res))
             if keyword.arg:
                 arg_expls.append(keyword.arg + "=" + expl)
@@ -1076,7 +1102,7 @@ def visit_Attribute(self, attr: ast.Attribute) -> tuple[ast.Name, str]:
 
     def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
         self.push_format_context()
-        left_res, left_expl = self.visit(comp.left)
+        left_res, left_expl = self.visit_operand(comp.left, comp.comparators)
         if isinstance(comp.left, ast.Compare | ast.BoolOp):
             left_expl = f"({left_expl})"
         if isinstance(left_res, ast.NamedExpr):
@@ -1089,18 +1115,9 @@ def visit_Compare(self, comp: ast.Compare) -> tuple[ast.expr, str]:
         syms: list[ast.expr] = []
         results = [left_res]
         for i, op, next_operand in it:
-            # If the next operand is a walrus that assigns to the same name as
-            # the current left_res, we must freeze left_res's value before the
-            # walrus modifies it.
-            match (next_operand, left_res):
-                case (
-                    ast.NamedExpr(target=ast.Name(id=target_id)),
-                    ast.Name(id=name_id),
-                ) if target_id == name_id:
-                    left_res = self.assign(left_res)
-                    results[-1] = left_res
-
-            next_res, next_expl = self.visit(next_operand)
+            next_res, next_expl = self.visit_operand(
+                next_operand, comp.comparators[i + 1 :]
+            )
             if isinstance(next_operand, ast.Compare | ast.BoolOp):
                 next_expl = f"({next_expl})"
             if isinstance(next_res, ast.NamedExpr):

testing/test_assertrewrite_coverage.py

@@ -763,6 +763,33 @@ def __getitem__(self, key):
                 assert d["a"] == 100
             """)
 
+    def test_walrus_in_compare_evaluated_once(self) -> None:
+        assert_single_evaluation("""
+            def check():
+                def side_effect():
+                    counter[0] += 1
+                    return 42
+                assert (x := side_effect()) == 100
+            """)
+
+    def test_walrus_in_boolean_evaluated_once(self) -> None:
+        assert_single_evaluation("""
+            def check():
+                def side_effect():
+                    counter[0] += 1
+                    return 42
+                assert (x := side_effect()) and False
+            """)
+
+    def test_walrus_in_chained_compare_evaluated_once(self) -> None:
+        assert_single_evaluation("""
+            def check():
+                def side_effect():
+                    counter[0] += 1
+                    return 5
+                assert 1 < (x := side_effect()) < 3
+            """)
+
     def test_method_call_evaluated_once(self) -> None:
         assert_single_evaluation("""
             def check():
@@ -836,6 +863,72 @@ class TestEvaluationOrder:
     given it.
     """
 
+    def test_compare_left_operand_precedes_walrus(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                value = "Hello"
+                try:
+                    assert value != identity(value := value.lower())
+                except AssertionError:
+                    return "raised", value
+                return "passed", value
+            """)
+
+    def test_compare_reports_left_operand(self) -> None:
+        assert_introspects(
+            """
+            def check():
+                def identity(v):
+                    return v
+                value = 2
+                assert value == identity(value := 3)
+            """,
+            must_contain=["assert 2 == 3"],
+        )
+
+    def test_call_earlier_argument_precedes_walrus(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                def collect(*values):
+                    return values
+                value = "Hello"
+                try:
+                    assert collect(value, identity(value := value.lower())) == ("Hello", "hello")
+                except AssertionError:
+                    return "raised", value
+                return "passed", value
+            """)
+
+    def test_binop_left_operand_precedes_walrus(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                value = 1
+                try:
+                    assert value + identity(value := 5) == 6
+                except AssertionError:
+                    return "raised", value
+                return "passed", value
+            """)
+
+    def test_chained_compare_operands_in_order(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                value = 1
+                try:
+                    assert value < identity(value := 5) < 9
+                except AssertionError:
+                    return "raised", value
+                return "passed", value
+            """)
+
     def test_container_literal_operand_in_order(self) -> None:
         """Guard: ``generic_visit`` hoists container literals into a temporary."""
         assert_evaluation_order("""
@@ -914,6 +1007,36 @@ def take(self, value):
                 return "passed", obj
             """)
 
+    def test_keyword_argument_precedes_walrus(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                def collect(**kwargs):
+                    return kwargs
+                value = 1
+                try:
+                    assert collect(a=value, b=identity(value := 2)) == {"a": 1, "b": 2}
+                except AssertionError:
+                    return "raised", value
+                return "passed", value
+            """)
+
+    def test_double_star_argument_precedes_walrus(self) -> None:
+        assert_evaluation_order("""
+            def check():
+                def identity(v):
+                    return v
+                def collect(**kwargs):
+                    return kwargs
+                mapping = {"a": 1}
+                try:
+                    assert collect(**mapping, b=identity(mapping := {"a": 9})) == {"a": 1, "b": {"a": 9}}
+                except AssertionError:
+                    return "raised", mapping
+                return "passed", mapping
+            """)
+
     def test_ifexp_branches_in_order(self) -> None:
         """Guard: the condition is evaluated before the selected branch."""
         assert_evaluation_order("""