From 69cbb3a1e998d154c8b50f5d925cc6e0388473dd Mon Sep 17 00:00:00 2001 From: rkampani Date: Mon, 19 May 2025 16:41:14 -0500 Subject: [PATCH 1/5] Move statements before super(..) in Java constructor #733 --- .../java/migrate/util/RelocateSuperCall.java | 68 ++++++ .../migrate/util/RelocateSuperCallTest.java | 203 ++++++++++++++++++ 2 files changed, 271 insertions(+) create mode 100644 src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java create mode 100644 src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java b/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java new file mode 100644 index 0000000000..8bb2ec7f0b --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java @@ -0,0 +1,68 @@ +package org.openrewrite.java.migrate.util; + + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Statement; + +import java.util.List; + +@EqualsAndHashCode(callSuper = false) +@Value +public class RelocateSuperCall extends Recipe { + + @Override + public String getDisplayName() { + return "Move `super()` after conditionals (Java 25+)"; + } + + @Override + public String getDescription() { + return "Relocates `super()` calls to take advantage of the early construction context introduced by JEP 513 in Java 25+, allowing statements before constructor calls."; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check( + new UsesJavaVersion<>(25), + new RelocateSuperCallVisitor()); + } + + private static class RelocateSuperCallVisitor extends JavaIsoVisitor { + + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { + if (!method.isConstructor() || method.getBody() == null) { + return method; + } + + List statements = method.getBody().getStatements(); + if (statements.size() < 2) { + return method; + } + + Statement first = statements.get(0); + if (!(first instanceof J.MethodInvocation)) { + return method; + } + J.MethodInvocation methodInvocation = (J.MethodInvocation) first; + if (!"super".equals(methodInvocation.getSimpleName())) { + return method; + } + + // Move super() to the end + List updated = new java.util.ArrayList<>(statements); + updated.remove(0); + updated.add(methodInvocation); + + return method.withBody(method.getBody().withStatements(updated)); + } + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java b/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java new file mode 100644 index 0000000000..20eb71e08e --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java @@ -0,0 +1,203 @@ +package org.openrewrite.java.migrate.util; + +import org.junit.jupiter.api.Test; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.javaVersion; + +class RelocateSuperCallTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new RelocateSuperCall()) + .allSources(src -> src.markers(javaVersion(25))); + + } + @Test + void relocateSuperAfterIf() { + rewriteRun( + java( + """ + class A { + public A(String bar) { + super(); + if(bar.equals("test")) + throw new RuntimeException(); + } + } + """, + """ + class A { + public A(String bar) { + if(bar.equals("test")) + throw new RuntimeException(); + super(); + } + } + """ + ) + ); + } + + @Test + void relocateSuperAfterIfStatement() { + rewriteRun( + java( + // language=java + """ + class Person { + final int age; + public Person(int age) { + if (age < 0) { + throw new IllegalArgumentException("Invalid age"); + } + this.age = age; + } + } + + class Employee extends Person { + public Employee(int age) { + super(age); + if (age < 18 || age > 67) { + throw new IllegalArgumentException("Invalid employee age"); + } + } + } + """, + // Expected output + """ + class Person { + final int age; + public Person(int age) { + if (age < 0) { + throw new IllegalArgumentException("Invalid age"); + } + this.age = age; + } + } + + class Employee extends Person { + public Employee(int age) { + if (age < 18 || age > 67) { + throw new IllegalArgumentException("Invalid employee age"); + } + super(age); + } + } + """ + ) + ); + } + + @Test + void relocateSuperWithSafeFieldAssignmentOnly() { + rewriteRun( + java( + """ + class Outer { + class Inner { + int x; + int y = 100; + + Inner(int input) { + super(); + x = input; + y = 200; + } + } + } + """, + // Note: `y = 200` is illegal under early construction context since `y` has initializer + // So the expected result is same as input if `super()` is already last. + """ + class Outer { + class Inner { + int x; + int y = 100; + + Inner(int input) { + x = input; + y = 200; + super(); + } + } + } + """ + ) + ); + } + + @Test + void nprelocateSuperInInnerClassIfAlreadyAsLast_withSafeAssignments() { + rewriteRun( + java( + // language=java (before transformation) + """ + class Outer { + class Inner { + int x; + int y; + + Inner(int input) { + var tmp = input * 2; + x = tmp; + y = 42; + super(); + } + } + } + """, + // // No Change expected as super() valid with JDK25 version + spec -> spec.markers(javaVersion(25)) + ) + ); + } + + @Test + void noRelocateSuperAfterIf_givenBelowJDK25Version() { + rewriteRun( + java( + // Input + """ + class A { + public A(String bar) { + super(); + if(bar.equals("test")) + throw new RuntimeException(); + } + } + """, + // Simulate Java 8 environment + spec -> spec.markers(javaVersion(8)) + ) + ); + } + + @Test + void relocateSuperInInnerClass_withSafeAssignments() { + rewriteRun( + java( + // language=java (before transformation) + """ + class Outer { + class Inner { + int x; + int y; + + Inner(int input) { + super(); + var tmp = input * 2; + x = tmp; + y = 42; + } + } + } + """, + spec -> spec.markers(javaVersion(8)) + ) + ); + } +} + From 9b9aed64bc854c373c05e7a7332591a37872c9d2 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 20 May 2025 00:00:02 +0200 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/migrate/util/RelocateSuperCall.java | 15 +++++++++++++++ .../migrate/util/RelocateSuperCallTest.java | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java b/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java index 8bb2ec7f0b..528b2ca314 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java +++ b/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java @@ -1,3 +1,18 @@ +/* + * Copyright 2025 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.util; diff --git a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java b/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java index 20eb71e08e..161677faf6 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java @@ -1,5 +1,22 @@ +/* + * Copyright 2025 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.util; +import org.openrewrite.DocumentExample; + import org.junit.jupiter.api.Test; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -15,6 +32,8 @@ public void defaults(RecipeSpec spec) { .allSources(src -> src.markers(javaVersion(25))); } + + @DocumentExample @Test void relocateSuperAfterIf() { rewriteRun( From 7f18dbe4aac031d6efb8a764efc860fad6b6c3bb Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 20 May 2025 00:07:39 +0200 Subject: [PATCH 3/5] Update RelocateSuperCallTest.java --- .../openrewrite/java/migrate/util/RelocateSuperCallTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java b/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java index 161677faf6..da81ae5309 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java @@ -15,9 +15,8 @@ */ package org.openrewrite.java.migrate.util; -import org.openrewrite.DocumentExample; - import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -219,4 +218,3 @@ class Inner { ); } } - From d69ad7dc1faaf974861bbb223bd06b42bdd17d59 Mon Sep 17 00:00:00 2001 From: rkampani Date: Mon, 19 May 2025 20:46:47 -0500 Subject: [PATCH 4/5] Moved the file out of the util package and updated the JDK 25 rewrite YAML configuration to reflect the change --- .../openrewrite/java/migrate/{util => }/RelocateSuperCall.java | 2 +- src/main/resources/META-INF/rewrite/java-version-25.yml | 1 + .../java/migrate/{util => }/RelocateSuperCallTest.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) rename src/main/java/org/openrewrite/java/migrate/{util => }/RelocateSuperCall.java (98%) rename src/test/java/org/openrewrite/java/migrate/{util => }/RelocateSuperCallTest.java (99%) diff --git a/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java b/src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java similarity index 98% rename from src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java rename to src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java index 528b2ca314..083fdde2b1 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/RelocateSuperCall.java +++ b/src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.util; +package org.openrewrite.java.migrate; import lombok.EqualsAndHashCode; diff --git a/src/main/resources/META-INF/rewrite/java-version-25.yml b/src/main/resources/META-INF/rewrite/java-version-25.yml index 961cc0668f..0d4727ab47 100644 --- a/src/main/resources/META-INF/rewrite/java-version-25.yml +++ b/src/main/resources/META-INF/rewrite/java-version-25.yml @@ -29,6 +29,7 @@ recipeList: - org.openrewrite.java.migrate.RemoveSecurityPolicy - org.openrewrite.java.migrate.RemoveSecurityManager - org.openrewrite.java.migrate.SystemGetSecurityManagerToNull + - org.openrewrite.java.migrate.RelocateSuperCall --- type: specs.openrewrite.org/v1beta/recipe diff --git a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java b/src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java similarity index 99% rename from src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java rename to src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java index da81ae5309..66ec77bdfa 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/RelocateSuperCallTest.java +++ b/src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.util; +package org.openrewrite.java.migrate; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; From b2161f39a6bf1f38f6e755c14ac7997bb0d30364 Mon Sep 17 00:00:00 2001 From: rkampani Date: Tue, 2 Sep 2025 20:21:15 -0500 Subject: [PATCH 5/5] updated with early construction contexts --- .../java/migrate/RelocateSuperCall.java | 124 ++++++++++++- .../java/migrate/RelocateSuperCallTest.java | 175 ++++++++---------- 2 files changed, 197 insertions(+), 102 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java b/src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java index 083fdde2b1..69f8fe84ea 100644 --- a/src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java +++ b/src/main/java/org/openrewrite/java/migrate/RelocateSuperCall.java @@ -63,21 +63,129 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex return method; } - Statement first = statements.get(0); - if (!(first instanceof J.MethodInvocation)) { - return method; + + int superCallIdx = -1; + for (int i = 0; i < statements.size(); i++) { + Statement stmt = statements.get(i); + + // Check if this statement contains a super() call + if (stmt.printTrimmed(getCursor()).startsWith("super(")) { + superCallIdx = i; + break; + } else if (stmt instanceof J.MethodInvocation) { + J.MethodInvocation mi = (J.MethodInvocation) stmt; + if ("super".equals(mi.getSimpleName())) { + superCallIdx = i; + break; + } + } } - J.MethodInvocation methodInvocation = (J.MethodInvocation) first; - if (!"super".equals(methodInvocation.getSimpleName())) { + // Move super() to the end + if (superCallIdx == -1) { return method; } - // Move super() to the end + // Check for forbidden usages before super() + for (int i = 0; i < superCallIdx; i++) { + Statement stmt = statements.get(i); + // Example checks (expand as needed): + if (stmt instanceof J.MethodInvocation) { + J.MethodInvocation mi = (J.MethodInvocation) stmt; + if (mi.getSelect() != null) { + String select = mi.getSelect().printTrimmed(); + if ("this".equals(select) || "super".equals(select)) { + // Log or comment: forbidden usage before super() + // ctx.getOnError().accept(...); + } + } + } + if (stmt instanceof J.Assignment) { + J.Assignment assign = (J.Assignment) stmt; + // Check assignment to initialized fields (requires symbol table) + // For now, just log assignment + } + // Add more checks for field accesses, etc. + } + + // Find optimal position for super() call + int optimalPosition = findOptimalSuperPosition(statements, superCallIdx); + + if (optimalPosition == superCallIdx) { + return method; // No change needed + } + List updated = new java.util.ArrayList<>(statements); - updated.remove(0); - updated.add(methodInvocation); + Statement superCall = updated.remove(superCallIdx); + + // Adjust insertion position if we removed an element before it + int insertPos = optimalPosition > superCallIdx ? optimalPosition - 1 : optimalPosition; + updated.add(insertPos, superCall); return method.withBody(method.getBody().withStatements(updated)); + } + + private int findOptimalSuperPosition(List statements, int currentSuperIdx) { + // Find the latest position where super() can be safely placed + int optimalPosition = 0; // Start at the beginning + + for (int i = 0; i < statements.size(); i++) { + if (i == currentSuperIdx) continue; // Skip the current super() call + + Statement stmt = statements.get(i); + + // Check for invalid field reads (this.field access that's not assignment) + if (containsThisFieldAccess(stmt)) { + // Found an invalid operation - super() should be before this + return optimalPosition; + } + + // If it's a safe operation (assignment, conditionals, etc.), we can move past it + if (isSafeBeforeSuper(stmt)) { + optimalPosition = i + 1; + } + } + + // If no invalid operations found, super() can go at the end + return optimalPosition; + } + + private boolean containsThisFieldAccess(Statement stmt) { + // Check if statement contains this.field read (not assignment) + String stmtStr = stmt.printTrimmed(); + + if (stmtStr.contains("this.")) { + // If it's an assignment to this.field, it's safe + if (stmtStr.trim().startsWith("this.") && stmtStr.contains(" = ")) { + return false; + } + // Any other this.field usage is invalid before super() + return true; + } + return false; + } + + private boolean isSafeBeforeSuper(Statement stmt) { + // Safe operations that can happen before super() in Java 25+ + if (stmt instanceof J.Assignment) { + J.Assignment assign = (J.Assignment) stmt; + // Check if it's an assignment to this.field + if (assign.getVariable() instanceof J.FieldAccess) { + J.FieldAccess fa = (J.FieldAccess) assign.getVariable(); + if (fa.getTarget() instanceof J.Identifier && + "this".equals(((J.Identifier) fa.getTarget()).getSimpleName())) { + return true; // this.field = value is safe + } + } + } + + // Conditionals and variable declarations are generally safe + if (stmt instanceof J.If || stmt instanceof J.VariableDeclarations) { + return true; + } + + return false; + } + } } diff --git a/src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java b/src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java index 66ec77bdfa..4c89a5167d 100644 --- a/src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java +++ b/src/test/java/org/openrewrite/java/migrate/RelocateSuperCallTest.java @@ -38,20 +38,36 @@ void relocateSuperAfterIf() { rewriteRun( java( """ - class A { - public A(String bar) { - super(); - if(bar.equals("test")) - throw new RuntimeException(); + class Person { + int age; + Person(int age) { + if (age < 0) + throw new IllegalArgumentException("Age cannot be negative"); + this.age = age; + } + } + class Employee extends Person { + Employee(int age) { + super(age); + if (age < 18 || age > 67) + throw new IllegalArgumentException("Age must be between 18 and 67"); } } """, """ - class A { - public A(String bar) { - if(bar.equals("test")) - throw new RuntimeException(); - super(); + class Person { + int age; + Person(int age) { + if (age < 0) + throw new IllegalArgumentException("Age cannot be negative"); + this.age = age; + } + } + class Employee extends Person { + Employee(int age) { + if (age < 18 || age > 67) + throw new IllegalArgumentException("Age must be between 18 and 67"); + super(age); } } """ @@ -63,44 +79,48 @@ public A(String bar) { void relocateSuperAfterIfStatement() { rewriteRun( java( - // language=java """ class Person { - final int age; - public Person(int age) { - if (age < 0) { - throw new IllegalArgumentException("Invalid age"); - } + int age; + Person(int age) { + if (age < 0) + throw new IllegalArgumentException("Age cannot be negative"); this.age = age; } } - class Employee extends Person { - public Employee(int age) { + String officeID; + String department; + Employee(int age, String officeID, String department) { super(age); - if (age < 18 || age > 67) { - throw new IllegalArgumentException("Invalid employee age"); - } + if (age < 18 || age > 67) + throw new IllegalArgumentException("Age must be between 18 and 67"); + if (department == null) + throw new IllegalArgumentException("Department cannot be null"); + this.officeID = officeID; + this.department = department; } } """, - // Expected output """ class Person { - final int age; - public Person(int age) { - if (age < 0) { - throw new IllegalArgumentException("Invalid age"); - } + int age; + Person(int age) { + if (age < 0) + throw new IllegalArgumentException("Age cannot be negative"); this.age = age; } } - class Employee extends Person { - public Employee(int age) { - if (age < 18 || age > 67) { - throw new IllegalArgumentException("Invalid employee age"); - } + String officeID; + String department; + Employee(int age, String officeID, String department) { + if (age < 18 || age > 67) + throw new IllegalArgumentException("Age must be between 18 and 67"); + if (department == null) + throw new IllegalArgumentException("Department cannot be null"); + this.officeID = officeID; + this.department = department; super(age); } } @@ -114,85 +134,52 @@ void relocateSuperWithSafeFieldAssignmentOnly() { rewriteRun( java( """ + class Person { + int age; + Person(int age) { + if (age < 0) + throw new IllegalArgumentException("Age cannot be negative"); + this.age = age; + } + } class Outer { - class Inner { - int x; - int y = 100; - - Inner(int input) { - super(); - x = input; - y = 200; + int i; + void hello() { System.out.println("Hello"); } + class Inner extends Person { + int j; + Inner(int age, int j) { + super(age); + this.j = j; } } } """, - // Note: `y = 200` is illegal under early construction context since `y` has initializer - // So the expected result is same as input if `super()` is already last. """ - class Outer { - class Inner { - int x; - int y = 100; - - Inner(int input) { - x = input; - y = 200; - super(); - } + class Person { + int age; + Person(int age) { + if (age < 0) + throw new IllegalArgumentException("Age cannot be negative"); + this.age = age; } } - """ - ) - ); - } - - @Test - void nprelocateSuperInInnerClassIfAlreadyAsLast_withSafeAssignments() { - rewriteRun( - java( - // language=java (before transformation) - """ class Outer { - class Inner { - int x; - int y; - - Inner(int input) { - var tmp = input * 2; - x = tmp; - y = 42; - super(); + int i; + void hello() { System.out.println("Hello"); } + class Inner extends Person { + int j; + Inner(int age, int j) { + this.j = j; + super(age); } } } - """, - // // No Change expected as super() valid with JDK25 version - spec -> spec.markers(javaVersion(25)) - ) - ); - } - - @Test - void noRelocateSuperAfterIf_givenBelowJDK25Version() { - rewriteRun( - java( - // Input """ - class A { - public A(String bar) { - super(); - if(bar.equals("test")) - throw new RuntimeException(); - } - } - """, - // Simulate Java 8 environment - spec -> spec.markers(javaVersion(8)) ) ); } + @Test void relocateSuperInInnerClass_withSafeAssignments() { rewriteRun(