Fix walrus double eval#14446
Draft
shuckc wants to merge 3 commits intopytest-dev:mainfrom
Draft
Conversation
….NamedExpr node directly, which was then referenced in multiple places in the rewritten AST (the comparison statement, the results tuple for _call_reprcompare, and format expressions). Each reference re-evaluated the walrus operator at runtime, causing side effects to fire multiple times. Fix (in src/_pytest/assertion/rewrite.py): 1. visit_NamedExpr: Returns the NamedExpr inline (preserving evaluation order for function args and comparators) but references the target variable for display instead of re-evaluating the expression. 2. visit_Compare (left side): When comp.left is a NamedExpr, hoists it into a temp variable before processing comparators. This ensures comparators that reference the walrus target see the assigned value (fixing the assert (obj := "foo") == f(obj) case). 3. visit_Compare (comparators): For NamedExpr comparators, uses the target variable Name in results (failure message) instead of the NamedExpr node, preventing re-evaluation in the failure path. Also saves the left value in a temp when a walrus will overwrite it (for correct failure messages). 4. visit_BoolOp: Saves the short-circuit condition in a per-operand temp variable for the explanation path, so walrus modifications to the original variable don't corrupt the failure message. 5. visit_Assert: Clears variables_overwrite at the start of each assert, preventing stale walrus mappings from leaking between statements. Closes pytest-dev#14445
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses issue #14445
Root cause: The assertion rewriter's
visit_NamedExprreturned theast.NamedExprnode directly, which was then referenced in multiple places in the rewritten AST (the comparison statement, the results tuple for_call_reprcompare, and format expressions). Each reference re-evaluated the walrus operator at runtime, causing side effects to fire multiple times.Fix (in
src/_pytest/assertion/rewrite.py):visit_NamedExpr: Returns theNamedExprinline (preserving evaluation order for function args and comparators) but references the target variable for display instead of re-evaluating the expression.visit_Compare(left side): Whencomp.leftis aNamedExpr, hoists it into a temp variable before processing comparators. This ensures comparators that reference the walrus target see the assigned value(fixing the assert
(obj := "foo") == f(obj)case).visit_Compare(comparators): ForNamedExprcomparators, uses the target variable Name in results (failure message) instead of theNamedExprnode, preventing re-evaluation in the failure path. Also saves theleft value in a temp when a walrus will overwrite it (for correct failure messages).
visit_BoolOp: Saves the short-circuit condition in a per-operand temp variable for the explanation path, so walrus modifications to the original variable don't corrupt the failure message.visit_Assert: Clears variables_overwrite at the start of each assert, preventing stale walrus mappings from leaking between statements.Note this PR updates the error message emitted by two tasks - I believe they present the value from before, rather than at the time the assertion fails.
TODO:
PR Checklist:
closes #XYZWto the PR description and/or commits (whereXYZWis the issue number). See the github docs for more information.Co-authored-bycommit trailers.changelogdirectory, with a name like<ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.AUTHORSin alphabetical order.