Skip to content

Commit 53b3725

Browse files
timtebeektimtebeek
authored
Fix IfElseIfConstructToSwitch type attribution for non-JDK types (#994)
* Fix IfElseIfConstructToSwitch type attribution for non-JDK types JavaTemplate uses raw string substitution (#{}) for class names in generated switch cases. The template parser can resolve JDK types like Integer (in java.lang) but cannot resolve non-JDK types from just a name string, producing missing type information on case labels. Add fixTypeAttribution() post-processing that restores original type information from the instanceof checks onto the generated switch case variable declarations. Re-enable the recipe in UpgradeToJava21. * Use ListUtils.map in fixTypeAttribution
1 parent eec22b2 commit 53b3725

File tree

3 files changed

+145
-6
lines changed

3 files changed

+145
-6
lines changed

‎src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import lombok.Value;
2020
import org.jspecify.annotations.Nullable;
2121
import org.openrewrite.*;
22+
import org.openrewrite.internal.ListUtils;
2223
import org.openrewrite.java.JavaIsoVisitor;
2324
import org.openrewrite.java.JavaTemplate;
2425
import org.openrewrite.java.JavaVisitor;
@@ -31,12 +32,10 @@
3132
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
3233
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
3334

34-
import java.util.LinkedHashMap;
35-
import java.util.Map;
36-
import java.util.Objects;
37-
import java.util.Optional;
35+
import java.util.*;
3836
import java.util.concurrent.atomic.AtomicBoolean;
3937

38+
import static java.util.Collections.singletonList;
4039
import static org.openrewrite.java.migrate.lang.NullCheck.Matcher.nullCheck;
4140
import static org.openrewrite.java.tree.J.Block.createEmptyBlock;
4241

@@ -218,7 +217,42 @@ public J.Return visitReturn(J.Return return_, AtomicBoolean atomicBoolean) {
218217
}
219218
switchBody.append("}\n");
220219

221-
return JavaTemplate.apply(switchBody.toString(), cursor, if_.getCoordinates().replace(), arguments).withPrefix(if_.getPrefix());
220+
J.Switch result = JavaTemplate.apply(switchBody.toString(), cursor, if_.getCoordinates().replace(), arguments).withPrefix(if_.getPrefix());
221+
return fixTypeAttribution(result);
222+
}
223+
224+
/**
225+
* JavaTemplate uses raw string substitution (#{}), which loses type information
226+
* for non-JDK types. This method restores the original type information from the
227+
* instanceof checks onto the generated switch case labels.
228+
*/
229+
private J.Switch fixTypeAttribution(J.Switch switch_) {
230+
Iterator<J.InstanceOf> instanceOfs = patternMatchers.keySet().iterator();
231+
return switch_.withCases(switch_.getCases().withStatements(
232+
ListUtils.map(switch_.getCases().getStatements(), stmt -> {
233+
if (stmt instanceof J.Case && instanceOfs.hasNext()) {
234+
J.Case case_ = (J.Case) stmt;
235+
if (!case_.getCaseLabels().isEmpty() && case_.getCaseLabels().get(0) instanceof J.VariableDeclarations) {
236+
J.InstanceOf instanceOf = instanceOfs.next();
237+
J.VariableDeclarations varDecl = (J.VariableDeclarations) case_.getCaseLabels().get(0);
238+
// Replace typeExpression with the original clazz (which has proper type info)
239+
varDecl = varDecl.withTypeExpression(
240+
varDecl.getTypeExpression() != null ?
241+
instanceOf.getClazz().withPrefix(varDecl.getTypeExpression().getPrefix()) :
242+
instanceOf.getClazz().withPrefix(Space.EMPTY));
243+
// Fix variable type from original pattern
244+
if (instanceOf.getPattern() instanceof J.Identifier && !varDecl.getVariables().isEmpty()) {
245+
J.Identifier originalPattern = (J.Identifier) instanceOf.getPattern();
246+
J.VariableDeclarations.NamedVariable var0 = varDecl.getVariables().get(0);
247+
varDecl = varDecl.withVariables(singletonList(
248+
var0.withType(originalPattern.getType())
249+
.withName(var0.getName().withType(originalPattern.getType()))));
250+
}
251+
return case_.withCaseLabels(singletonList(varDecl.withPrefix(case_.getCaseLabels().get(0).getPrefix())));
252+
}
253+
}
254+
return stmt;
255+
})));
222256
}
223257

224258
private Optional<Expression> switchOn() {

‎src/main/resources/META-INF/rewrite/java-version-21.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ recipeList:
4444
- org.openrewrite.java.migrate.lang.SwitchCaseAssignmentsToSwitchExpression
4545
- org.openrewrite.java.migrate.lang.SwitchCaseReturnsToSwitchExpression
4646
- org.openrewrite.java.migrate.lang.SwitchExpressionYieldToArrow
47-
#- org.openrewrite.java.migrate.lang.IfElseIfConstructToSwitch # FIXME `casecase` seen near non JDK types
47+
- org.openrewrite.java.migrate.lang.IfElseIfConstructToSwitch
4848
- org.openrewrite.java.migrate.SwitchPatternMatching
4949
- org.openrewrite.java.migrate.lang.NullCheckAsSwitchCase
5050

‎src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,111 @@ static String formatter(Object obj) {
341341
);
342342
}
343343

344+
@Test
345+
void nullCheckWithNonJdkTypes() {
346+
rewriteRun(
347+
java(
348+
"""
349+
public class Dog {}
350+
"""
351+
),
352+
java(
353+
"""
354+
public class Cat {}
355+
"""
356+
),
357+
//language=java
358+
java(
359+
"""
360+
class Test {
361+
static String describe(Object obj) {
362+
String result;
363+
if (obj == null) {
364+
result = "nothing";
365+
} else if (obj instanceof Dog d) {
366+
result = "dog";
367+
} else if (obj instanceof Cat c) {
368+
result = "cat";
369+
} else {
370+
result = "unknown";
371+
}
372+
return result;
373+
}
374+
}
375+
""",
376+
"""
377+
class Test {
378+
static String describe(Object obj) {
379+
String result;
380+
switch (obj) {
381+
case null -> result = "nothing";
382+
case Dog d -> result = "dog";
383+
case Cat c -> result = "cat";
384+
default -> result = "unknown";
385+
}
386+
return result;
387+
}
388+
}
389+
"""
390+
)
391+
);
392+
}
393+
394+
@Test
395+
void threeNonJdkTypes() {
396+
rewriteRun(
397+
java(
398+
"""
399+
public class Dog {}
400+
"""
401+
),
402+
java(
403+
"""
404+
public class Cat {}
405+
"""
406+
),
407+
java(
408+
"""
409+
public class Bird {}
410+
"""
411+
),
412+
//language=java
413+
java(
414+
"""
415+
class Test {
416+
static String describe(Object obj) {
417+
String result;
418+
if (obj instanceof Dog d) {
419+
result = "dog";
420+
} else if (obj instanceof Cat c) {
421+
result = "cat";
422+
} else if (obj instanceof Bird b) {
423+
result = "bird";
424+
} else {
425+
result = "unknown";
426+
}
427+
return result;
428+
}
429+
}
430+
""",
431+
"""
432+
class Test {
433+
static String describe(Object obj) {
434+
String result;
435+
switch (obj) {
436+
case Dog d -> result = "dog";
437+
case Cat c -> result = "cat";
438+
case Bird b -> result = "bird";
439+
default -> result = "unknown";
440+
}
441+
return result;
442+
}
443+
}
444+
"""
445+
)
446+
);
447+
}
448+
344449
@Test
345450
void switchBlockForNestedClasses() {
346451
rewriteRun(

0 commit comments

Comments
 (0)