fix(rewrite): freeze walrus and starred operands too by RonnyPfannschmidt · Pull Request #14814 · pytest-dev/pytest

RonnyPfannschmidt · 2026-07-31T19:04:23Z

Stacked on #14447 → #14921 → #14813. Its diff includes theirs.

visit_operand() (added in #14447) only froze a bare name, so two other unhoisted operands kept being evaluated after everything that follows them:

assert collect((x := 1), identity(x := 2)) == (1, 2)      # passes (1, 2) to plain Python, (2, 2) rewritten
assert collect(*items, identity(items := [9])) == (1, [9]) # passes (1, [9]) plain, (9, [9]) rewritten

A walrus operator left in place assigns only once the enclosing expression is assembled, which is after the later arguments have run — so the earlier argument saw the later assignment. A starred argument hid its value inside an ast.Starred, where the existing Name check could not reach it.

Lands the order-starred-argument case and the last order-call-argument case of the coverage matrix in #14813, as passing tests (+2).

The starred case was found by the matrix, not by hand — it is what the order-* axis is for.

The rewriter is read through its failure messages; what it actually generates is invisible unless one hand-writes an ast.unparse harness. Reviewing a change to it means asking "what does the emitted code look like now, and how does that differ from what it was". Add a script that answers exactly that: it dumps a snippet's rewritten form -- as source, or as an AST -- for the source as written, for this checkout, or for any released pytest version, and diffs two of them. Released versions are fetched on demand via ``uv run --with``, so no version under comparison has to be installed. By default it diffs the snippet as written against this checkout, which is the "show me what rewriting does here" case: python scripts/diff-assert-rewrite.py -c 'assert (x := f()) and (x := False)' It exits 1 when the sides differ, so it can also be used as a check.

Fixes pytest-dev#14445 - assertion rewriting evaluated NamedExpr (:=) expressions multiple times, causing side effects to fire repeatedly. The root cause was the `variables_overwrite` mechanism which stored and re-evaluated NamedExpr AST nodes in subsequent assertions, in `_call_reprcompare`'s results tuple, and in explanation formatting. The fix: - visit_NamedExpr: reference the target variable in explanations instead of re-evaluating the full expression - visit_Compare: assign left-side NamedExpr to a temp before right-side hoisting; freeze left_res when a comparator walrus targets the same name; replace NamedExpr entries in `results` with target variables - visit_BoolOp: capture short-circuit condition in a stable temp for the explanation path; remove walrus target rename logic - visit_Call: remove variables_overwrite substitution (walrus now properly assigns to user variables in its natural evaluation position) - Remove variables_overwrite, scope tracking, Sentinel class Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

Add tests for two remaining walrus double-evaluation scenarios: - Bare NamedExpr as BoolOp operand evaluated twice via condition check - Same walrus target in chained comparison evaluated multiple times Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

Use the already-assigned res_var to build the short-circuit condition instead of the raw visitor result, preventing bare NamedExpr operands from being evaluated a second time when checking truthiness. Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

In a chained comparison like `(x := f()) < (x := g()) < (x := h())`, each NamedExpr comparator is now assigned to a temp variable so it evaluates exactly once. Previously the raw NamedExpr node would be reused as left_res in the next iteration, causing double evaluation. Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

@bluetech

When multiple walrus operators target the same variable in a BoolOp (e.g., `assert (x := side_effect()) and (x := False)`), the assertion explanation previously showed the final value of `x` for all operands because the format context evaluated lazily after all operands ran. Fix by tracking Name/NamedExpr operand values in stable @py_assert variables (via self.assign) immediately after evaluation, then pointing the explanation format context at the tracked copy. This uses the same value-tracking mechanism already used by visit_Call, visit_Attribute, etc. Fixes the case reported by @bluetech in PR review. Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

Replace the blanket snapshot-all-operands approach with a targeted one: pre-scan the BoolOp to find walrus targets, then only snapshot operands whose value a later walrus would corrupt. Snapshot rules: - NamedExpr (non-last): always, to avoid re-evaluating side effects - Name with later walrus conflict: to freeze the pre-overwrite value - Everything else: use res directly (stable @py_assert or plain name) Non-walrus BoolOps now generate identical code to 8.3.5 (no snapshots). Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>

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

visit_operand() only froze a bare name, so two other unhoisted operands kept being evaluated after everything that follows them: assert collect((x := 1), identity(x := 2)) == (1, 2) assert collect(*items, identity(items := [9])) == (1, [9]) A walrus operator left in place assigns once the enclosing expression is assembled, which is after the later arguments have run -- so the earlier argument saw the later assignment. A starred argument hid its value inside an ast.Starred, where the existing Name check could not see it. Closes the order-starred-argument group and the remaining order-call-argument entry in the coverage matrix.

…eeds visit_operand freezes a walrus operand whenever anything follows it, and a comparison always has at least one comparator -- so by the time visit_Compare looks at its left operand, a NamedExpr has already been copied into a temporary. The special case that did it here can never run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jul 31, 2026

RonnyPfannschmidt force-pushed the ronny/fix-walrus-raw-operands branch from 16b94a1 to cc6afa4 Compare July 31, 2026 19:04

This was referenced Jul 31, 2026

feat(rewrite): introspect container[key] in failure messages #14815

Draft

feat(rewrite): introspect the condition of a ternary #14816

Draft

feat(rewrite): show a method call on one line #14817

Draft

RonnyPfannschmidt force-pushed the ronny/fix-walrus-raw-operands branch from cc6afa4 to d271ad5 Compare July 31, 2026 20:18

This was referenced Jul 31, 2026

fix(rewrite): freeze operands against any later side effect #14821

Draft

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

Draft

RonnyPfannschmidt force-pushed the ronny/fix-walrus-raw-operands branch from d271ad5 to 5af71a7 Compare August 10, 2026 07:12

RonnyPfannschmidt force-pushed the ronny/fix-walrus-raw-operands branch from 5af71a7 to fd41d89 Compare August 20, 2026 04:35

This was referenced Aug 20, 2026

test(rewrite): record the introspection gaps nothing fixes yet #14916

Draft

test(rewrite): systematic coverage matrix for assertion rewriting #14813

Merged

RonnyPfannschmidt and others added 11 commits August 22, 2026 07:54

Add changelog fragment for pytest-dev#14445

5167153

Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Sonnet 4 <claude@anthropic.com>

RonnyPfannschmidt force-pushed the ronny/fix-walrus-raw-operands branch from fd41d89 to 14cbbbf Compare August 22, 2026 19:37

Navigation Menu

Uh oh!

Uh oh!

fix(rewrite): freeze walrus and starred operands too - #14814

fix(rewrite): freeze walrus and starred operands too#14814
RonnyPfannschmidt wants to merge 11 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:ronny/fix-walrus-raw-operands

RonnyPfannschmidt commented Jul 31, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

RonnyPfannschmidt commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

RonnyPfannschmidt commented Jul 31, 2026 •

edited

Loading