Hoist private static final constants onto the recipe class

Anonymous classes can not declare constants, so nested visitors that
declare them were skipped. Move them up to the enclosing recipe class
instead, unless that would collide with an existing field.

Author: timtebeek

src/main/java/org/openrewrite/java/recipes/InlineNestedVisitorClass.java

@@ -44,7 +44,9 @@
 import org.openrewrite.style.Style;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 @Value
@@ -57,8 +59,9 @@ public class InlineNestedVisitorClass extends Recipe {
 
     String description = "Recipes that return a named, private, static nested visitor class straight from " +
             "`getVisitor()` (or `getScanner()`) can declare that visitor anonymously instead, which keeps the " +
-            "visitor next to the recipe metadata that configures it. Only applied when the nested class is used " +
-            "exactly once, and when nothing would be lost by inlining it.";
+            "visitor next to the recipe metadata that configures it. Any `private static final` constants the " +
+            "nested class declares are hoisted onto the recipe class, as anonymous classes can not declare them. " +
+            "Only applied when the nested class is used exactly once, and when nothing would be lost by inlining it.";
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
@@ -71,7 +74,9 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
                     return cd;
                 }
 
+                boolean canHoist = getCursor().getParentTreeCursor().getValue() instanceof J.CompilationUnit;
                 List<J.ClassDeclaration> inlined = new ArrayList<>();
+                List<Statement> hoisted = new ArrayList<>();
                 List<Statement> mapped = ListUtils.map(statements, statement -> {
                     if (!(statement instanceof J.MethodDeclaration)) {
                         return statement;
@@ -89,8 +94,28 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
                     if (supertype == null) {
                         return statement;
                     }
-                    inlined.add(nested);
+                    // Anonymous classes can not declare constants, so they move up onto the recipe class instead
+                    List<Statement> constants = staticFields(nested);
+                    if (!constants.isEmpty() && (!canHoist || collidesWithExistingField(statements, constants))) {
+                        return statement;
+                    }
+
                     Cursor cursor = getCursor();
+                    J.Block body = nested.getBody();
+                    if (!constants.isEmpty()) {
+                        boolean leading = constants.contains(body.getStatements().get(0));
+                        List<Statement> kept = ListUtils.map(body.getStatements(), s -> constants.contains(s) ? null : s);
+                        if (leading) {
+                            kept = ListUtils.mapFirst(kept, first -> first.withPrefix(singleLine(first.getPrefix())));
+                        }
+                        body = body.withStatements(kept);
+                        for (Statement constant : constants) {
+                            hoisted.add(shiftIndent(constant, cursor, -1));
+                        }
+                    }
+
+                    inlined.add(nested);
+                    J.Block anonymousBody = shiftIndent(body, cursor, 1);
                     return (Statement) new JavaIsoVisitor<Integer>() {
                         @Override
                         public J.NewClass visitNewClass(J.NewClass nc, Integer p) {
@@ -99,7 +124,7 @@ public J.NewClass visitNewClass(J.NewClass nc, Integer p) {
                             }
                             return nc
                                     .withClazz(supertype.withPrefix(Space.SINGLE_SPACE))
-                                    .withBody(indentOneLevel(nested.getBody(), cursor));
+                                    .withBody(anonymousBody);
                         }
                     }.visitNonNull(method, 0);
                 });
@@ -112,6 +137,12 @@ public J.NewClass visitNewClass(J.NewClass nc, Integer p) {
                     Space prefix = statements.get(0).getPrefix();
                     remaining = ListUtils.mapFirst(remaining, first -> first.withPrefix(prefix));
                 }
+                if (!hoisted.isEmpty() && !remaining.isEmpty()) {
+                    Space prefix = remaining.get(0).getPrefix();
+                    remaining = ListUtils.mapFirst(remaining, first -> first.withPrefix(blankLineBefore(prefix)));
+                    remaining = ListUtils.concatAll(
+                            ListUtils.mapFirst(hoisted, first -> first.withPrefix(prefix)), remaining);
+                }
                 return cd.withBody(cd.getBody().withStatements(remaining));
             }
 
@@ -186,13 +217,53 @@ private boolean canInline(J.ClassDeclaration nested) {
                         return false;
                     }
                     if (statement instanceof J.VariableDeclarations &&
-                            ((J.VariableDeclarations) statement).hasModifier(J.Modifier.Type.Static)) {
+                            ((J.VariableDeclarations) statement).hasModifier(J.Modifier.Type.Static) &&
+                            !isPrivateStaticFinalField(statement)) {
                         return false;
                     }
                 }
                 return countReferences(nested) == 1;
             }
 
