Skip to content

Commit eac4344

Browse files
knutwannhedenknutwannheden
authored
Fix MigrateCollections* template errors on complex expression contexts (#1067)
The `MigrateCollections{Empty,Singleton,Unmodifiable}*` family applied `JavaTemplate` in `.contextSensitive()` mode. When the `Collections.emptyList()` (etc.) call sat inside a switch expression (`case 0 -> Collections.emptyList()`), a ternary branch, a method argument, or similar non-statement contexts, the block-statement stub generator in rewrite-core either produced 0 statements (for `J.SwitchExpression` parents, which it does not handle) or 2 statements (for some substitutions with method-invocation values), causing `IllegalArgumentException: Expected a template that would generate exactly one statement to replace one statement, but generated {0,2}`. Dropping `.contextSensitive()` makes these simple static-factory templates context-free, so the stub is a trivial `class Template {{ Object o = List.of(); }}` that always parses to one statement regardless of surrounding syntax. Also guard every `MigrateCollections*` recipe with `KotlinFileChecker`/`GroovyFileChecker` preconditions — consistent with `ReplaceUnusedVariablesWithUnderscore` and `IfElseIfConstructToSwitch` — since `JavaTemplate` cannot reliably target non-Java trees, and the idiomatic replacement in Kotlin (`emptyList()`) and Groovy (`[]`) isn't `List.of()` anyway.
1 parent 0ca5cc8 commit eac4344

12 files changed

Lines changed: 383 additions & 14 deletions

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsEmptyList.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.openrewrite.java.search.UsesJavaVersion;
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
30+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
2931

3032
public class MigrateCollectionsEmptyList extends Recipe {
3133
private static final MethodMatcher EMPTY_LIST = new MethodMatcher("java.util.Collections emptyList()");
@@ -39,7 +41,9 @@ public class MigrateCollectionsEmptyList extends Recipe {
3941
@Override
4042
public TreeVisitor<?, ExecutionContext> getVisitor() {
4143
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
42-
new UsesMethod<>(EMPTY_LIST));
44+
new UsesMethod<>(EMPTY_LIST),
45+
Preconditions.not(new KotlinFileChecker<>()),
46+
Preconditions.not(new GroovyFileChecker<>()));
4347
return Preconditions.check(check, new JavaIsoVisitor<ExecutionContext>() {
4448

4549
@Override
@@ -50,7 +54,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
5054
maybeRemoveImport("java.util.Collections");
5155
maybeAddImport("java.util.List");
5256
return JavaTemplate.builder("List.of()")
53-
.contextSensitive()
5457
.imports("java.util.List")
5558
.build()
5659
.apply(updateCursor(m), m.getCoordinates().replace());

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsEmptyMap.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.openrewrite.java.search.UsesJavaVersion;
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
30+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
2931

3032
public class MigrateCollectionsEmptyMap extends Recipe {
3133
private static final MethodMatcher EMPTY_MAP = new MethodMatcher("java.util.Collections emptyMap()");
@@ -39,7 +41,9 @@ public class MigrateCollectionsEmptyMap extends Recipe {
3941
@Override
4042
public TreeVisitor<?, ExecutionContext> getVisitor() {
4143
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
42-
new UsesMethod<>(EMPTY_MAP));
44+
new UsesMethod<>(EMPTY_MAP),
45+
Preconditions.not(new KotlinFileChecker<>()),
46+
Preconditions.not(new GroovyFileChecker<>()));
4347
return Preconditions.check(check, new JavaIsoVisitor<ExecutionContext>() {
4448

4549
@Override
@@ -50,7 +54,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
5054
maybeRemoveImport("java.util.Collections");
5155
maybeAddImport("java.util.Map");
5256
return JavaTemplate.builder("Map.of()")
53-
.contextSensitive()
5457
.imports("java.util.Map")
5558
.build()
5659
.apply(updateCursor(m), m.getCoordinates().replace());

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsEmptySet.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.openrewrite.java.search.UsesJavaVersion;
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
30+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
2931

3032
public class MigrateCollectionsEmptySet extends Recipe {
3133
private static final MethodMatcher EMPTY_SET = new MethodMatcher("java.util.Collections emptySet()");
@@ -39,7 +41,9 @@ public class MigrateCollectionsEmptySet extends Recipe {
3941
@Override
4042
public TreeVisitor<?, ExecutionContext> getVisitor() {
4143
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
42-
new UsesMethod<>(EMPTY_SET));
44+
new UsesMethod<>(EMPTY_SET),
45+
Preconditions.not(new KotlinFileChecker<>()),
46+
Preconditions.not(new GroovyFileChecker<>()));
4347
return Preconditions.check(check, new JavaIsoVisitor<ExecutionContext>() {
4448

4549
@Override
@@ -50,7 +54,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
5054
maybeRemoveImport("java.util.Collections");
5155
maybeAddImport("java.util.Set");
5256
return JavaTemplate.builder("Set.of()")
53-
.contextSensitive()
5457
.imports("java.util.Set")
5558
.build()
5659
.apply(updateCursor(m), m.getCoordinates().replace());

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsSingletonList.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.openrewrite.java.tree.JavaType;
2727
import org.openrewrite.java.tree.JavaType.ShallowClass;
2828
import org.openrewrite.java.tree.Space;
29+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
30+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
2931

3032
import static java.util.Collections.emptyList;
3133

@@ -41,7 +43,9 @@ public class MigrateCollectionsSingletonList extends Recipe {
4143
@Override
4244
public TreeVisitor<?, ExecutionContext> getVisitor() {
4345
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
44-
new UsesMethod<>(SINGLETON_LIST), new NoMissingTypes());
46+
new UsesMethod<>(SINGLETON_LIST), new NoMissingTypes(),
47+
Preconditions.not(new KotlinFileChecker<>()),
48+
Preconditions.not(new GroovyFileChecker<>()));
4549
return Preconditions.check(check, new JavaIsoVisitor<ExecutionContext>() {
4650
@Override
4751
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsSingletonMap.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.Expression;
2929
import org.openrewrite.java.tree.J;
30+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
31+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
3032

3133
import java.util.List;
3234
import java.util.StringJoiner;
@@ -42,7 +44,11 @@ public class MigrateCollectionsSingletonMap extends Recipe {
4244

4345
@Override
4446
public TreeVisitor<?, ExecutionContext> getVisitor() {
45-
return Preconditions.check(Preconditions.and(new UsesJavaVersion<>(9), new UsesMethod<>(SINGLETON_MAP)), new JavaVisitor<ExecutionContext>() {
47+
return Preconditions.check(Preconditions.and(
48+
new UsesJavaVersion<>(9),
49+
new UsesMethod<>(SINGLETON_MAP),
50+
Preconditions.not(new KotlinFileChecker<>()),
51+
Preconditions.not(new GroovyFileChecker<>())), new JavaVisitor<ExecutionContext>() {
4652
@Override
4753
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
4854
J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
@@ -54,7 +60,6 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
5460
args.forEach(o -> mapOf.add("#{any()}"));
5561

5662
return JavaTemplate.builder(mapOf.toString())
57-
.contextSensitive()
5863
.imports("java.util.Map")
5964
.build()
6065
.apply(

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsSingletonSet.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.openrewrite.java.search.UsesJavaVersion;
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
30+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
2931

3032
public class MigrateCollectionsSingletonSet extends Recipe {
3133
private static final MethodMatcher SINGLETON_SET = new MethodMatcher("java.util.Collections singleton(..)", true);
@@ -39,7 +41,9 @@ public class MigrateCollectionsSingletonSet extends Recipe {
3941
@Override
4042
public TreeVisitor<?, ExecutionContext> getVisitor() {
4143
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
42-
new UsesMethod<>(SINGLETON_SET));
44+
new UsesMethod<>(SINGLETON_SET),
45+
Preconditions.not(new KotlinFileChecker<>()),
46+
Preconditions.not(new GroovyFileChecker<>()));
4347
return Preconditions.check(check, new JavaVisitor<ExecutionContext>() {
4448
@Override
4549
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
@@ -49,7 +53,6 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
4953
maybeRemoveImport("java.util.Collections");
5054
maybeAddImport("java.util.Set");
5155
return JavaTemplate.builder("Set.of(#{any()})")
52-
.contextSensitive()
5356
.imports("java.util.Set")
5457
.build()
5558
.apply(updateCursor(m), m.getCoordinates().replace(), m.getArguments().get(0));

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsUnmodifiableList.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.openrewrite.java.search.UsesJavaVersion;
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
30+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
2931

3032
public class MigrateCollectionsUnmodifiableList extends Recipe {
3133
private static final MethodMatcher UNMODIFIABLE_LIST = new MethodMatcher("java.util.Collections unmodifiableList(java.util.List)", true);
@@ -40,7 +42,9 @@ public class MigrateCollectionsUnmodifiableList extends Recipe {
4042
@Override
4143
public TreeVisitor<?, ExecutionContext> getVisitor() {
4244
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
43-
new UsesMethod<>(UNMODIFIABLE_LIST));
45+
new UsesMethod<>(UNMODIFIABLE_LIST),
46+
Preconditions.not(new KotlinFileChecker<>()),
47+
Preconditions.not(new GroovyFileChecker<>()));
4448
return Preconditions.check(check, new JavaVisitor<ExecutionContext>() {
4549
@Override
4650
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {

‎src/main/java/org/openrewrite/java/migrate/util/MigrateCollectionsUnmodifiableSet.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.openrewrite.java.search.UsesMethod;
2828
import org.openrewrite.java.tree.Expression;
2929
import org.openrewrite.java.tree.J;
30+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
31+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
3032

3133
import java.util.List;
3234
import java.util.StringJoiner;
@@ -44,7 +46,9 @@ public class MigrateCollectionsUnmodifiableSet extends Recipe {
4446
@Override
4547
public TreeVisitor<?, ExecutionContext> getVisitor() {
4648
TreeVisitor<?, ExecutionContext> check = Preconditions.and(new UsesJavaVersion<>(9),
47-
new UsesMethod<>(UNMODIFIABLE_SET));
49+
new UsesMethod<>(UNMODIFIABLE_SET),
50+
Preconditions.not(new KotlinFileChecker<>()),
51+
Preconditions.not(new GroovyFileChecker<>()));
4852
return Preconditions.check(check, new JavaVisitor<ExecutionContext>() {
4953
@Override
5054
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
@@ -62,7 +66,6 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
6266
args.forEach(o -> setOf.add("#{any()}"));
6367

6468
return JavaTemplate.builder(setOf.toString())
65-
.contextSensitive()
6669
.imports("java.util.Set")
6770
.build()
6871
.apply(updateCursor(m), m.getCoordinates().replace(), args.toArray());

‎src/test/java/org/openrewrite/java/migrate/util/MigrateCollectionsEmptyListTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,93 @@ class Test {
8181
)
8282
);
8383
}
84+
85+
@Test
86+
void emptyListAsMethodArgument() {
87+
rewriteRun(
88+
//language=java
89+
java(
90+
"""
91+
import java.util.*;
92+
93+
class Test {
94+
void use(List<String> l) {}
95+
void call() {
96+
use(Collections.emptyList());
97+
}
98+
}
99+
""",
100+
"""
101+
import java.util.List;
102+
103+
class Test {
104+
void use(List<String> l) {}
105+
void call() {
106+
use(List.of());
107+
}
108+
}
109+
"""
110+
)
111+
);
112+
}
113+
114+
@Test
115+
void emptyListInTernary() {
116+
rewriteRun(
117+
//language=java
118+
java(
119+
"""
120+
import java.util.*;
121+
122+
class Test {
123+
List<String> get(boolean flag, List<String> other) {
124+
return flag ? Collections.emptyList() : other;
125+
}
126+
}
127+
""",
128+
"""
129+
import java.util.List;
130+
131+
class Test {
132+
List<String> get(boolean flag, List<String> other) {
133+
return flag ? List.of() : other;
134+
}
135+
}
136+
"""
137+
)
138+
);
139+
}
140+
141+
@Test
142+
void emptyListInSwitchExpression() {
143+
rewriteRun(
144+
//language=java
145+
java(
146+
"""
147+
import java.util.*;
148+
149+
class Test {
150+
List<String> get(int i) {
151+
return switch (i) {
152+
case 0 -> Collections.emptyList();
153+
default -> null;
154+
};
155+
}
156+
}
157+
""",
158+
"""
159+
import java.util.List;
160+
161+
class Test {
162+
List<String> get(int i) {
163+
return switch (i) {
164+
case 0 -> List.of();
165+
default -> null;
166+
};
167+
}
168+
}
169+
"""
170+
)
171+
);
172+
}
84173
}

‎src/test/java/org/openrewrite/java/migrate/util/MigrateCollectionsEmptyMapTest.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,66 @@ class Test {
8181
)
8282
);
8383
}
84+
85+
@Test
86+
void emptyMapAsArgument() {
87+
rewriteRun(
88+
//language=java
89+
java(
90+
"""
91+
import java.util.*;
92+
93+
class Test {
94+
void use(Map<String, String> m) {}
95+
void call() {
96+
use(Collections.emptyMap());
97+
}
98+
}
99+
""",
100+
"""
101+
import java.util.Map;
102+
103+
class Test {
104+
void use(Map<String, String> m) {}
105+
void call() {
106+
use(Map.of());
107+
}
108+
}
109+
"""
110+
)
111+
);
112+
}
113+
114+
@Test
115+
void emptyMapInSwitchExpression() {
116+
rewriteRun(
117+
//language=java
118+
java(
119+
"""
120+
import java.util.*;
121+
122+
class Test {
123+
Map<String, String> get(int i) {
124+
return switch (i) {
125+
case 0 -> Collections.emptyMap();
126+
default -> null;
127+
};
128+
}
129+
}
130+
""",
131+
"""
132+
import java.util.Map;
133+
134+
class Test {
135+
Map<String, String> get(int i) {
136+
return switch (i) {
137+
case 0 -> Map.of();
138+
default -> null;
139+
};
140+
}
141+
}
142+
"""
143+
)
144+
);
145+
}
84146
}

0 commit comments

Comments
 (0)
⚡