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 21fc0554a2..6a4eb7bcff 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArguments.java @@ -79,14 +79,19 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex return md; } + // Only act when at least one argument actually does work worth surfacing. This is the + // cheapest decisive check, so make it first and bail before any name generation, scope + // scanning, or templating that an unaffected constructor would not need. + if (args.stream().noneMatch(arg -> arg instanceof J.MethodInvocation || arg instanceof J.NewClass)) { + return md; + } + // Plan the extraction: hoist everything but trivial, side-effect-free arguments, in order. - // Only act when at least one extracted argument actually does work worth surfacing. Set usedNames = new LinkedHashSet<>(); String[] names = new String[args.size()]; Set imports = new LinkedHashSet<>(); StringBuilder declarations = new StringBuilder(); List declarationArgs = new ArrayList<>(); - boolean anyComplex = false; for (int i = 0; i < args.size(); i++) { Expression arg = args.get(i); if (isInlineSafe(arg)) { @@ -105,14 +110,10 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex if (paramType instanceof JavaType.FullyQualified) { imports.add(((JavaType.FullyQualified) paramType).getFullyQualifiedName()); } - anyComplex |= arg instanceof J.MethodInvocation || arg instanceof J.NewClass; - } - if (!anyComplex) { - return md; } // 1. Declare the extracted arguments right before the constructor invocation, preserving their order - JavaTemplate.Builder declarationTemplate = JavaTemplate.builder(declarations.toString()).contextSensitive(); + JavaTemplate.Builder declarationTemplate = JavaTemplate.builder(declarations.toString()); for (String fqn : imports) { declarationTemplate.imports(fqn); maybeAddImport(fqn); @@ -146,8 +147,7 @@ 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.builder(argumentList.toString()).contextSensitive().build() - .apply(getCursor(), mi.getCoordinates().replaceArguments(), inlineArgs.toArray()); + return JavaTemplate.apply(argumentList.toString(), getCursor(), mi.getCoordinates().replaceArguments(), inlineArgs.toArray()); } return mi; } 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 b0d8175d5d..a12e5341fa 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/ExtractExplicitConstructorInvocationArgumentsTest.java @@ -18,9 +18,17 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledForJreRange; import org.openrewrite.DocumentExample; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.condition.JRE.JAVA_25; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.javaVersion; @@ -499,6 +507,162 @@ class Child extends Parent { ); } + @Test + void extractWithSourcePathParameterType() { + rewriteRun( + //language=java + java( + """ + package foo; + public class Widget { + } + """ + ), + //language=java + java( + """ + package foo; + + class Parent { + Parent(Widget w) { + } + } + + class Child extends Parent { + Child(Widget w) { + super(identity(w)); + } + + static Widget identity(Widget w) { + return w; + } + } + """, + """ + package foo; + + class Parent { + Parent(Widget w) { + } + } + + class Child extends Parent { + Child(Widget w) { + Widget w1 = identity(w); + super(w1); + } + + static Widget identity(Widget w) { + return w; + } + } + """, + // Even with a context-free template, the rewritten reference must carry both its type and + // its binding to the freshly created local; otherwise downstream type-aware recipes break. + spec -> spec.afterRecipe(cu -> { + List refs = new ArrayList<>(); + new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, Integer p) { + if ("super".equals(mi.getSimpleName())) { + for (Expression a : mi.getArguments()) { + if (a instanceof J.Identifier) { + refs.add((J.Identifier) a); + } + } + } + return super.visitMethodInvocation(mi, p); + } + }.visit(cu, 0); + assertThat(refs).hasSize(1); + assertThat(refs.get(0).getType()).as("type").isNotNull(); + assertThat(refs.get(0).getFieldType()).as("fieldType (variable binding)").isNotNull(); + }) + ) + ); + } + + @Test + void extractWithSourcePathWideningSupertype() { + rewriteRun( + //language=java + java( + """ + package foo; + public class Base { + } + """ + ), + //language=java + java( + """ + package foo; + public class Derived extends Base { + } + """ + ), + //language=java + java( + """ + package foo; + + class Parent { + Parent(Base b) { + } + } + + class Child extends Parent { + Child() { + super(makeDerived()); + } + + static Derived makeDerived() { + return new Derived(); + } + } + """, + """ + package foo; + + class Parent { + Parent(Base b) { + } + } + + class Child extends Parent { + Child() { + Base b = makeDerived(); + super(b); + } + + static Derived makeDerived() { + return new Derived(); + } + } + """, + // The extracted variable is declared with a widening supertype ('Base') that lives in a sibling + // source and is referenced nowhere else in this unit. A context-free template still resolves it + // via the shared type cache, so the declared type must be a real FQ type, not Unknown. + spec -> spec.afterRecipe(cu -> { + List decls = new ArrayList<>(); + new JavaIsoVisitor() { + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations vd, Integer p) { + if ("b".equals(vd.getVariables().get(0).getSimpleName())) { + decls.add(vd); + } + return super.visitVariableDeclarations(vd, p); + } + }.visit(cu, 0); + assertThat(decls).hasSize(1); + assertThat(decls.get(0).getTypeExpression().getType()).as("declared type 'Base'").isNotNull(); + assertThat(TypeUtils.asFullyQualified(decls.get(0).getTypeExpression().getType())) + .as("declared type is a resolved FQ type, not Unknown").isNotNull(); + }) + ) + ); + } + @Test void extractThisDelegationArgument() { rewriteRun(