-
Notifications
You must be signed in to change notification settings - Fork 88
721: Recipe to move fields to the top of class definition #736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than Tim's comments, approving.
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
* 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
Co-authored-by: Tim te Beek <[email protected]>
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClassTest.java
Show resolved
Hide resolved
Fix sort order of fields by modifier Co-authored-by: Jenson3210 <[email protected]>
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
…Class.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/main/java/org/openrewrite/staticanalysis/MoveFieldsToTopOfClass.java
Outdated
Show resolved
Hide resolved
…Class.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The current issue is keeping a static intializer with the variable it's initializing. For instance, this simple test case will fail.
In addition, I would expect a static initializer block that initializes more than one variable to appear after the last variable declared (of those being initialized). I would also expect that other static blocks that aren't initializing a variable (doing something you want done when the class is first loaded) to come after all the variable declarations. None of these are being handled. Also, as @Jenson3210 points out, this could be a reorder statements recipe - which it is tending toward anyway. I'll add that how the statements are ordered should be a Style (even for the field decls, the ordering for which follows Oracle code conventions Lastly, this recipe in its current state messes up the formatting. But, as discussed with @sambsnyd, trying to maintain spacing seems overreach for this recipe. I feel, however, that without handling statement reordering and formatting this recipe in its current incantation just makes a mess of things. In short, moving the fields to the top is fairly trivial. Doing it correctly, however, is a bit tricky. |
What's changed?
Added MoveFieldsToTopOfClass recipe and tests. This recipe 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.
What's your motivation?
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
This PR adds the recipe to static-analysis.yml. I'm not sure if that should be done in a separate PR, or not done at all.
Checklist