Skip to content

Commit cde569b

Browse files
Python RPC: don't send prefix/markers for delegating wrapper types (#7424)
`Py.ExpressionStatement` and `Py.StatementExpression` delegate `prefix` and `markers` to their wrapped child. The Java receiver and the Python sender already skip them in `preVisit`, but the Java sender was emitting them anyway. The Python receiver tried to drain those extra messages with `q.receive(None)`, which only consumes the header — not the nested `Space` comments-list and whitespace messages. The queue then desynchronized, and a later `receive_list_defined` for `Space.comments` under a nested node (typically the expression under a `Py.Await`) hit a `NO_CHANGE` where it expected the positions array, producing: Expected positions array but got: RpcObjectData(state=NO_CHANGE, value=None, ...) The failure only surfaced for CHANGE (recipe-modified tree), not ADD, because on ADD the receiver can recover via the `valueType` on the header. Flagship recipes like `Common static analysis issues` regularly hit this on Python sources. Align the Java sender with the existing contract: send only `id` for `Py.ExpressionStatement` / `Py.StatementExpression`. Simplify the Python receiver to match. A new round-trip test reproduces the production signature (fails without the Java sender change).
1 parent 5a3cbe7 commit cde569b

3 files changed

Lines changed: 143 additions & 19 deletions

File tree

rewrite-python/rewrite/src/rewrite/rpc/python_receiver.py

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -81,27 +81,12 @@ def _visit(self, tree: Any, q: RpcReceiveQueue) -> Any:
8181
if tree is None:
8282
return None
8383

84-
# First handle common J fields via pre_visit
85-
# Java's preVisit always sends id, prefix, markers for all J elements.
86-
# ExpressionStatement and StatementExpression delegate prefix/markers to their child,
87-
# but we still need to receive them to stay in sync with the queue.
84+
# First handle common J fields via pre_visit.
85+
# ExpressionStatement and StatementExpression delegate prefix/markers to their
86+
# wrapped child: the sender skips them here and sends them via the child's preVisit.
8887
if isinstance(tree, J):
89-
if isinstance(tree, ExpressionStatement):
90-
# Java sends id, prefix, markers even though prefix/markers delegate to expression
91-
# We must receive them to stay in sync, but only use the id
92-
# Note: tree.prefix/markers would fail on fresh instances since _expression is None,
93-
# so we pass None as the before value - the RPC data contains the actual values anyway.
88+
if isinstance(tree, (ExpressionStatement, StatementExpression)):
9489
new_id = q.receive(tree.id)
95-
q.receive(None) # Receive but discard prefix - delegated to expression
96-
q.receive(None) # Receive but discard markers - delegated to expression
97-
tree = tree.replace(id=new_id) if new_id is not tree.id else tree
98-
elif isinstance(tree, StatementExpression):
99-
# Java sends id, prefix, markers even though prefix/markers delegate to statement
100-
# We must receive them to stay in sync, but only use the id
101-
# Note: tree.prefix/markers would fail on fresh instances since _statement is None
102-
new_id = q.receive(tree.id)
103-
q.receive(None) # Receive but discard prefix - delegated to statement
104-
q.receive(None) # Receive but discard markers - delegated to statement
10590
tree = tree.replace(id=new_id) if new_id is not tree.id else tree
10691
else:
10792
tree = self._pre_visit(tree, q)

