feat(rewrite): introspect the condition of a ternary by RonnyPfannschmidt · Pull Request #14816 · pytest-dev/pytest

RonnyPfannschmidt · 2026-07-31T19:05:27Z

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

A conditional expression showed only its result, so a failure gave no hint which way it went.

assert 0 == 99
 +  where 0 = (... if True else ...)

The branches keep their original nodes: only the selected one may run, so neither can be hoisted into a statement. That is also why this visitor needs no freeze, unlike the subscript container in #14815 — leaving the branches in place leaves them evaluated after the condition, which is Python's order. TestEvaluationOrder::test_ifexp_branches_in_order in #14813 covers that claim rather than leaving it as a comment.

Lands the introspect-ifexp cases of the coverage matrix in #14813, as passing tests (+3).

Supersedes part of #14448.

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

TestAssertionRewriteWalrusOperator predates the coverage matrix and asks its questions through runpytest(): twelve tests that mostly check ret == 0. Nine of them the matrix already answers in-process and more precisely -- what the failure message says, what the operands saw, how often each ran. Two are worth more than a deletion, so they move rather than vanish: assert not (a and ((a := False) is False)) reads back as an introspection case, and the composite chain assert a and True and ((a := False) is False) and (a is False) and ... as an evaluation-order one. That second is why the deletion is not just tidying: it passes on main, and it passes there because of the bug. Main rewrites the walrus target to an internal temp and substitutes the stored NamedExpr into every later read of the name -- including the next statement, so the `assert a is None` that is supposed to verify the outcome re-runs the walrus and creates it. Asked through returned values instead of ret == 0, main fails the case. TestIssue14445 loses the four tests that restate matrix single-evaluation entries this PR already adds, and keeps the two reproducers from the issue. One test survives in place: nothing the rewriter stores may outlive a statement, which needs two tests in one module to observe and so cannot be said in-process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

…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>

The rewriter only ever visits expressions inside an assert condition, so an attribute always arrives in Load context and the fallback never runs. Removing it keeps the next visitor from copying a guard that cannot fire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A subscript was opaque: the message showed the value it produced with no indication of which container or key it came from. Decompose it the way attribute access already is. The container goes through visit_operand() because taking the expression away from generic_visit() takes away the hoisting that kept it ordered -- without that, `assert box[identity(box := other)] == 1` would start reading the post-walrus container. The order-axis guard in the coverage matrix fails if this is dropped. Slices keep the generic treatment; decomposing start/stop/step is rarely what a failure message needs. Closes the introspect-subscript group in the coverage matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A conditional expression showed only its result, so a failure gave no hint which way it went. Introspect the condition and report it as "(... if <cond> else ...)". The branches keep their original nodes: only the selected one may run, so neither can be hoisted into a statement. That leaves them evaluated after the condition, which is Python's order, so unlike the subscript container they need no freeze -- the order-axis guard covers it. Closes the introspect-ifexp group in the coverage matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

RonnyPfannschmidt force-pushed the ronny/rewrite-visit-ifexp branch from f3d9fd1 to 09585c5 Compare July 31, 2026 19:05

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

This was referenced Jul 31, 2026

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

Draft

feat(rewrite): address assertion rewriting blind spots with systematic tests #14448

Closed

RonnyPfannschmidt force-pushed the ronny/rewrite-visit-ifexp branch from 09585c5 to 8bc3e80 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/rewrite-visit-ifexp branch from 8bc3e80 to cb650f7 Compare August 10, 2026 07:23

RonnyPfannschmidt force-pushed the ronny/rewrite-visit-ifexp branch from cb650f7 to ec9a6be 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 force-pushed the ronny/rewrite-visit-ifexp branch 3 times, most recently from 0e9c4fd to 23ec438 Compare August 26, 2026 09:33

RonnyPfannschmidt and others added 14 commits August 26, 2026 12:03

Add changelog fragment for pytest-dev#14445

54bd8e9

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

RonnyPfannschmidt force-pushed the ronny/rewrite-visit-ifexp branch from 23ec438 to c812cee Compare August 26, 2026 10:04

Navigation Menu

Uh oh!

Uh oh!

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

feat(rewrite): introspect the condition of a ternary#14816
RonnyPfannschmidt wants to merge 14 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:ronny/rewrite-visit-ifexp

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