Skip to content

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

Draft
RonnyPfannschmidt wants to merge 6 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:ronny/rewrite-flat-method-call
Draft

RonnyPfannschmidt wants to merge 6 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:ronny/rewrite-flat-method-call

Conversation

@RonnyPfannschmidt

@RonnyPfannschmidt RonnyPfannschmidt commented Jul 31, 2026 •

Copy link
Copy Markdown
Member

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

obj.method() reported the bound method as an intermediate of its own:

where 42 = compute()
  where compute = Obj().compute

which spends a line on something nobody asked about. Build the explanation from the receiver and the attribute name instead:

where 42 = Obj().compute()

The bound method keeps its own temporary even though it no longer has its own explanation, because Python looks it up before evaluating the arguments. Inlining the attribute into the rewritten call — which is what the obvious implementation does — moves the lookup after the arguments, and with it the read of the receiver. That version fails two guards:

I wrote it the inline way first and the guards caught it, which is the whole argument for landing the matrix ahead of the visitors.

testing/python/raises_group.py::test_assert_matches asserts on an exact rewritten message and is updated for the flat format.

Lands the introspect-method-call-flat cases of the coverage matrix in #14813 as passing tests, plus the new lookup-order guard above (+6).

Supersedes part of #14448.

@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from 3a5cbb0 to e7ae134 Compare July 31, 2026 19:06
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jul 31, 2026
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from e7ae134 to 2480b28 Compare July 31, 2026 20:18
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from 2480b28 to aacbd89 Compare August 10, 2026 07:24
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from aacbd89 to 77dd763 Compare August 20, 2026 04:35
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch 4 times, most recently from aa87473 to 3403e04 Compare August 26, 2026 10:04
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from 3403e04 to d1741e4 Compare September 1, 2026 11:54
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from d1741e4 to cbcda2d Compare September 15, 2026 11:34
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from cbcda2d to 5e842e9 Compare September 23, 2026 07:00
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch 2 times, most recently from fe4b892 to 01d8827 Compare October 11, 2026 07:26
RonnyPfannschmidt and others added 6 commits October 14, 2026 22:10
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>
obj.method() reported the bound method as an intermediate of its own:

    where 42 = compute()
      where compute = Obj().compute

which spends a line on something nobody asked about.  Build the
explanation from the receiver and the attribute name instead:

    where 42 = Obj().compute()

The bound method keeps its own temporary even though it no longer has
its own explanation, because Python looks it up before evaluating the
arguments -- inlining the attribute into the rewritten call would move
the lookup after them, and with it the read of the receiver.  Both
order-axis guards in the coverage matrix fail if that temporary goes.

Closes the introspect-method-call-flat group in the coverage matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the ronny/rewrite-flat-method-call branch from 01d8827 to 31ef28c Compare October 14, 2026 20:28

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant