diff --git a/src/main/java/org/openrewrite/java/migrate/javax/RemoveTemporalAnnotation.java b/src/main/java/org/openrewrite/java/migrate/javax/RemoveTemporalAnnotation.java index 7e71a83aeb..0bb67c01a5 100644 --- a/src/main/java/org/openrewrite/java/migrate/javax/RemoveTemporalAnnotation.java +++ b/src/main/java/org/openrewrite/java/migrate/javax/RemoveTemporalAnnotation.java @@ -114,7 +114,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m } // Remove @Temporal annotation - return (J.VariableDeclarations) new RemoveAnnotation("javax.persistence.Temporal").getVisitor().visit(multiVariable, ctx); + return (J.VariableDeclarations) new RemoveAnnotation("javax.persistence.Temporal").getVisitor().visit(multiVariable, ctx, getCursor().getParentOrThrow()); } } ); diff --git a/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java b/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java index cce4e50f53..20358d6337 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java @@ -23,6 +23,7 @@ import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.NoMissingTypes; import org.openrewrite.java.search.SemanticallyEqual; import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.java.tree.Expression; @@ -55,6 +56,7 @@ public class IfElseIfConstructToSwitch extends Recipe { public TreeVisitor getVisitor() { TreeVisitor preconditions = Preconditions.and( new UsesJavaVersion<>(21), + new NoMissingTypes(), Preconditions.not(new KotlinFileChecker<>()), Preconditions.not(new GroovyFileChecker<>()) ); diff --git a/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java b/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java index f422346870..38368efeee 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java @@ -937,30 +937,110 @@ else if (obj instanceof Long l) { } @Test - void unresolvableTypesKeepCaseSpacing() { - // https://github.com/openrewrite/rewrite/issues/7458 + void voidMethodInvocationsWithChainedInstanceOfPatternMatch() { rewriteRun( - spec -> spec.typeValidationOptions(TypeValidation.none()), + spec -> spec.recipes(new InstanceOfPatternMatch(), new IfElseIfConstructToSwitch()), + //language=java + java( + """ + package com.example; + public class Container { + public void add(Object o) {} + } + """ + ), + //language=java + java( + """ + package com.example; + public class Reference {} + """ + ), + //language=java + java( + """ + package com.example; + public class Element {} + """ + ), + //language=java + java( + """ + package com.example; + public class Qualifier {} + """ + ), //language=java java( """ package com.example; - import com.example.missing.Http2Frame; - import com.example.missing.Http2ResetFrame; - import com.example.missing.Http2GoAwayFrame; class Test { - protected void incrementCounter(Http2Frame frame) { - long errorCode; - if (frame instanceof Http2ResetFrame resetFrame) { - errorCode = resetFrame.errorCode(); - } else if (frame instanceof Http2GoAwayFrame goAwayFrame) { - errorCode = goAwayFrame.errorCode(); + private void addTo(Container container, Object obj) { + if (obj instanceof Reference) { + container.add((Reference) obj); + } else if (obj instanceof Element) { + container.add((Element) obj); } else { - errorCode = -1; + container.add((Qualifier) obj); } } } """, + """ + package com.example; + class Test { + private void addTo(Container container, Object obj) { + switch (obj) { + case Reference reference -> container.add(reference); + case Element element -> container.add(element); + case null, default -> container.add((Qualifier) obj); + } + } + } + """ + ) + ); + } + + @Test + void noChangeWhenInstanceofTypeMissing() { + // Missing type info on the instanceof's type can cause JavaTemplate to mis-parse + // the generated switch, leaking parameter stubs. Stay conservative when types are unknown. + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.none()), + //language=java + java( + """ + package com.example; + import com.example.missing.Container; + import com.example.missing.Reference; + import com.example.missing.Element; + import com.example.missing.Qualifier; + class Test { + private void addTo(Container container, Object obj) { + if (obj instanceof Reference reference) { + container.add(reference); + } else if (obj instanceof Element element) { + container.add(element); + } else { + container.add((Qualifier) obj); + } + } + } + """ + ) + ); + } + + @Test + void noChangeWhenInstanceofTypeUnresolvable() { + // https://github.com/openrewrite/rewrite/issues/7458 + // Previously converted to switch even with unresolvable types; now conservative to + // avoid mis-parsed templates leaking JavaTemplate parameter stubs. + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.none()), + //language=java + java( """ package com.example; import com.example.missing.Http2Frame; @@ -969,10 +1049,12 @@ protected void incrementCounter(Http2Frame frame) { class Test { protected void incrementCounter(Http2Frame frame) { long errorCode; - switch (frame) { - case Http2ResetFrame resetFrame -> errorCode = resetFrame.errorCode(); - case Http2GoAwayFrame goAwayFrame -> errorCode = goAwayFrame.errorCode(); - case null, default -> errorCode = -1; + if (frame instanceof Http2ResetFrame resetFrame) { + errorCode = resetFrame.errorCode(); + } else if (frame instanceof Http2GoAwayFrame goAwayFrame) { + errorCode = goAwayFrame.errorCode(); + } else { + errorCode = -1; } } }