rewrite-python/src/main/java/org/openrewrite/python/internal/rpc/PythonSender.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ public class PythonSender extends PythonVisitor<RpcSendQueue> {
4545
@Override
4646
public J preVisit(J j, RpcSendQueue q) {
4747
q.getAndSend(j, Tree::getId);
48+
if (j instanceof Py.ExpressionStatement || j instanceof Py.StatementExpression) {
49+
// prefix and markers delegate to the wrapped child, which receives them
50+
// through its own preVisit. Sending them here would duplicate the data
51+
// and the receiver skips them anyway.
52+
return j;
53+
}
4854
q.getAndSend(j, J::getPrefix, space -> visitSpace(space, q));
4955
q.getAndSend(j, Tree::getMarkers);
5056

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/*
2+
* Copyright 2026 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.python.internal.rpc;
17+
18+
import org.junit.jupiter.api.BeforeEach;
19+
import org.junit.jupiter.api.Test;
20+
import org.openrewrite.Tree;
21+
import org.openrewrite.java.tree.J;
22+
import org.openrewrite.java.tree.JavaType;
23+
import org.openrewrite.java.tree.Space;
24+
import org.openrewrite.marker.Markers;
25+
import org.openrewrite.python.tree.Py;
26+
import org.openrewrite.rpc.RpcObjectData;
27+
import org.openrewrite.rpc.RpcReceiveQueue;
28+
import org.openrewrite.rpc.RpcSendQueue;
29+
30+
import java.nio.file.Path;
31+
import java.util.ArrayDeque;
32+
import java.util.ArrayList;
33+
import java.util.Collections;
34+
import java.util.Deque;
35+
import java.util.HashMap;
36+
import java.util.IdentityHashMap;
37+
import java.util.List;
38+
import java.util.UUID;
39+
40+
import static org.assertj.core.api.Assertions.assertThat;
41+
42+
class PythonSenderReceiverRoundTripTest {
43+
44+
private Deque<List<RpcObjectData>> batches;
45+
private RpcSendQueue sq;
46+
private RpcReceiveQueue rq;
47+
48+
@BeforeEach
49+
void setUp() {
50+
batches = new ArrayDeque<>();
51+
String sourceFileType = Py.CompilationUnit.class.getName();
52+
IdentityHashMap<Object, Integer> localRefs = new IdentityHashMap<>();
53+
sq = new RpcSendQueue(1, e -> batches.addLast(encode(e)), localRefs, sourceFileType, false);
54+
rq = new RpcReceiveQueue(new HashMap<>(), batches::removeFirst, sourceFileType, null);
55+
}
56+
57+
@Test
58+
void expressionStatementWithAwaitChangeRoundTrip() {
59+
// Reproduces the production failure where the Python receiver hits
60+
// "Expected positions array but got: RpcObjectData(NO_CHANGE)" inside
61+
// `_receive_space` → `space.comments` under a nested Py.Await.
62+
//
63+
// Root cause: Java's PythonSender.preVisit previously emitted
64+
// id/prefix/markers for every J node, including Py.ExpressionStatement
65+
// and Py.StatementExpression, even though their prefix/markers delegate
66+
// to the wrapped child and the receiver already skips them. When the
67+
// wrapped expression's prefix was CHANGE (not ADD), the extra Space
68+
// sub-messages (comments list header + whitespace) leaked into the
69+
// queue and eventually misaligned a nested receive_list.
70+
//
71+
// This test drives a round trip where the await's expression prefix
72+
// changes between `before` and `after`, which before the fix caused
73+
// a queue desynchronization visible as a wrong value in a later field.
74+
Py.ExpressionStatement before = expressionStatementOfAwait("x", Space.EMPTY);
75+
Py.ExpressionStatement after = expressionStatementOfAwait("x", Space.SINGLE_SPACE);
76+
77+
// send the diff (CHANGE path)
78+
sq.send(after, before, null);
79+
sq.flush();
80+
81+
Py.ExpressionStatement received = rq.receive(before);
82+
83+
assertThat(received).isNotNull();
84+
assertThat(received.getId()).isEqualTo(after.getId());
85+
assertThat(received.getExpression()).isInstanceOf(Py.Await.class);
86+
Py.Await receivedAwait = (Py.Await) received.getExpression();
87+
assertThat(receivedAwait.getPrefix().getWhitespace())
88+
.as("await prefix whitespace must survive the CHANGE round trip")
89+
.isEqualTo(" ");
90+
assertThat(receivedAwait.getPrefix().getComments()).isEmpty();
91+
}
92+
93+
@Test
94+
void expressionStatementWithAwaitAddRoundTrip() {
95+
// Sanity check: the first-time ADD path also works (it did before the fix
96+
// because the receiver could recover via the value_type on the header).
97+
Py.ExpressionStatement after = expressionStatementOfAwait("x", Space.EMPTY);
98+
99+
sq.send(after, null, null);
100+
sq.flush();
101+
102+
Py.ExpressionStatement received = rq.receive(null);
103+
104+
assertThat(received).isNotNull();
105+
assertThat(received.getId()).isEqualTo(after.getId());
106+
assertThat(received.getExpression()).isInstanceOf(Py.Await.class);
107+
}
108+
109+
private static Py.ExpressionStatement expressionStatementOfAwait(String name, Space awaitPrefix) {
110+
UUID esId = Tree.randomId();
111+
UUID awaitId = Tree.randomId();
112+
UUID identId = Tree.randomId();
113+
J.Identifier identifier = new J.Identifier(
114+
identId, Space.EMPTY, Markers.EMPTY,
115+
Collections.emptyList(), name, null, null
116+
);
117+
Py.Await await = new Py.Await(awaitId, awaitPrefix, Markers.EMPTY, identifier, (JavaType) null);
118+
return new Py.ExpressionStatement(esId, await);
119+
}
120+
121+
private List<RpcObjectData> encode(List<RpcObjectData> batch) {
122+
List<RpcObjectData> encoded = new ArrayList<>();
123+
for (RpcObjectData data : batch) {
124+
if (data.getValue() instanceof UUID || data.getValue() instanceof Path) {
125+
encoded.add(new RpcObjectData(data.getState(), data.getValueType(),
126+
data.getValue().toString(), data.getRef(), false));
127+
} else {
128+
encoded.add(data);
129+
}
130+
}
131+
return encoded;
132+
}
133+
}

0 commit comments

Comments
 (0)