diff --git a/src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java b/src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java index 6a4eb7bcff..3556b7a290 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java @@ -17,6 +17,7 @@ import lombok.Getter; import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; @@ -112,16 +113,6 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex } } - // 1. Declare the extracted arguments right before the constructor invocation, preserving their order - JavaTemplate.Builder declarationTemplate = JavaTemplate.builder(declarations.toString()); - for (String fqn : imports) { - declarationTemplate.imports(fqn); - maybeAddImport(fqn); - } - md = declarationTemplate.build() - .apply(getCursor(), superCall.getCoordinates().before(), declarationArgs.toArray()); - - // 2. Replace the now-redundant arguments with references to the new local variables StringBuilder argumentList = new StringBuilder(); List inlineArgs = new ArrayList<>(); for (int i = 0; i < args.size(); i++) { @@ -135,7 +126,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex inlineArgs.add(args.get(i)); } } - return (J.MethodDeclaration) new JavaIsoVisitor() { + md = (J.MethodDeclaration) new JavaIsoVisitor() { @Override public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx2) { // Do not descend into nested/local classes; their own `super(..)`/`this(..)` calls @@ -147,11 +138,25 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx2) { mi = super.visitMethodInvocation(mi, ctx2); if (isExplicitConstructorInvocation(mi)) { - return JavaTemplate.apply(argumentList.toString(), getCursor(), mi.getCoordinates().replaceArguments(), inlineArgs.toArray()); + return JavaTemplate.apply(argumentList.toString(), getCursor(), + mi.getCoordinates().replaceArguments(), inlineArgs.toArray()); } return mi; } }.visitNonNull(md, ctx, getCursor().getParentOrThrow()); + + J.MethodInvocation updatedSuperCall = findExplicitConstructorInvocation(md.getBody().getStatements()); + if (updatedSuperCall == null) { + return md; + } + JavaTemplate.Builder declarationTemplate = JavaTemplate.builder(declarations.toString()); + for (String fqn : imports) { + declarationTemplate.imports(fqn); + maybeAddImport(fqn); + } + return declarationTemplate.build().apply( + new Cursor(getCursor().getParentOrThrow(), md), + updatedSuperCall.getCoordinates().before(), declarationArgs.toArray()); } private J.@Nullable MethodInvocation findExplicitConstructorInvocation(List statements) { diff --git a/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsJava25Test.java b/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsJava25Test.java new file mode 100644 index 0000000000..8a15ac8666 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsJava25Test.java @@ -0,0 +1,76 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.migrate.lang; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledForJreRange; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.junit.jupiter.api.condition.JRE.JAVA_25; +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.javaVersion; + +@EnabledForJreRange(min = JAVA_25) +class ExtractExplicitConstructorInvocationArgumentsJava25Test implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec + .allSources(src -> src.markers(javaVersion(25))) + .recipe(new ExtractExplicitConstructorInvocationArguments()); + } + + @Test + void extractsSymphonyConstructorArgumentsBeforeSuperInvocation() { + rewriteRun( + java( + """ + import java.time.Clock; + import java.util.Objects; + + class Parent { + Parent(Clock clock, String config) { + } + } + + class Child extends Parent { + Child(Clock clock, String config) { + super(Objects.requireNonNull(clock), Objects.requireNonNull(config)); + } + } + """, + """ + import java.time.Clock; + import java.util.Objects; + + class Parent { + Parent(Clock clock, String config) { + } + } + + class Child extends Parent { + Child(Clock clock, String config) { + Clock clock1 = Objects.requireNonNull(clock); + String config1 = Objects.requireNonNull(config); + super(clock1, config1); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java index 00d51c26cd..3f6baddaeb 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java @@ -242,6 +242,47 @@ class Child extends Parent { ); } + @Test + void preserveInvocationAndArgumentCommentsOnce() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + + class Parent { + Parent(String first, String second) { + } + } + + class Child extends Parent { + Child(String value) { + // Explain the delegation. + super(/* validate first */ Objects.requireNonNull(value), Objects.requireNonNull(value)); + } + } + """, + """ + import java.util.Objects; + + class Parent { + Parent(String first, String second) { + } + } + + class Child extends Parent { + Child(String value) { + // Explain the delegation. + String first = /* validate first */ Objects.requireNonNull(value); + String second = Objects.requireNonNull(value); + super(first, second); + } + } + """ + ) + ); + } + @Test void leaveTrivialArgumentsInlineWhenExtractingSiblings() { rewriteRun( @@ -647,7 +688,8 @@ static Derived makeDerived() { new JavaIsoVisitor() { @Override public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations vd, Integer p) { - if ("b".equals(vd.getVariables().getFirst().getSimpleName())) { + if ("b".equals(vd.getVariables().getFirst().getSimpleName()) && + vd.getVariables().getFirst().getInitializer() != null) { decls.add(vd); } return super.visitVariableDeclarations(vd, p);