From a6a91610002f34d373d6c46bb03845164b5a54c2 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 9 Sep 2025 14:35:44 -0400 Subject: [PATCH 01/10] Add MoveFieldsToTopOfClass --- .../MoveFieldsToTopOfClass.java | 138 +++++ .../META-INF/rewrite/static-analysis.yml | 1 + .../MoveFieldsToTopOfClassTest.java | 482 ++++++++++++++++++ 3 files changed, 621 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java new file mode 100644 index 0000000000..62b52f38b4 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -0,0 +1,138 @@ +/* + * 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.staticanalysis; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; + +@EqualsAndHashCode(callSuper = false) +@Value +public class MoveFieldsToTopOfClass extends Recipe { + + @Override + public String getDisplayName() { + return "Move fields to the top of class definition"; + } + + @Override + public String getDescription() { + return "Reorders class members so that all field declarations appear before any method declarations, " + + "constructors, or other class members. This improves code organization and readability by " + + "grouping field declarations together at the top of the class. Comments associated with fields " + + "are preserved during the reordering."; + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(2); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaIsoVisitor() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); + + List statements = cd.getBody().getStatements(); + if (statements.isEmpty()) { + return cd; + } + + List fieldDeclarations = new ArrayList<>(); + List otherStatements = new ArrayList<>(); + + // Separate field declarations from other statements + for (Statement statement : statements) { + if (statement instanceof J.VariableDeclarations) { + fieldDeclarations.add(statement); + } else { + otherStatements.add(statement); + } + } + + // If all fields are already at the top, no changes needed + if (fieldDeclarations.isEmpty() || isAlreadyOrdered(statements, fieldDeclarations)) { + return cd; + } + + // Reorder: fields first, then other statements + List reorderedStatements = new ArrayList<>(); + + // Add field declarations with preserved comments and proper spacing + for (int i = 0; i < fieldDeclarations.size(); i++) { + Statement field = fieldDeclarations.get(i); + if (i == 0) { + // First field should have the same prefix as the first statement originally had, + // but preserve its original comments + Space originalPrefix = field.getPrefix(); + Space firstStatementPrefix = statements.get(0).getPrefix(); + + // Combine: use first statement's whitespace but preserve field's comments + Space newPrefix = firstStatementPrefix.withComments(originalPrefix.getComments()); + field = field.withPrefix(newPrefix); + } + reorderedStatements.add(field); + } + + // Add other statements + reorderedStatements.addAll(otherStatements); + + return autoFormat(cd.withBody(cd.getBody().withStatements(reorderedStatements)), ctx); + } + + private boolean isAlreadyOrdered(List allStatements, List fieldDeclarations) { + if (fieldDeclarations.isEmpty()) { + return true; + } + + int firstNonFieldIndex = -1; + for (int i = 0; i < allStatements.size(); i++) { + if (!(allStatements.get(i) instanceof J.VariableDeclarations)) { + firstNonFieldIndex = i; + break; + } + } + + // If there are no non-field statements, or all fields come before first non-field + if (firstNonFieldIndex == -1) { + return true; + } + + // Check if any field declarations come after the first non-field statement + for (int i = firstNonFieldIndex; i < allStatements.size(); i++) { + if (allStatements.get(i) instanceof J.VariableDeclarations) { + return false; + } + } + + return true; + } + }; + } +} diff --git a/src/main/resources/META-INF/rewrite/static-analysis.yml b/src/main/resources/META-INF/rewrite/static-analysis.yml index c38c1d0036..ff391c174a 100644 --- a/src/main/resources/META-INF/rewrite/static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/static-analysis.yml @@ -25,6 +25,7 @@ recipeList: - org.openrewrite.java.format.EmptyNewlineAtEndOfFile - org.openrewrite.staticanalysis.ForLoopControlVariablePostfixOperators - org.openrewrite.staticanalysis.FinalizePrivateFields + - org.openrewrite.staticanalysis.MoveFieldsToTopOfClass - org.openrewrite.java.format.MethodParamPad - org.openrewrite.java.format.NoWhitespaceAfter - org.openrewrite.java.format.NoWhitespaceBefore diff --git a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java new file mode 100644 index 0000000000..13877e440b --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java @@ -0,0 +1,482 @@ +/* + * 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.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class MoveFieldsToTopOfClassTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new MoveFieldsToTopOfClass()); + } + + @DocumentExample + @Test + void moveFieldsToTop() { + rewriteRun( + //language=java + java( + """ + class A { + void foo(String bar) { + int i = Integer.parseInt(bar); + } + + // Important field + String myField = "important"; + } + """, + """ + class A { + // Important field + String myField = "important"; + + void foo(String bar) { + int i = Integer.parseInt(bar); + } + } + """ + ) + ); + } + + @Test + void moveMultipleFields() { + rewriteRun( + //language=java + java( + """ + class Example { + public void method1() { + System.out.println("method1"); + } + + private String field1 = "value1"; + + public Example() { + // constructor + } + + protected int field2 = 42; + + public void method2() { + System.out.println("method2"); + } + + public static final String CONSTANT = "constant"; + } + """, + """ + class Example { + private String field1 = "value1"; + + protected int field2 = 42; + + public static final String CONSTANT = "constant"; + + public void method1() { + System.out.println("method1"); + } + + public Example() { + // constructor + } + + public void method2() { + System.out.println("method2"); + } + } + """ + ) + ); + } + + @Test + void fieldsAlreadyAtTop() { + rewriteRun( + //language=java + java( + """ + class AlreadyOrdered { + private String field1 = "value1"; + protected int field2 = 42; + public static final String CONSTANT = "constant"; + + public void method() { + System.out.println("method"); + } + + public AlreadyOrdered() { + // constructor + } + } + """ + ) + ); + } + + @Test + void onlyFieldsInClass() { + rewriteRun( + //language=java + java( + """ + class OnlyFields { + private String field1 = "value1"; + protected int field2 = 42; + public static final String CONSTANT = "constant"; + } + """ + ) + ); + } + + @Test + void onlyMethodsInClass() { + rewriteRun( + //language=java + java( + """ + class OnlyMethods { + public void method1() { + System.out.println("method1"); + } + + public OnlyMethods() { + // constructor + } + + public void method2() { + System.out.println("method2"); + } + } + """ + ) + ); + } + + @Test + void emptyClass() { + rewriteRun( + //language=java + java( + """ + class Empty { + } + """ + ) + ); + } + + @Test + void fieldsBetweenMethods() { + rewriteRun( + //language=java + java( + """ + class Mixed { + public void firstMethod() { + System.out.println("first"); + } + + private String field1 = "value1"; + + public void secondMethod() { + System.out.println("second"); + } + + private int field2 = 10; + + public void thirdMethod() { + System.out.println("third"); + } + } + """, + """ + class Mixed { + private String field1 = "value1"; + + private int field2 = 10; + + public void firstMethod() { + System.out.println("first"); + } + + public void secondMethod() { + System.out.println("second"); + } + + public void thirdMethod() { + System.out.println("third"); + } + } + """ + ) + ); + } + + @Test + void staticAndInstanceFields() { + rewriteRun( + //language=java + java( + """ + class StaticAndInstance { + public void method() { + System.out.println("method"); + } + + private static final String STATIC_FIELD = "static"; + private String instanceField = "instance"; + public final int publicField = 100; + + public StaticAndInstance() { + // constructor + } + } + """, + """ + class StaticAndInstance { + private static final String STATIC_FIELD = "static"; + private String instanceField = "instance"; + public final int publicField = 100; + + public void method() { + System.out.println("method"); + } + + public StaticAndInstance() { + // constructor + } + } + """ + ) + ); + } + + @Test + void innerClassesAndFields() { + rewriteRun( + //language=java + java( + """ + class OuterClass { + public void outerMethod() { + System.out.println("outer method"); + } + + private String outerField = "outer"; + + class InnerClass { + public void innerMethod() { + System.out.println("inner method"); + } + + private String innerField = "inner"; + } + + protected int anotherOuterField = 42; + } + """, + """ + class OuterClass { + private String outerField = "outer"; + + protected int anotherOuterField = 42; + + public void outerMethod() { + System.out.println("outer method"); + } + + class InnerClass { + private String innerField = "inner"; + + public void innerMethod() { + System.out.println("inner method"); + } + } + } + """ + ) + ); + } + + @Test + void preserveCommentsWithFields() { + rewriteRun( + //language=java + java( + """ + class A { + void foo(String bar) { + int i = Integer.parseInt(bar); + } + + // this is my field, not yours + String myField = "important"; + } + """, + """ + class A { + // this is my field, not yours + String myField = "important"; + + void foo(String bar) { + int i = Integer.parseInt(bar); + } + } + """ + ) + ); + } + + @Test + void preserveMultilineCommentsWithFields() { + rewriteRun( + //language=java + java( + """ + class A { + public void method() { + System.out.println("method"); + } + + /* + * This is a multiline comment + * for my field + */ + private String field1 = "value1"; + + /** + * Javadoc comment for field2 + * @deprecated this field is old + */ + @Deprecated + private int field2 = 42; + } + """, + """ + class A { + /* + * This is a multiline comment + * for my field + */ + private String field1 = "value1"; + + /** + * Javadoc comment for field2 + * @deprecated this field is old + */ + @Deprecated + private int field2 = 42; + + public void method() { + System.out.println("method"); + } + } + """ + ) + ); + } + + @Test + void preserveCommentsWithMixedMembers() { + rewriteRun( + //language=java + java( + """ + class Mixed { + // Method comment + public void firstMethod() { + System.out.println("first"); + } + + // Field comment 1 + private String field1 = "value1"; + + /* Another method comment */ + public void secondMethod() { + System.out.println("second"); + } + + // Field comment 2 + private int field2 = 10; + } + """, + """ + class Mixed { + // Field comment 1 + private String field1 = "value1"; + + // Field comment 2 + private int field2 = 10; + + // Method comment + public void firstMethod() { + System.out.println("first"); + } + + /* Another method comment */ + public void secondMethod() { + System.out.println("second"); + } + } + """ + ) + ); + } + + @Test + void variableDeclarationsWithMultipleVariables() { + rewriteRun( + //language=java + java( + """ + class MultipleVars { + public void method() { + System.out.println("method"); + } + + private int x = 1, y = 2, z = 3; + + public MultipleVars() { + // constructor + } + } + """, + """ + class MultipleVars { + private int x = 1, y = 2, z = 3; + + public void method() { + System.out.println("method"); + } + + public MultipleVars() { + // constructor + } + } + """ + ) + ); + } +} From 160aff6b2a1d3993e8dd960d7377dd521a2d82da Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 9 Sep 2025 14:47:25 -0400 Subject: [PATCH 02/10] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index 62b52f38b4..7fef383cd3 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -20,7 +20,6 @@ import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; -import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.Space; @@ -92,7 +91,6 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex // but preserve its original comments Space originalPrefix = field.getPrefix(); Space firstStatementPrefix = statements.get(0).getPrefix(); - // Combine: use first statement's whitespace but preserve field's comments Space newPrefix = firstStatementPrefix.withComments(originalPrefix.getComments()); field = field.withPrefix(newPrefix); From 8e3c325aa046eac9426e256ee4d91fbd7304d409 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Thu, 11 Sep 2025 17:35:47 -0400 Subject: [PATCH 03/10] * Eliminated ArrayList allocations * Implemented Comparator-based sorting - public static final > protected static final > private static final - > public final > protected final > private final - > public > protected > private * Preserved comments and spacing - Proper formatting maintained --- .../MoveFieldsToTopOfClass.java | 150 +++++++++++------- .../MoveFieldsToTopOfClassTest.java | 19 +-- 2 files changed, 104 insertions(+), 65 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index 7fef383cd3..bd4ace3048 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -24,10 +24,15 @@ import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.Space; import org.openrewrite.java.tree.Statement; +import org.openrewrite.style.GeneralFormatStyle; +import org.openrewrite.style.Style; import java.time.Duration; -import java.util.ArrayList; +import java.util.Comparator; import java.util.List; +import java.util.stream.IntStream; + +import static org.openrewrite.java.format.AutodetectGeneralFormatStyle.autodetectGeneralFormatStyle; @EqualsAndHashCode(callSuper = false) @Value @@ -35,15 +40,15 @@ public class MoveFieldsToTopOfClass extends Recipe { @Override public String getDisplayName() { - return "Move fields to the top of class definition"; + return "Move fields to the top of class declaration"; } @Override public String getDescription() { return "Reorders class members so that all field declarations appear before any method declarations, " + - "constructors, or other class members. This improves code organization and readability by " + - "grouping field declarations together at the top of the class. Comments associated with fields " + - "are preserved during the reordering."; + "constructors, or other class members. This improves code organization and readability by " + + "grouping field declarations together at the top of the class. Comments associated with fields " + + "are preserved during the reordering."; } @Override @@ -63,74 +68,107 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex return cd; } - List fieldDeclarations = new ArrayList<>(); - List otherStatements = new ArrayList<>(); + // The spacing of the original first statement is preserved so that + // it can be applied to the new first statement. + Statement originalFirstStatement = statements.get(0); - // Separate field declarations from other statements - for (Statement statement : statements) { - if (statement instanceof J.VariableDeclarations) { - fieldDeclarations.add(statement); - } else { - otherStatements.add(statement); - } - } + // Sort statements: fields first (public static, protected, private), then non-fields + statements.sort(createStatementComparator()); - // If all fields are already at the top, no changes needed - if (fieldDeclarations.isEmpty() || isAlreadyOrdered(statements, fieldDeclarations)) { - return cd; + // Adjust prefix of the first statement if it's not the original first statement + if (statements.get(0) != originalFirstStatement) { + Statement firstStatement = adjustPrefix(statements.get(0), originalFirstStatement.getPrefix()); + statements.set(0, firstStatement); } - // Reorder: fields first, then other statements - List reorderedStatements = new ArrayList<>(); - - // Add field declarations with preserved comments and proper spacing - for (int i = 0; i < fieldDeclarations.size(); i++) { - Statement field = fieldDeclarations.get(i); - if (i == 0) { - // First field should have the same prefix as the first statement originally had, - // but preserve its original comments - Space originalPrefix = field.getPrefix(); - Space firstStatementPrefix = statements.get(0).getPrefix(); - // Combine: use first statement's whitespace but preserve field's comments - Space newPrefix = firstStatementPrefix.withComments(originalPrefix.getComments()); - field = field.withPrefix(newPrefix); - } - reorderedStatements.add(field); - } + // ensure proper spacing before the first non-field statement + IntStream.range(1, statements.size()) + .filter(i -> !(statements.get(i) instanceof J.VariableDeclarations)) + .findFirst() + .ifPresent(i -> statements.set(i, ensurePrecedingNewline(statements.get(i)))); - // Add other statements - reorderedStatements.addAll(otherStatements); + return cd.withBody(cd.getBody().withStatements(statements)); + } - return autoFormat(cd.withBody(cd.getBody().withStatements(reorderedStatements)), ctx); + private Statement adjustPrefix(Statement field, Space originalPrefix) { + Space fieldPrefix = field.getPrefix(); + Space newPrefix = originalPrefix.withComments(fieldPrefix.getComments()); + return field.withPrefix(newPrefix); } - private boolean isAlreadyOrdered(List allStatements, List fieldDeclarations) { - if (fieldDeclarations.isEmpty()) { - return true; + private Statement ensurePrecedingNewline(Statement firstNonField) { + J.CompilationUnit cu = getCursor().firstEnclosing(J.CompilationUnit.class); + if (cu == null) { + return firstNonField; } + GeneralFormatStyle generalFormatStyle = Style.from(GeneralFormatStyle.class, cu, () -> autodetectGeneralFormatStyle(cu)); + String newLine = generalFormatStyle.newLine(); + + Space prefix = firstNonField.getPrefix(); + // if there is not an empty line before the first non-field, add one, + // preserving any comments and ensuring proper indentation + if (!prefix.getLastWhitespace().endsWith(String.format("%s\\s*%s", newLine, newLine))) { + String indent = prefix.getIndent(); + Space newPrefix = Space.format(newLine + newLine + indent).withComments(prefix.getComments()); + return firstNonField.withPrefix(newPrefix); + } + return firstNonField; + } + + + private Comparator createStatementComparator() { + return (s1, s2) -> { + boolean s1IsField = s1 instanceof J.VariableDeclarations; + boolean s2IsField = s2 instanceof J.VariableDeclarations; - int firstNonFieldIndex = -1; - for (int i = 0; i < allStatements.size(); i++) { - if (!(allStatements.get(i) instanceof J.VariableDeclarations)) { - firstNonFieldIndex = i; - break; - } + // Fields come before non-fields + if (s1IsField && !s2IsField) return -1; + if (!s1IsField && s2IsField) return 1; + if (!s1IsField) return 0; // Both are non-fields, preserve order + + // Both are fields - sort by visibility: public static first, then protected, then private + J.VariableDeclarations field1 = (J.VariableDeclarations) s1; + J.VariableDeclarations field2 = (J.VariableDeclarations) s2; + + int priority1 = getFieldPriority(field1); + int priority2 = getFieldPriority(field2); + + return Integer.compare(priority1, priority2); + }; + } + + private int getFieldPriority(J.VariableDeclarations field) { + // Use bitmask where lower values have higher priority + // Bit 3-2: visibility (00=public, 01=protected, 10=private) + // Bit 1: static/final (0=static final, 1=final or instance) + // Bit 0: final (0=final, 1=instance) + + int priority = 0b000; // Start from 0 + + // Visibility bits (bits 3-2) + if (field.hasModifier(J.Modifier.Type.Private)) { + priority |= 0b1000; // private + } else if (field.hasModifier(J.Modifier.Type.Protected)) { + priority |= 0b0100; // protected } + // public gets 0b0000 - // If there are no non-field statements, or all fields come before first non-field - if (firstNonFieldIndex == -1) { - return true; + // Static/final precedence (bit 1) + boolean isStatic = field.hasModifier(J.Modifier.Type.Static); + boolean isFinal = field.hasModifier(J.Modifier.Type.Final); + + if (!isStatic || !isFinal) { + priority |= 0b010; // not (static final) } - // Check if any field declarations come after the first non-field statement - for (int i = firstNonFieldIndex; i < allStatements.size(); i++) { - if (allStatements.get(i) instanceof J.VariableDeclarations) { - return false; - } + // Final bit (bit 0) + if (!isFinal) { + priority |= 0b001; // not final } - return true; + return priority; } + }; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java index 13877e440b..4d8c50ff5b 100644 --- a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java @@ -87,11 +87,11 @@ public void method2() { """, """ class Example { - private String field1 = "value1"; + public static final String CONSTANT = "constant"; protected int field2 = 42; - public static final String CONSTANT = "constant"; + private String field1 = "value1"; public void method1() { System.out.println("method1"); @@ -117,9 +117,9 @@ void fieldsAlreadyAtTop() { java( """ class AlreadyOrdered { - private String field1 = "value1"; - protected int field2 = 42; public static final String CONSTANT = "constant"; + protected int field2 = 42; + private String field1 = "value1"; public void method() { System.out.println("method"); @@ -141,9 +141,9 @@ void onlyFieldsInClass() { java( """ class OnlyFields { - private String field1 = "value1"; - protected int field2 = 42; public static final String CONSTANT = "constant"; + protected int field2 = 42; + private String field1 = "value1"; } """ ) @@ -256,9 +256,10 @@ public StaticAndInstance() { """, """ class StaticAndInstance { + public final int publicField = 100; + private static final String STATIC_FIELD = "static"; private String instanceField = "instance"; - public final int publicField = 100; public void method() { System.out.println("method"); @@ -299,10 +300,10 @@ public void innerMethod() { """, """ class OuterClass { - private String outerField = "outer"; - protected int anotherOuterField = 42; + private String outerField = "outer"; + public void outerMethod() { System.out.println("outer method"); } From 7c74a4cdd26e9c78739970b04ac496d1fa3fc5de Mon Sep 17 00:00:00 2001 From: David Grieve Date: Thu, 11 Sep 2025 17:37:06 -0400 Subject: [PATCH 04/10] Update src/main/resources/META-INF/rewrite/static-analysis.yml Co-authored-by: Tim te Beek --- src/main/resources/META-INF/rewrite/static-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/META-INF/rewrite/static-analysis.yml b/src/main/resources/META-INF/rewrite/static-analysis.yml index ff391c174a..f4fa0161b1 100644 --- a/src/main/resources/META-INF/rewrite/static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/static-analysis.yml @@ -25,7 +25,7 @@ recipeList: - org.openrewrite.java.format.EmptyNewlineAtEndOfFile - org.openrewrite.staticanalysis.ForLoopControlVariablePostfixOperators - org.openrewrite.staticanalysis.FinalizePrivateFields - - org.openrewrite.staticanalysis.MoveFieldsToTopOfClass +# - org.openrewrite.staticanalysis.MoveFieldsToTopOfClass - org.openrewrite.java.format.MethodParamPad - org.openrewrite.java.format.NoWhitespaceAfter - org.openrewrite.java.format.NoWhitespaceBefore From f39cdc5cb1bc7d98e795d13713c0643e0d62c6ec Mon Sep 17 00:00:00 2001 From: David Grieve Date: Fri, 12 Sep 2025 15:41:51 -0400 Subject: [PATCH 05/10] Fix for "Expected recipe to complete in 1 cycle, but took 2 cycles." Fix sort order of fields by modifier Co-authored-by: Jenson3210 --- .../MoveFieldsToTopOfClass.java | 66 ++++++++--------- .../MoveFieldsToTopOfClassTest.java | 74 +++++++++++++++---- 2 files changed, 92 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index bd4ace3048..5d02dbf12d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -83,9 +83,9 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex // ensure proper spacing before the first non-field statement IntStream.range(1, statements.size()) - .filter(i -> !(statements.get(i) instanceof J.VariableDeclarations)) - .findFirst() - .ifPresent(i -> statements.set(i, ensurePrecedingNewline(statements.get(i)))); + .filter(i -> !(statements.get(i) instanceof J.VariableDeclarations)) + .findFirst() + .ifPresent(i -> statements.set(i, ensurePrecedingNewline(statements.get(i)))); return cd.withBody(cd.getBody().withStatements(statements)); } @@ -107,10 +107,9 @@ private Statement ensurePrecedingNewline(Statement firstNonField) { Space prefix = firstNonField.getPrefix(); // if there is not an empty line before the first non-field, add one, // preserving any comments and ensuring proper indentation - if (!prefix.getLastWhitespace().endsWith(String.format("%s\\s*%s", newLine, newLine))) { + if (!prefix.getWhitespace().matches(String.format("\\s*%1s\\s*%1s\\s*", newLine, newLine))) { String indent = prefix.getIndent(); - Space newPrefix = Space.format(newLine + newLine + indent).withComments(prefix.getComments()); - return firstNonField.withPrefix(newPrefix); + return firstNonField.withPrefix(prefix.withWhitespace(newLine + newLine + indent).withComments(prefix.getComments())); } return firstNonField; } @@ -126,47 +125,46 @@ private Comparator createStatementComparator() { if (!s1IsField && s2IsField) return 1; if (!s1IsField) return 0; // Both are non-fields, preserve order - // Both are fields - sort by visibility: public static first, then protected, then private + // Both are fields - sort by visibility and modifiers J.VariableDeclarations field1 = (J.VariableDeclarations) s1; J.VariableDeclarations field2 = (J.VariableDeclarations) s2; - int priority1 = getFieldPriority(field1); - int priority2 = getFieldPriority(field2); + int priority1 = getFieldSortOrder(field1); + int priority2 = getFieldSortOrder(field2); return Integer.compare(priority1, priority2); }; } - private int getFieldPriority(J.VariableDeclarations field) { - // Use bitmask where lower values have higher priority - // Bit 3-2: visibility (00=public, 01=protected, 10=private) - // Bit 1: static/final (0=static final, 1=final or instance) - // Bit 0: final (0=final, 1=instance) - - int priority = 0b000; // Start from 0 - - // Visibility bits (bits 3-2) - if (field.hasModifier(J.Modifier.Type.Private)) { - priority |= 0b1000; // private - } else if (field.hasModifier(J.Modifier.Type.Protected)) { - priority |= 0b0100; // protected + // bitmasks for field sorting + // order: public static final < static final < protected static final < private static final < + // public static < static < protected static < private static < + // public final < final < protected final < private final < + // public < package-private < protected < private + private static final int PACKAGE_PRIVATE = 1; + private static final int PROTECTED = PACKAGE_PRIVATE << 1; + private static final int PRIVATE = PROTECTED << 1; + private static final int NON_FINAL = PRIVATE << 1; + private static final int NON_STATIC = NON_FINAL << 1; + + private int getFieldSortOrder(J.VariableDeclarations field) { + int order = 0; + if (field.hasModifier(J.Modifier.Type.Protected)) { + order |= PROTECTED; + } else if (field.hasModifier(J.Modifier.Type.Private)) { + order |= PRIVATE; + } else { + order |= PACKAGE_PRIVATE; } - // public gets 0b0000 - // Static/final precedence (bit 1) - boolean isStatic = field.hasModifier(J.Modifier.Type.Static); - boolean isFinal = field.hasModifier(J.Modifier.Type.Final); - - if (!isStatic || !isFinal) { - priority |= 0b010; // not (static final) + if (!field.hasModifier(J.Modifier.Type.Static)) { + order |= NON_STATIC; } - // Final bit (bit 0) - if (!isFinal) { - priority |= 0b001; // not final + if (!field.hasModifier(J.Modifier.Type.Final)) { + order |= NON_FINAL; } - - return priority; + return order; } }; diff --git a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java index 4d8c50ff5b..b0634dcdf4 100644 --- a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java @@ -256,9 +256,8 @@ public StaticAndInstance() { """, """ class StaticAndInstance { - public final int publicField = 100; - private static final String STATIC_FIELD = "static"; + public final int publicField = 100; private String instanceField = "instance"; public void method() { @@ -331,7 +330,7 @@ class A { void foo(String bar) { int i = Integer.parseInt(bar); } - + // this is my field, not yours String myField = "important"; } @@ -340,7 +339,7 @@ void foo(String bar) { class A { // this is my field, not yours String myField = "important"; - + void foo(String bar) { int i = Integer.parseInt(bar); } @@ -360,13 +359,13 @@ class A { public void method() { System.out.println("method"); } - + /* * This is a multiline comment * for my field */ private String field1 = "value1"; - + /** * Javadoc comment for field2 * @deprecated this field is old @@ -382,14 +381,14 @@ class A { * for my field */ private String field1 = "value1"; - + /** * Javadoc comment for field2 * @deprecated this field is old */ @Deprecated private int field2 = 42; - + public void method() { System.out.println("method"); } @@ -410,15 +409,15 @@ class Mixed { public void firstMethod() { System.out.println("first"); } - + // Field comment 1 private String field1 = "value1"; - + /* Another method comment */ public void secondMethod() { System.out.println("second"); } - + // Field comment 2 private int field2 = 10; } @@ -427,15 +426,15 @@ public void secondMethod() { class Mixed { // Field comment 1 private String field1 = "value1"; - + // Field comment 2 private int field2 = 10; - + // Method comment public void firstMethod() { System.out.println("first"); } - + /* Another method comment */ public void secondMethod() { System.out.println("second"); @@ -480,4 +479,51 @@ public MultipleVars() { ) ); } + + @Test + void variableDeclarationsAnnotations() { + rewriteRun( + //language=java + java( + """ + class A { + + public void method() { + System.out.println("method"); + } + + @Deprecated + private final int annotationNewLine; + + @Deprecated + private final int annotationNewLineWithNewLines; + + @Deprecated private final int annotationSameLine; + @Deprecated private final int annotationSameLineNoNewline; + + @Deprecated private final int annotationSameLineWithNewLines; + } + """, + """ + class A { + + @Deprecated + private final int annotationNewLine; + + @Deprecated + private final int annotationNewLineWithNewLines; + + @Deprecated private final int annotationSameLine; + @Deprecated private final int annotationSameLineNoNewline; + + @Deprecated private final int annotationSameLineWithNewLines; + + public void method() { + System.out.println("method"); + } + } + """ + ) + ); + } } From a5ae444ebb2a6c8257ce2e651550106dea5a51b1 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Fri, 12 Sep 2025 15:56:30 -0400 Subject: [PATCH 06/10] Update src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../staticanalysis/MoveFieldsToTopOfClass.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index 5d02dbf12d..de9f1c46e5 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -121,12 +121,17 @@ private Comparator createStatementComparator() { boolean s2IsField = s2 instanceof J.VariableDeclarations; // Fields come before non-fields - if (s1IsField && !s2IsField) return -1; - if (!s1IsField && s2IsField) return 1; - if (!s1IsField) return 0; // Both are non-fields, preserve order - - // Both are fields - sort by visibility and modifiers - J.VariableDeclarations field1 = (J.VariableDeclarations) s1; + if (s1IsField && !s2IsField) { + return -1; + } + if (!s1IsField && s2IsField) { + return 1; + } + if (!s1IsField) { + return 0; // Both are non-fields, preserve order + + // Both are fields - sort by visibility and modifiers + } J.VariableDeclarations field2 = (J.VariableDeclarations) s2; int priority1 = getFieldSortOrder(field1); From edb51069d240c13dc1aafbe73a600f241e5245f2 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 16 Sep 2025 15:47:07 -0400 Subject: [PATCH 07/10] do not fix line spacing --- .../MoveFieldsToTopOfClass.java | 95 ++++--------------- .../MoveFieldsToTopOfClassTest.java | 20 ++-- 2 files changed, 30 insertions(+), 85 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index de9f1c46e5..4450a407b0 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -22,17 +22,11 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.Space; import org.openrewrite.java.tree.Statement; -import org.openrewrite.style.GeneralFormatStyle; -import org.openrewrite.style.Style; import java.time.Duration; import java.util.Comparator; import java.util.List; -import java.util.stream.IntStream; - -import static org.openrewrite.java.format.AutodetectGeneralFormatStyle.autodetectGeneralFormatStyle; @EqualsAndHashCode(callSuper = false) @Value @@ -63,83 +57,36 @@ public TreeVisitor getVisitor() { public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); - List statements = cd.getBody().getStatements(); - if (statements.isEmpty()) { - return cd; - } - - // The spacing of the original first statement is preserved so that - // it can be applied to the new first statement. - Statement originalFirstStatement = statements.get(0); - - // Sort statements: fields first (public static, protected, private), then non-fields - statements.sort(createStatementComparator()); - - // Adjust prefix of the first statement if it's not the original first statement - if (statements.get(0) != originalFirstStatement) { - Statement firstStatement = adjustPrefix(statements.get(0), originalFirstStatement.getPrefix()); - statements.set(0, firstStatement); - } - - // ensure proper spacing before the first non-field statement - IntStream.range(1, statements.size()) - .filter(i -> !(statements.get(i) instanceof J.VariableDeclarations)) - .findFirst() - .ifPresent(i -> statements.set(i, ensurePrecedingNewline(statements.get(i)))); + List statements = + // getStatements() returns a new List each time, so no need to defensively copy + cd.getBody().getStatements() + .stream() + .sorted(statementComparator) + .collect(java.util.stream.Collectors.toList()); return cd.withBody(cd.getBody().withStatements(statements)); } - private Statement adjustPrefix(Statement field, Space originalPrefix) { - Space fieldPrefix = field.getPrefix(); - Space newPrefix = originalPrefix.withComments(fieldPrefix.getComments()); - return field.withPrefix(newPrefix); - } - - private Statement ensurePrecedingNewline(Statement firstNonField) { - J.CompilationUnit cu = getCursor().firstEnclosing(J.CompilationUnit.class); - if (cu == null) { - return firstNonField; - } - GeneralFormatStyle generalFormatStyle = Style.from(GeneralFormatStyle.class, cu, () -> autodetectGeneralFormatStyle(cu)); - String newLine = generalFormatStyle.newLine(); - - Space prefix = firstNonField.getPrefix(); - // if there is not an empty line before the first non-field, add one, - // preserving any comments and ensuring proper indentation - if (!prefix.getWhitespace().matches(String.format("\\s*%1s\\s*%1s\\s*", newLine, newLine))) { - String indent = prefix.getIndent(); - return firstNonField.withPrefix(prefix.withWhitespace(newLine + newLine + indent).withComments(prefix.getComments())); - } - return firstNonField; - } - - - private Comparator createStatementComparator() { - return (s1, s2) -> { - boolean s1IsField = s1 instanceof J.VariableDeclarations; - boolean s2IsField = s2 instanceof J.VariableDeclarations; + private final Comparator statementComparator = + (s1, s2) -> { + boolean s1IsField = s1 instanceof J.VariableDeclarations; + boolean s2IsField = s2 instanceof J.VariableDeclarations; - // Fields come before non-fields - if (s1IsField && !s2IsField) { - return -1; - } - if (!s1IsField && s2IsField) { - return 1; - } - if (!s1IsField) { - return 0; // Both are non-fields, preserve order + // Fields come before non-fields + if (s1IsField && !s2IsField) return -1; + if (!s1IsField && s2IsField) return 1; + if (!s1IsField) return 0; // Both are non-fields, preserve order // Both are fields - sort by visibility and modifiers - } - J.VariableDeclarations field2 = (J.VariableDeclarations) s2; + J.VariableDeclarations field1 = (J.VariableDeclarations) s1; + J.VariableDeclarations field2 = (J.VariableDeclarations) s2; - int priority1 = getFieldSortOrder(field1); - int priority2 = getFieldSortOrder(field2); + int priority1 = getFieldSortOrder(field1); + int priority2 = getFieldSortOrder(field2); + + return Integer.compare(priority1, priority2); + }; - return Integer.compare(priority1, priority2); - }; - } // bitmasks for field sorting // order: public static final < static final < protected static final < private static final < diff --git a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java index b0634dcdf4..3ba93f8313 100644 --- a/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java @@ -40,7 +40,6 @@ class A { void foo(String bar) { int i = Integer.parseInt(bar); } - // Important field String myField = "important"; } @@ -49,7 +48,6 @@ void foo(String bar) { class A { // Important field String myField = "important"; - void foo(String bar) { int i = Integer.parseInt(bar); } @@ -87,12 +85,12 @@ public void method2() { """, """ class Example { + public static final String CONSTANT = "constant"; protected int field2 = 42; private String field1 = "value1"; - public void method1() { System.out.println("method1"); } @@ -213,10 +211,10 @@ public void thirdMethod() { """, """ class Mixed { + private String field1 = "value1"; private int field2 = 10; - public void firstMethod() { System.out.println("first"); } @@ -256,10 +254,10 @@ public StaticAndInstance() { """, """ class StaticAndInstance { + private static final String STATIC_FIELD = "static"; public final int publicField = 100; private String instanceField = "instance"; - public void method() { System.out.println("method"); } @@ -299,17 +297,17 @@ public void innerMethod() { """, """ class OuterClass { + protected int anotherOuterField = 42; private String outerField = "outer"; - public void outerMethod() { System.out.println("outer method"); } class InnerClass { - private String innerField = "inner"; + private String innerField = "inner"; public void innerMethod() { System.out.println("inner method"); } @@ -337,9 +335,9 @@ void foo(String bar) { """, """ class A { + // this is my field, not yours String myField = "important"; - void foo(String bar) { int i = Integer.parseInt(bar); } @@ -376,6 +374,7 @@ public void method() { """, """ class A { + /* * This is a multiline comment * for my field @@ -388,7 +387,6 @@ class A { */ @Deprecated private int field2 = 42; - public void method() { System.out.println("method"); } @@ -424,12 +422,12 @@ public void secondMethod() { """, """ class Mixed { + // Field comment 1 private String field1 = "value1"; // Field comment 2 private int field2 = 10; - // Method comment public void firstMethod() { System.out.println("first"); @@ -465,8 +463,8 @@ public MultipleVars() { """, """ class MultipleVars { - private int x = 1, y = 2, z = 3; + private int x = 1, y = 2, z = 3; public void method() { System.out.println("method"); } From ef87dd4b91f772ecf9a207dc00f5a2c40729c3d0 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 16 Sep 2025 16:00:09 -0400 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../MoveFieldsToTopOfClass.java | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index 4450a407b0..7818031b05 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -28,6 +28,8 @@ import java.util.Comparator; import java.util.List; +import static java.util.stream.Collectors.toList; + @EqualsAndHashCode(callSuper = false) @Value public class MoveFieldsToTopOfClass extends Recipe { @@ -62,7 +64,7 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex cd.getBody().getStatements() .stream() .sorted(statementComparator) - .collect(java.util.stream.Collectors.toList()); + .collect(toList()); return cd.withBody(cd.getBody().withStatements(statements)); } @@ -73,12 +75,17 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex boolean s2IsField = s2 instanceof J.VariableDeclarations; // Fields come before non-fields - if (s1IsField && !s2IsField) return -1; - if (!s1IsField && s2IsField) return 1; - if (!s1IsField) return 0; // Both are non-fields, preserve order - - // Both are fields - sort by visibility and modifiers - J.VariableDeclarations field1 = (J.VariableDeclarations) s1; + if (s1IsField && !s2IsField) { + return -1; + } + if (!s1IsField && s2IsField) { + return 1; + } + if (!s1IsField) { + return 0; // Both are non-fields, preserve order + + // Both are fields - sort by visibility and modifiers + } J.VariableDeclarations field2 = (J.VariableDeclarations) s2; int priority1 = getFieldSortOrder(field1); From d0a22b535e8de8022a2049efdf29eb97733fe07d Mon Sep 17 00:00:00 2001 From: David Grieve Date: Tue, 16 Sep 2025 16:05:38 -0400 Subject: [PATCH 09/10] fix compile error introduced by review bot --- .../org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index 7818031b05..9178a25d90 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -86,6 +86,8 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex // Both are fields - sort by visibility and modifiers } + + J.VariableDeclarations field1 = (J.VariableDeclarations) s1; J.VariableDeclarations field2 = (J.VariableDeclarations) s2; int priority1 = getFieldSortOrder(field1); From 3e9c2bf6871639794a55fe168dd43f8849a4c4e8 Mon Sep 17 00:00:00 2001 From: David Grieve Date: Sun, 21 Sep 2025 18:53:23 -0500 Subject: [PATCH 10/10] Update src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java index 9178a25d90..7818031b05 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java @@ -86,8 +86,6 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex // Both are fields - sort by visibility and modifiers } - - J.VariableDeclarations field1 = (J.VariableDeclarations) s1; J.VariableDeclarations field2 = (J.VariableDeclarations) s2; int priority1 = getFieldSortOrder(field1);