Skip to content

Commit 4ea1191

Browse files
authored
Skip var when reassignment relies on declared supertype (#1078)
* Skip `var` when reassignment relies on declared supertype When a generic variable is declared as a supertype but initialized with a concrete subtype constructor or method, switching to `var` narrows the inferred type and breaks later reassignments. Detect reassignment within the enclosing method and skip the transformation in that case. Fixes #1077 * Fix flaky `JohnzonJavaxtoJakartaTest` and use `.reduce` in var helper The test's `2.1.\d+` regex matched `<johnzon.version>2.1.0</johnzon.version>` before reaching the jakarta.json-api dependency, so when jakarta.json-api was released as 2.1.3 the expected/actual diverged. Anchor the regex to the jakarta.json-api artifact. Also consolidate the `isReassigned` visitor to use `.reduce(...)` directly, matching the existing idiom in NullCheck and other recipes.
1 parent 1ead1d8 commit 4ea1191

5 files changed

Lines changed: 83 additions & 2 deletions

File tree

src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
import org.jspecify.annotations.Nullable;
2020
import org.openrewrite.Cursor;
2121
import org.openrewrite.internal.ListUtils;
22+
import org.openrewrite.java.JavaIsoVisitor;
2223
import org.openrewrite.java.tree.*;
2324
import org.openrewrite.marker.Markers;
2425

2526
import java.util.List;
27+
import java.util.concurrent.atomic.AtomicBoolean;
2628
import java.util.function.UnaryOperator;
2729

2830
import static java.util.Collections.emptyList;
@@ -233,6 +235,41 @@ public static boolean initializedByStaticMethod(@Nullable Expression initializer
233235
return invocation.getMethodType().hasFlags(Flag.Static);
234236
}
235237

238+
/**
239+
* Determine whether the variable declared in {@code vd} is later reassigned within the
240+
* enclosing method. Reassignments matter when the declared type differs from the
241+
* initializer's concrete type, since switching to {@code var} narrows the inferred type
242+
* and may break subsequent assignments of supertype values.
243+
*
244+
* @param cursor location of the visitor
245+
* @param vd variable definition at hand
246+
* @return true iff the variable is reassigned within the enclosing method
247+
*/
248+
public static boolean isReassigned(Cursor cursor, J.VariableDeclarations vd) {
249+
JavaType.Variable variableType = vd.getVariables().get(0).getVariableType();
250+
if (variableType == null) {
251+
return false;
252+
}
253+
J.MethodDeclaration method = cursor.firstEnclosing(J.MethodDeclaration.class);
254+
if (method == null) {
255+
return false;
256+
}
257+
return new JavaIsoVisitor<AtomicBoolean>() {
258+
@Override
259+
public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean found) {
260+
if (found.get()) {
261+
return assignment;
262+
}
263+
if (assignment.getVariable() instanceof J.Identifier &&
264+
variableType.equals(((J.Identifier) assignment.getVariable()).getFieldType())) {
265+
found.set(true);
266+
return assignment;
267+
}
268+
return super.visitAssignment(assignment, found);
269+
}
270+
}.reduce(method, new AtomicBoolean(false)).get();
271+
}
272+
236273
public static J.VariableDeclarations transformToVar(J.VariableDeclarations vd) {
237274
return transformToVar(vd, it -> it);
238275
}

src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForGenericMethodInvocations.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations v
7979
return vd;
8080
}
8181

82+
// Switching to var narrows the variable type to the initializer's concrete type, which can break later
83+
// reassignments that rely on the declared (super)type. Skip when types differ and the variable is reassigned.
84+
if (!TypeUtils.isOfType(vd.getType(), invocation.getType()) &&
85+
DeclarationCheck.isReassigned(getCursor(), vd)) {
86+
return vd;
87+
}
88+
8289
if (vd.getType() instanceof JavaType.FullyQualified) {
8390
maybeRemoveImport((JavaType.FullyQualified) vd.getType());
8491
}

src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForGenericsConstructors.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.openrewrite.java.tree.J;
2828
import org.openrewrite.java.tree.JavaType;
2929
import org.openrewrite.java.tree.TypeTree;
30+
import org.openrewrite.java.tree.TypeUtils;
3031

3132
import java.util.ArrayList;
3233
import java.util.List;
@@ -78,6 +79,14 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations v
7879
return vd;
7980
}
8081

82+
// Switching to var narrows the variable type to the initializer's concrete type, which can break later
83+
// reassignments that rely on the declared (super)type. Skip when types differ and the variable is reassigned.
84+
Expression initializer = variable.getInitializer();
85+
if (initializer != null && !TypeUtils.isOfType(vd.getType(), initializer.unwrap().getType()) &&
86+
DeclarationCheck.isReassigned(getCursor(), vd)) {
87+
return vd;
88+
}
89+
8190
if (vd.getType() instanceof JavaType.FullyQualified) {
8291
maybeRemoveImport((JavaType.FullyQualified) vd.getType());
8392
}

src/test/java/org/openrewrite/java/migrate/jakarta/JohnzonJavaxtoJakartaTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ void migrateJohnzonDependencies() {
6868
Matcher version = Pattern.compile("<johnzon.version>([0-9]+\\.[0-9]+\\.[0-9]+)</johnzon.version>")
6969
.matcher(actual);
7070

71-
Matcher jsonApiVersion = Pattern.compile("2.1.\\d+").matcher(actual);
71+
Matcher jsonApiVersion = Pattern.compile("(?s)<artifactId>jakarta\\.json-api</artifactId>\\s*<version>(2\\.1\\.\\d+)</version>")
72+
.matcher(actual);
7273
assertThat(jsonApiVersion.find()).describedAs("Expected jakarta.json-api 2.1.x version in %s", actual).isTrue();
7374

7475
assertThat(version.find()).isTrue();
@@ -94,7 +95,7 @@ void migrateJohnzonDependencies() {
9495
</dependency>
9596
</dependencies>
9697
</project>
97-
""".formatted(version.group(1), jsonApiVersion.group(0));
98+
""".formatted(version.group(1), jsonApiVersion.group(1));
9899
})
99100
),
100101
srcMainJava(

src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForGenericsConstructorsTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,33 @@ void m() {
170170
);
171171
}
172172

173+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/1077")
174+
@Test
175+
void reassignedToSupertype() {
176+
//language=java
177+
rewriteRun(
178+
java(
179+
"""
180+
import java.util.HashSet;
181+
import java.util.Set;
182+
183+
class A {
184+
Set<String> transfer(Set<String> in) {
185+
return in;
186+
}
187+
188+
void m() {
189+
Set<String> currentFacts = new HashSet<>();
190+
for (int i = 0; i < 3; i++) {
191+
currentFacts = transfer(currentFacts);
192+
}
193+
}
194+
}
195+
"""
196+
)
197+
);
198+
}
199+
173200
@Test
174201
void javaLowerThan10() {
175202
//language=java

0 commit comments

Comments
 (0)