+            private boolean isPrivateStaticFinalField(Statement statement) {
+                if (!(statement instanceof J.VariableDeclarations)) {
+                    return false;
+                }
+                J.VariableDeclarations field = (J.VariableDeclarations) statement;
+                return field.hasModifier(J.Modifier.Type.Private) &&
+                        field.hasModifier(J.Modifier.Type.Static) &&
+                        field.hasModifier(J.Modifier.Type.Final);
+            }
+
+            private List<Statement> staticFields(J.ClassDeclaration nested) {
+                List<Statement> constants = new ArrayList<>();
+                for (Statement statement : nested.getBody().getStatements()) {
+                    if (isPrivateStaticFinalField(statement)) {
+                        constants.add(statement);
+                    }
+                }
+                return constants;
+            }
+
+            private boolean collidesWithExistingField(List<Statement> statements, List<Statement> constants) {
+                Set<String> existing = new HashSet<>();
+                for (Statement statement : statements) {
+                    if (statement instanceof J.VariableDeclarations) {
+                        for (J.VariableDeclarations.NamedVariable variable : ((J.VariableDeclarations) statement).getVariables()) {
+                            existing.add(variable.getSimpleName());
+                        }
+                    }
+                }
+                for (Statement constant : constants) {
+                    for (J.VariableDeclarations.NamedVariable variable : ((J.VariableDeclarations) constant).getVariables()) {
+                        if (!existing.add(variable.getSimpleName())) {
+                            return true;
+                        }
+                    }
+                }
+                return false;
+            }
+
             private int countReferences(J.ClassDeclaration nested) {
                 JavaType.FullyQualified type = nested.getType();
                 if (type == null) {
@@ -212,33 +283,36 @@ public J.Identifier visitIdentifier(J.Identifier identifier, AtomicInteger count
                 return references.get();
             }
 
-            private J.Block indentOneLevel(J.Block body, Cursor cursor) {
-                J.Block shifted = ShiftFormat.indent(body, cursor, 1);
+            private <J2 extends J> J2 shiftIndent(J2 tree, Cursor cursor, int levels) {
+                J2 shifted = ShiftFormat.indent(tree, cursor, levels);
                 JavaSourceFile cu = cursor.firstEnclosingOrThrow(JavaSourceFile.class);
                 TabsAndIndentsStyle style = Style.from(TabsAndIndentsStyle.class, cu, IntelliJ::tabsAndIndents);
                 String indent = style.getUseTabCharacter() ? "\t" : StringUtils.repeat(" ", style.getIndentSize());
-                return (J.Block) new JavaIsoVisitor<Integer>() {
+                //noinspection unchecked
+                return (J2) new JavaIsoVisitor<Integer>() {
                     @Override
                     public Space visitSpace(Space space, Space.Location loc, Integer p) {
-                        return space.withComments(ListUtils.map(space.getComments(), comment -> indentComment(comment, indent)));
+                        return space.withComments(ListUtils.map(space.getComments(), comment -> shiftComment(comment, indent, levels)));
                     }
                 }.visitNonNull(shifted, 0);
             }
 
-            private Comment indentComment(Comment comment, String indent) {
+            private Comment shiftComment(Comment comment, String indent, int levels) {
                 if (comment instanceof TextComment) {
                     TextComment textComment = (TextComment) comment;
                     if (textComment.getText().contains("\n")) {
-                        return textComment.withText(textComment.getText().replace("\n", "\n" + indent));
+                        return textComment.withText(levels > 0 ?
+                                textComment.getText().replace("\n", "\n" + indent) :
+                                textComment.getText().replace("\n" + indent, "\n"));
                     }
                 } else if (comment instanceof Javadoc.DocComment) {
                     Javadoc.DocComment docComment = (Javadoc.DocComment) comment;
-                    return docComment.withBody(ListUtils.map(docComment.getBody(), doc -> indentJavadoc(doc, indent)));
+                    return docComment.withBody(ListUtils.map(docComment.getBody(), doc -> shiftJavadoc(doc, indent, levels)));
                 }
                 return comment;
             }
 
-            private Javadoc indentJavadoc(Javadoc doc, String indent) {
+            private Javadoc shiftJavadoc(Javadoc doc, String indent, int levels) {
                 if (!(doc instanceof Javadoc.LineBreak)) {
                     return doc;
                 }
@@ -247,7 +321,24 @@ private Javadoc indentJavadoc(Javadoc doc, String indent) {
                 while (i < margin.length() && Character.isWhitespace(margin.charAt(i))) {
                     i++;
                 }
-                return ((Javadoc.LineBreak) doc).withMargin(margin.substring(0, i) + indent + margin.substring(i));
+                String whitespace = margin.substring(0, i);
+                if (levels > 0) {
+                    whitespace += indent;
+                } else if (whitespace.endsWith(indent)) {
+                    whitespace = whitespace.substring(0, whitespace.length() - indent.length());
+                }
+                return ((Javadoc.LineBreak) doc).withMargin(whitespace + margin.substring(i));
+            }
+
+            private Space singleLine(Space prefix) {
+                String whitespace = prefix.getWhitespace();
+                int last = whitespace.lastIndexOf('\n');
+                return last < 0 ? prefix : prefix.withWhitespace(whitespace.substring(last));
+            }
+
+            private Space blankLineBefore(Space prefix) {
+                String whitespace = prefix.getWhitespace();
+                return whitespace.startsWith("\n\n") ? prefix : prefix.withWhitespace("\n" + whitespace);
             }
 
             private @Nullable TypeTree supertypeOf(J.ClassDeclaration nested) {

src/main/resources/META-INF/rewrite/recipes.csv

@@ -13,7 +13,7 @@ maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.Exampl
 maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.ExecutionContextParameterName,Use a standard name for `ExecutionContext`,Visitors that are parameterized with `ExecutionContext` should use the parameter name `ctx`.,1,Recipes,Java,,Basic building blocks for transforming Java code.,"[{""name"":""parameterName"",""type"":""String"",""displayName"":""Parameter name"",""description"":""The name or prefix to use for the `ExecutionContext` parameter."",""example"":""ctx""}]",
 maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.FindRecipes,Find OpenRewrite recipes,"This recipe finds all OpenRewrite recipes, primarily to produce a data table that is being used to experiment with fine-tuning a large language model to produce more recipes.",1,Recipes,Java,,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.table.RewriteRecipeSource"",""displayName"":""Rewrite recipe source code"",""instanceName"":""Rewrite recipe source code"",""description"":""This table contains the source code of recipes along with their metadata for use in an experiment fine-tuning large language models to produce more recipes."",""columns"":[{""name"":""displayName"",""type"":""String"",""displayName"":""Recipe name"",""description"":""The name of the recipe.""},{""name"":""description"",""type"":""String"",""displayName"":""Recipe description"",""description"":""The description of the recipe.""},{""name"":""recipeType"",""type"":""RecipeType"",""displayName"":""Recipe type"",""description"":""Differentiate between Java and YAML recipes, as they may be two independent data sets used in LLM fine-tuning.""},{""name"":""sourceCode"",""type"":""String"",""displayName"":""Recipe source code"",""description"":""The full source code of the recipe.""},{""name"":""options"",""type"":""String"",""displayName"":""Recipe options"",""description"":""JSON format of recipe options.""}]}]"
 maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.GenerateDeprecatedMethodRecipes,Generate `InlineMethodCalls` recipes for deprecated delegating methods,"Finds `@Deprecated` method declarations whose body is a single delegation call to another method in the same class, and generates a declarative YAML recipe file containing `InlineMethodCalls` entries for each.",1,Recipes,Java,,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.java.recipes.DeprecatedMethodDelegations"",""displayName"":""Deprecated method delegations"",""instanceName"":""Deprecated method delegations"",""description"":""Deprecated methods that delegate to another method in the same class, suitable for inlining via `InlineMethodCalls`."",""columns"":[{""name"":""methodPattern"",""type"":""String"",""displayName"":""Method pattern"",""description"":""The method pattern of the deprecated method.""},{""name"":""replacement"",""type"":""String"",""displayName"":""Replacement"",""description"":""The replacement expression to inline.""},{""name"":""recipeYaml"",""type"":""String"",""displayName"":""Recipe YAML"",""description"":""A YAML snippet that can be copied into a recipe list.""}]}]"
-maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.InlineNestedVisitorClass,Inline nested visitor classes into the returning method,"Recipes that return a named, private, static nested visitor class straight from `getVisitor()` (or `getScanner()`) can declare that visitor anonymously instead, which keeps the visitor next to the recipe metadata that configures it. Only applied when the nested class is used exactly once, and when nothing would be lost by inlining it.",1,Recipes,Java,,Basic building blocks for transforming Java code.,,
+maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.InlineNestedVisitorClass,Inline nested visitor classes into the returning method,"Recipes that return a named, private, static nested visitor class straight from `getVisitor()` (or `getScanner()`) can declare that visitor anonymously instead, which keeps the visitor next to the recipe metadata that configures it. Any `private static final` constants the nested class declares are hoisted onto the recipe class, as anonymous classes can not declare them. Only applied when the nested class is used exactly once, and when nothing would be lost by inlining it.",1,Recipes,Java,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.IsLiteralNullRecipe,"Use `J.Literal.isLiteralValue(expression, null)`","Replace `expression instanceof J.Literal && ((J.Literal) expression).getValue() == null` with `J.Literal.isLiteralValue(expression, null)`.",1,Recipes,Java,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.JavaRecipeBestPractices,Java Recipe best practices,Best practices for Java recipe development.,25,Recipes,Java,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-rewrite,org.openrewrite.java.recipes.MissingOptionExample,Find missing `@Option` `example` values,"Find `@Option` annotations that are missing `example` values for documentation, and add a TODO comment.",1,Recipes,Java,,Basic building blocks for transforming Java code.,,