From b48ed21ae1850458460dc36b6e96fafd7cf4f1a4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 12 May 2026 12:37:38 -0400 Subject: [PATCH 1/4] Use `contextSensitive()` template in `IfElseIfConstructToSwitch` The template references symbols visible from its insertion scope (variables, types, methods), so parsing outside that context can fail on Java 21 pattern-switch syntax and leak `__P__` parameter stubs into the result. --- .../lang/IfElseIfConstructToSwitch.java | 6 +- .../lang/IfElseIfConstructToSwitchTest.java | 142 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) 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..ccf2e056ad 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java @@ -221,7 +221,11 @@ public J.Return visitReturn(J.Return return_, AtomicBoolean atomicBoolean) { } switchBody.append("}\n"); - J.Switch result = JavaTemplate.apply(switchBody.toString(), cursor, if_.getCoordinates().replace(), arguments).withPrefix(if_.getPrefix()); + J.Switch result = JavaTemplate.builder(switchBody.toString()) + .contextSensitive() + .build() + .apply(cursor, if_.getCoordinates().replace(), arguments) + .withPrefix(if_.getPrefix()); return fixTypeAttribution(result); } 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..c702dccf7f 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java @@ -936,6 +936,148 @@ else if (obj instanceof Long l) { } } + @Test + void voidMethodInvocationsWithChainedInstanceOfPatternMatch() { + rewriteRun( + 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; + class Test { + 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 { + 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 unresolvableTypesWithVoidMethodInvocationsAndExistingPatterns() { + 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); + } + } + } + """, + """ + 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) { + switch (obj) { + case Reference reference -> container.add(reference); + case Element element -> container.add(element); + case null, default -> container.add((Qualifier) obj); + } + } + } + """ + ) + ); + } + + @Test + void unresolvableTypesWithVoidMethodInvocationsUnchanged() { + // When types are unresolvable, InstanceOfPatternMatch can't add pattern variables, + // so IfElseIfConstructToSwitch leaves the if-chain unchanged rather than producing + // a broken switch with leaked JavaTemplate placeholders. + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.none()) + .recipes(new InstanceOfPatternMatch(), new IfElseIfConstructToSwitch()), + //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) { + container.add((Reference) obj); + } else if (obj instanceof Element) { + container.add((Element) obj); + } else { + container.add((Qualifier) obj); + } + } + } + """ + ) + ); + } + @Test void unresolvableTypesKeepCaseSpacing() { // https://github.com/openrewrite/rewrite/issues/7458 From 40da3fd2a63529b81ec989166678effaac6cacf2 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 18 May 2026 10:35:51 +0200 Subject: [PATCH 2/4] Require well-formed instanceof types in `IfElseIfConstructToSwitch` Replace the speculative `contextSensitive()` template change with a precondition: skip the conversion when any `instanceof`'s type is missing or unknown. A reporter observed JavaTemplate parameter stubs (`__P__./*__pN__*/voidp()`) leaking into the generated switch when types were unresolved; staying conservative avoids the broken output without guessing at the precise template parser interaction. --- .../lang/IfElseIfConstructToSwitch.java | 12 ++-- .../lang/IfElseIfConstructToSwitchTest.java | 72 ++----------------- 2 files changed, 13 insertions(+), 71 deletions(-) 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 ccf2e056ad..52a78e2066 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitch.java @@ -31,6 +31,7 @@ import org.openrewrite.java.tree.Space; import org.openrewrite.java.tree.Statement; import org.openrewrite.java.tree.TypeTree; +import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.Markers; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; @@ -174,6 +175,11 @@ private boolean validatePotentialCandidate() { })) { return false; } + // Missing type information on the instanceof's type can cause JavaTemplate to mis-parse + // the generated Java 21 switch and leak parameter stubs into the output. + if (patternMatchers.keySet().stream().anyMatch(instanceOf -> !TypeUtils.isWellFormedType(((TypeTree) instanceOf.getClazz()).getType()))) { + return false; + } boolean nullCaseInSwitch = nullCheckedParameter != null && SemanticallyEqual.areEqual(nullCheckedParameter, switchOn.get()); boolean hasLastElseBlock = else_ != null; @@ -221,11 +227,7 @@ public J.Return visitReturn(J.Return return_, AtomicBoolean atomicBoolean) { } switchBody.append("}\n"); - J.Switch result = JavaTemplate.builder(switchBody.toString()) - .contextSensitive() - .build() - .apply(cursor, if_.getCoordinates().replace(), arguments) - .withPrefix(if_.getPrefix()); + J.Switch result = JavaTemplate.apply(switchBody.toString(), cursor, if_.getCoordinates().replace(), arguments).withPrefix(if_.getPrefix()); return fixTypeAttribution(result); } 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 c702dccf7f..38368efeee 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/IfElseIfConstructToSwitchTest.java @@ -1003,7 +1003,9 @@ private void addTo(Container container, Object obj) { } @Test - void unresolvableTypesWithVoidMethodInvocationsAndExistingPatterns() { + 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 @@ -1025,62 +1027,16 @@ private void addTo(Container container, Object obj) { } } } - """, - """ - 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) { - switch (obj) { - case Reference reference -> container.add(reference); - case Element element -> container.add(element); - case null, default -> container.add((Qualifier) obj); - } - } - } - """ - ) - ); - } - - @Test - void unresolvableTypesWithVoidMethodInvocationsUnchanged() { - // When types are unresolvable, InstanceOfPatternMatch can't add pattern variables, - // so IfElseIfConstructToSwitch leaves the if-chain unchanged rather than producing - // a broken switch with leaked JavaTemplate placeholders. - rewriteRun( - spec -> spec.typeValidationOptions(TypeValidation.none()) - .recipes(new InstanceOfPatternMatch(), new IfElseIfConstructToSwitch()), - //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) { - container.add((Reference) obj); - } else if (obj instanceof Element) { - container.add((Element) obj); - } else { - container.add((Qualifier) obj); - } - } - } """ ) ); } @Test - void unresolvableTypesKeepCaseSpacing() { + 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 @@ -1102,22 +1058,6 @@ protected void incrementCounter(Http2Frame frame) { } } } - """, - """ - 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; - switch (frame) { - case Http2ResetFrame resetFrame -> errorCode = resetFrame.errorCode(); - case Http2GoAwayFrame goAwayFrame -> errorCode = goAwayFrame.errorCode(); - case null, default -> errorCode = -1; - } - } - } """ ) ); From 1fed355bb74cec9f168a32d5827d524fd32a3679 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 18 May 2026 10:40:56 +0200 Subject: [PATCH 3/4] Use `NoMissingTypes` precondition instead of inline type check --- .../java/migrate/lang/IfElseIfConstructToSwitch.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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 52a78e2066..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; @@ -31,7 +32,6 @@ import org.openrewrite.java.tree.Space; import org.openrewrite.java.tree.Statement; import org.openrewrite.java.tree.TypeTree; -import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.Markers; import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; @@ -56,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<>()) ); @@ -175,11 +176,6 @@ private boolean validatePotentialCandidate() { })) { return false; } - // Missing type information on the instanceof's type can cause JavaTemplate to mis-parse - // the generated Java 21 switch and leak parameter stubs into the output. - if (patternMatchers.keySet().stream().anyMatch(instanceOf -> !TypeUtils.isWellFormedType(((TypeTree) instanceOf.getClazz()).getType()))) { - return false; - } boolean nullCaseInSwitch = nullCheckedParameter != null && SemanticallyEqual.areEqual(nullCheckedParameter, switchOn.get()); boolean hasLastElseBlock = else_ != null; From 6142b7258cb2dff8cb4ab8d62e811663568efa86 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 18 May 2026 11:04:34 +0200 Subject: [PATCH 4/4] Pass parent cursor when invoking `RemoveAnnotation` visitor `RemoveAnnotationVisitor.maybeRemoveBlankLines` walks up to the enclosing `JavaSourceFile`, but the visitor was invoked without a parent cursor, causing `IllegalStateException: Expected to find enclosing JavaSourceFile` after a recent framework change. --- .../java/migrate/javax/RemoveTemporalAnnotation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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()); } } );