From d6ac95e4511d31c36b3f938cdb53c2e34c76482b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 12 Jun 2026 13:12:36 +0200 Subject: [PATCH 1/2] Fix ChangeMethodInvocationReturnType changing variable type for nested matches When the matched method invocation appeared nested inside the initializer (e.g. as an argument to another call) rather than as the initializer itself, the recipe still rewrote the declared variable type. For example `Cell c = row.createCell(i, other.getCellType())` would have `Cell` replaced with `CellType`, producing uncompilable code. Only rewrite the declared type when a variable's initializer is itself the matched method invocation. Fixes openrewrite/rewrite-apache#134 --- .../ChangeMethodInvocationReturnType.java | 12 ++++++++- .../ChangeMethodInvocationReturnTypeTest.java | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java index 622e1f791ff..9ef24a007c7 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java @@ -19,6 +19,7 @@ import lombok.Value; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; @@ -77,7 +78,16 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m JavaType.FullyQualified originalType = multiVariable.getTypeAsFullyQualified(); J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx); - if (methodUpdated) { + // Only change the declared type when a variable's initializer is itself the matched + // method invocation. A match nested deeper (e.g. as an argument to another call, such + // as `Cell c = row.createCell(i, other.getCellType())`) must not change the variable type. + boolean initializedByMatch = mv.getVariables().stream().anyMatch(v -> { + Expression initializer = v.getInitializer(); + return initializer instanceof J.MethodInvocation && + methodMatcher.matches((J.MethodInvocation) initializer); + }); + + if (methodUpdated && initializedByMatch) { JavaType newType = JavaType.buildType(newReturnType); JavaType.FullyQualified newFieldType = TypeUtils.asFullyQualified(newType); diff --git a/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java b/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java index a74340a33a4..d807f8331db 100644 --- a/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java +++ b/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java @@ -80,6 +80,33 @@ void bar() { ); } + @Test + void shouldNotChangeVariableTypeWhenMatchIsNestedInInitializer() { + rewriteRun( + //language=java + java( + """ + class Foo { + void bar() { + int direct = Integer.parseInt("1"); + // Integer.parseInt(...) is only an argument here, not the initializer of `s` + String s = String.valueOf(Integer.parseInt("2")); + } + } + """, + """ + class Foo { + void bar() { + long direct = Integer.parseInt("1"); + // Integer.parseInt(...) is only an argument here, not the initializer of `s` + String s = String.valueOf(Integer.parseInt("2")); + } + } + """ + ) + ); + } + @Test void replaceVariableAssignmentFullyQualified() { rewriteRun( From 5a01bdbf90c94bf69c8ec74a72fc4c18876b4375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 12 Jun 2026 14:13:40 +0200 Subject: [PATCH 2/2] Rewrite declared type for parenthesized and ternary initializers Unwrap parentheses and recurse into ternary branches when checking whether a variable is initialized by the matched invocation. Typecast initializers stay excluded: the cast determines the declared type. --- .../ChangeMethodInvocationReturnType.java | 29 +++++- .../ChangeMethodInvocationReturnTypeTest.java | 96 +++++++++++++++++++ 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java index 9ef24a007c7..751f1143117 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangeMethodInvocationReturnType.java @@ -17,6 +17,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.tree.Expression; @@ -81,11 +82,8 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m // Only change the declared type when a variable's initializer is itself the matched // method invocation. A match nested deeper (e.g. as an argument to another call, such // as `Cell c = row.createCell(i, other.getCellType())`) must not change the variable type. - boolean initializedByMatch = mv.getVariables().stream().anyMatch(v -> { - Expression initializer = v.getInitializer(); - return initializer instanceof J.MethodInvocation && - methodMatcher.matches((J.MethodInvocation) initializer); - }); + boolean initializedByMatch = mv.getVariables().stream() + .anyMatch(v -> isInitializedByMatch(v.getInitializer())); if (methodUpdated && initializedByMatch) { JavaType newType = JavaType.buildType(newReturnType); @@ -117,6 +115,27 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return mv; } + + /** + * Returns true when the matched invocation is the direct initializer, is inside + * wrapping parentheses (stripped before checking), or is a branch of a ternary — + * in all of those positions the invocation determines the variable's type. + */ + private boolean isInitializedByMatch(@Nullable Expression expression) { + if (expression == null) { + return false; + } + Expression unwrapped = expression.unwrap(); + if (unwrapped instanceof J.MethodInvocation) { + return methodMatcher.matches((J.MethodInvocation) unwrapped); + } + if (unwrapped instanceof J.Ternary) { + J.Ternary ternary = (J.Ternary) unwrapped; + return isInitializedByMatch(ternary.getTruePart()) || + isInitializedByMatch(ternary.getFalsePart()); + } + return false; + } }; } } diff --git a/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java b/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java index d807f8331db..32cd79b9a9f 100644 --- a/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java +++ b/rewrite-java/src/test/java/org/openrewrite/java/ChangeMethodInvocationReturnTypeTest.java @@ -107,6 +107,102 @@ void bar() { ); } + @Test + void replaceParenthesizedInitializer() { + rewriteRun( + //language=java + java( + """ + class Foo { + void bar() { + int one = (Integer.parseInt("1")); + } + } + """, + """ + class Foo { + void bar() { + long one = (Integer.parseInt("1")); + } + } + """ + ) + ); + } + + @Test + void replaceTernaryInitializerWithMatchInBothBranches() { + rewriteRun( + //language=java + java( + """ + class Foo { + void bar(boolean flag) { + int one = flag ? Integer.parseInt("1") : Integer.parseInt("2"); + } + } + """, + """ + class Foo { + void bar(boolean flag) { + long one = flag ? Integer.parseInt("1") : Integer.parseInt("2"); + } + } + """ + ) + ); + } + + @Test + void replaceTernaryInitializerWithMatchInOneBranch() { + rewriteRun( + //language=java + java( + """ + class Foo { + void bar(boolean flag) { + int one = flag ? (Integer.parseInt("1")) : 0; + } + } + """, + """ + class Foo { + void bar(boolean flag) { + long one = flag ? (Integer.parseInt("1")) : 0; + } + } + """ + ) + ); + } + + @Test + void shouldNotChangeVariableTypeWhenInitializerIsTypeCast() { + rewriteRun( + //language=java + java( + """ + class Foo { + void bar() { + int direct = Integer.parseInt("1"); + // the cast, not the matched invocation, determines the type of `one` + int one = (int) Integer.parseInt("2"); + } + } + """, + """ + class Foo { + void bar() { + long direct = Integer.parseInt("1"); + // the cast, not the matched invocation, determines the type of `one` + int one = (int) Integer.parseInt("2"); + } + } + """ + ) + ); + } + @Test void replaceVariableAssignmentFullyQualified() { rewriteRun(