From d63358260992402cefceaaf47892554e44466c82 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Wed, 24 Jun 2026 13:08:30 -0400 Subject: [PATCH 1/4] Extend Use{List,Set,Map}Of to recognise prose-statement chains UseListOf, UseSetOf, and UseMapOf previously matched only the anonymous-class idiom: List l = new ArrayList<>() {{ add("a"); add("b"); }}; Extends each recipe to also recognise the more common prose-statement shape, where a no-arg constructor declaration is followed by a chain of add(..) or put(..) calls on the declared variable: List l = new ArrayList<>(); l.add("a"); l.add("b"); These are collapsed into the constructor with the immutable factory: List l = new ArrayList<>(List.of("a", "b")); Output is always wrapped in the mutable ArrayList/HashSet/HashMap so downstream mutations (.sort(), .add(), etc.) remain valid. Recognised shapes per recipe: - UseListOf: target.add(arg) chains; output uses List.of(..). - UseSetOf: target.add(arg) chains; output uses Set.of(..). - UseMapOf: target.put(k, v) chains; output uses Map.of(k, v, ..) up to 10 pairs, Map.ofEntries(Map.entry(k, v), ..) beyond. Bail conditions (all three): raw LHS, non-no-arg constructor (e.g. capacity hint), intervening non-recognised statement, an argument that references the target variable (depends on step-by-step mutation), null literal argument. Threshold: at least 2 add/put statements; a single call is not worth the rewrite churn. Implementation: a visitBlock pre-pass scans for prose patterns and records (initializer UUID -> [args]) on a cursor message; the existing visitNewClass override consults the map and applies the JavaTemplate at the correct cursor depth. After super.visitBlock, the post-pass filters absorbed add/put statements from the block. Regenerate recipes.csv to reflect the new descriptions. --- .../java/migrate/util/UseListOf.java | 198 ++++++++++++++++- .../java/migrate/util/UseMapOf.java | 205 +++++++++++++++++- .../java/migrate/util/UseSetOf.java | 171 ++++++++++++++- .../resources/META-INF/rewrite/recipes.csv | 15 +- .../java/migrate/util/UseListOfTest.java | 169 +++++++++++++++ .../java/migrate/util/UseMapOfTest.java | 177 +++++++++++++++ .../java/migrate/util/UseSetOfTest.java | 137 ++++++++++++ 7 files changed, 1060 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java index 9ad7d2bfe5..6a79ff2dce 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java @@ -20,6 +20,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; @@ -30,19 +31,31 @@ import org.openrewrite.java.tree.Statement; import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.StringJoiner; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; public class UseListOf extends Recipe { private static final MethodMatcher NEW_ARRAY_LIST = new MethodMatcher("java.util.ArrayList ()", true); private static final MethodMatcher LIST_ADD = new MethodMatcher("java.util.List add(..)", true); + private static final String PROSE_REWRITES_KEY = "use-list-of.prose-rewrites"; + @Getter final String displayName = "Prefer `List.of(..)`"; @Getter - final String description = "Prefer `List.of(..)` instead of using `java.util.List#add(..)` in anonymous ArrayList initializers in Java 10 or higher. " + - "This recipe will not modify code where the List is later mutated since `List.of` returns an immutable list."; + final String description = "Prefer `List.of(..)` in Java 10 or higher. Two input shapes are recognised:\n\n" + + "- Anonymous-class initialization (`new ArrayList<>() {{ add(\"a\"); add(\"b\"); }}`), " + + "which is replaced wholesale with `List.of(\"a\", \"b\")` (immutable result, matching the " + + "anonymous-class idiom's typical intent).\n" + + "- A `new ArrayList<>()` declaration followed by a chain of `target.add(..)` statements, " + + "which is collapsed to `new ArrayList<>(List.of(..))` (preserving the mutable `ArrayList`)."; @Override public TreeVisitor getVisitor() { @@ -54,6 +67,27 @@ public TreeVisitor getVisitor() { @Override public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { J.NewClass n = (J.NewClass) super.visitNewClass(newClass, ctx); + + // Prose-pattern: see if visitBlock (above us on the cursor) decided this + // initializer should be wrapped with `new ArrayList<>(List.of(..))`. + Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); + if (rewrites != null) { + List proseArgs = rewrites.get(n.getId()); + if (proseArgs != null) { + StringJoiner joiner = new StringJoiner(", ", "new ArrayList<>(List.of(", "))"); + for (int k = 0; k < proseArgs.size(); k++) { + joiner.add("#{any()}"); + } + maybeAddImport("java.util.List"); + return JavaTemplate.builder(joiner.toString()) + .contextSensitive() + .imports("java.util.ArrayList", "java.util.List") + .build() + .apply(updateCursor(n), n.getCoordinates().replace(), proseArgs.toArray()); + } + } + + // Anonymous-class form (original UseListOf logic, unchanged). J.Block body = n.getBody(); if (NEW_ARRAY_LIST.matches(n) && body != null && body.getStatements().size() == 1) { Statement statement = body.getStatements().get(0); @@ -65,7 +99,6 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } J.MethodInvocation add = (J.MethodInvocation) stat; - // List.add() takes only one argument if (add.getArguments().size() != 1) { return n; } @@ -85,7 +118,164 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + + @Override + public J visitBlock(J.Block block, ExecutionContext ctx) { + // Pre-pass: scan the ORIGINAL block to identify which initializers to + // rewrite and which `add(..)` statements to absorb. UUIDs are stable + // through super.visitBlock unless a child visitor rebuilds the node, + // and nothing else in this recipe touches the targeted initializers + // before visitNewClass fires. + Map> rewrites = new HashMap<>(); + Set absorbedAddIds = new HashSet<>(); + identifyProseRewrites(block, rewrites, absorbedAddIds); + + if (!rewrites.isEmpty()) { + getCursor().putMessage(PROSE_REWRITES_KEY, rewrites); + } + + J.Block b = (J.Block) super.visitBlock(block, ctx); + + if (absorbedAddIds.isEmpty()) { + return b; + } + // Post-pass: drop the now-absorbed `add(..)` statements from the block. + List filtered = new ArrayList<>(b.getStatements().size()); + for (Statement s : b.getStatements()) { + if (!absorbedAddIds.contains(s.getId())) { + filtered.add(s); + } + } + return b.withStatements(filtered); + } + + /** + * Walk the block's statements looking for: + * 
+                     *     List<T> name = new ArrayList<>();
+                     *     name.add(x1);
+                     *     name.add(x2);
+                     *     ...
+                     *
+ * For each such sequence with at least two adds, record + * (initializer UUID, [args]) in {@code rewrites} and the absorbed add + * statement UUIDs in {@code absorbedAddIds}. + */ + private void identifyProseRewrites( + J.Block block, + Map> rewrites, + Set absorbedAddIds) { + List stmts = block.getStatements(); + int i = 0; + while (i < stmts.size()) { + Statement stmt = stmts.get(i); + if (!(stmt instanceof J.VariableDeclarations)) { + i++; + continue; + } + J.VariableDeclarations decl = (J.VariableDeclarations) stmt; + String targetName = matchingTargetName(decl); + if (targetName == null) { + i++; + continue; + } + J.NewClass initializer = (J.NewClass) decl.getVariables().get(0).getInitializer(); + // (matchingTargetName already verified the initializer is a J.NewClass) + + List args = new ArrayList<>(); + List absorbedHere = new ArrayList<>(); + int j = i + 1; + while (j < stmts.size()) { + Statement next = stmts.get(j); + Expression arg = matchAddCallOn(next, targetName); + if (arg == null || expressionReferences(arg, targetName)) { + break; + } + args.add(arg); + absorbedHere.add(next.getId()); + j++; + } + if (args.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), args); + absorbedAddIds.addAll(absorbedHere); + i = j; + } else { + i++; + } + } + } + + /** + * Returns the variable name if {@code decl} is a single-variable, parameterized + * {@code List} declaration whose initializer is a no-arg {@code new ArrayList<>()} + * with no anonymous-class body. Returns {@code null} otherwise. + */ + private String matchingTargetName(J.VariableDeclarations decl) { + if (decl.getVariables().size() != 1) { + return null; + } + // Require parameterized LHS; for raw `List` we'd be guessing at a type argument. + if (!(decl.getTypeExpression() instanceof J.ParameterizedType)) { + return null; + } + J.VariableDeclarations.NamedVariable nv = decl.getVariables().get(0); + if (!(nv.getInitializer() instanceof J.NewClass)) { + return null; + } + J.NewClass nc = (J.NewClass) nv.getInitializer(); + if (!NEW_ARRAY_LIST.matches(nc)) { + return null; + } + // A body would put us in the anonymous-class case handled by visitNewClass directly. + if (nc.getBody() != null) { + return null; + } + return nv.getSimpleName(); + } + + /** + * If {@code stmt} is {@code targetName.add(arg)} matching {@link #LIST_ADD}, + * returns the single argument expression; otherwise {@code null}. Also returns + * {@code null} when the argument is the {@code null} literal, since + * {@code List.of(..)} rejects nulls. + */ + private Expression matchAddCallOn(Statement stmt, String targetName) { + if (!(stmt instanceof J.MethodInvocation)) { + return null; + } + J.MethodInvocation mi = (J.MethodInvocation) stmt; + if (!LIST_ADD.matches(mi)) { + return null; + } + if (mi.getArguments().size() != 1) { + return null; + } + if (!(mi.getSelect() instanceof J.Identifier)) { + return null; + } + if (!targetName.equals(((J.Identifier) mi.getSelect()).getSimpleName())) { + return null; + } + Expression arg = mi.getArguments().get(0); + if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { + return null; + } + return arg; + } + + private boolean expressionReferences(Expression expr, String name) { + AtomicBoolean found = new AtomicBoolean(false); + new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { + if (name.equals(id.getSimpleName())) { + f.set(true); + } + return id; + } + }.visit(expr, found); + return found.get(); + } }); } - } diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java index 9da115c789..a0f7381ecb 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java @@ -20,6 +20,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; @@ -31,18 +32,32 @@ import org.openrewrite.java.tree.TypeUtils; import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.StringJoiner; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; public class UseMapOf extends Recipe { private static final MethodMatcher NEW_HASH_MAP = new MethodMatcher("java.util.HashMap ()", true); private static final MethodMatcher MAP_PUT = new MethodMatcher("java.util.Map put(..)", true); + private static final String PROSE_REWRITES_KEY = "use-map-of.prose-rewrites"; + @Getter final String displayName = "Prefer `Map.of(..)`"; @Getter - final String description = "Prefer `Map.of(..)` instead of using `java.util.Map#put(..)` in Java 10 or higher."; + final String description = "Prefer `Map.of(..)` instead of using `java.util.Map#put(..)` in Java 10 or higher. " + + "Two input shapes are recognised:\n\n" + + "- Anonymous-class initialization (`new HashMap<>() {{ put(k, v); ... }}`), which is replaced " + + "wholesale with `Map.of(k, v, ...)` (or `Map.ofEntries(...)` past ten entries) — immutable result.\n" + + "- A `new HashMap<>()` declaration followed by a chain of `target.put(k, v)` statements, " + + "which is collapsed to `new HashMap<>(Map.of(..))` (or `new HashMap<>(Map.ofEntries(..))`) — " + + "preserving the mutable `HashMap`."; @Override public TreeVisitor getVisitor() { @@ -54,6 +69,38 @@ public TreeVisitor getVisitor() { @Override public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { J.NewClass n = (J.NewClass) super.visitNewClass(newClass, ctx); + + // Prose-pattern: see if visitBlock decided this initializer should be wrapped + // with `new HashMap<>(Map.of(..))` or `new HashMap<>(Map.ofEntries(..))`. + Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); + if (rewrites != null) { + List proseArgs = rewrites.get(n.getId()); + if (proseArgs != null) { + // proseArgs is [k1, v1, k2, v2, ...] + int pairCount = proseArgs.size() / 2; + boolean useEntries = pairCount > 10; + StringJoiner inner = useEntries ? + new StringJoiner(", ", "Map.ofEntries(", ")") : + new StringJoiner(", ", "Map.of(", ")"); + for (int p = 0; p < pairCount; p++) { + if (useEntries) { + inner.add("Map.entry(#{any()}, #{any()})"); + } else { + inner.add("#{any()}"); + inner.add("#{any()}"); + } + } + String src = "new HashMap<>(" + inner + ")"; + maybeAddImport("java.util.Map"); + return JavaTemplate.builder(src) + .contextSensitive() + .imports("java.util.HashMap", "java.util.Map") + .build() + .apply(updateCursor(n), n.getCoordinates().replace(), proseArgs.toArray()); + } + } + + // Anonymous-class form (original UseMapOf logic, unchanged). J.Block body = n.getBody(); if (NEW_HASH_MAP.matches(n) && body != null && body.getStatements().size() == 1 && TypeUtils.isOfClassType(n.getClazz() != null ? n.getClazz().getType() : null, "java.util.HashMap")) { @@ -111,6 +158,162 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + + @Override + public J visitBlock(J.Block block, ExecutionContext ctx) { + Map> rewrites = new HashMap<>(); + Set absorbedPutIds = new HashSet<>(); + identifyProseRewrites(block, rewrites, absorbedPutIds); + + if (!rewrites.isEmpty()) { + getCursor().putMessage(PROSE_REWRITES_KEY, rewrites); + } + + J.Block b = (J.Block) super.visitBlock(block, ctx); + + if (absorbedPutIds.isEmpty()) { + return b; + } + List filtered = new ArrayList<>(b.getStatements().size()); + for (Statement s : b.getStatements()) { + if (!absorbedPutIds.contains(s.getId())) { + filtered.add(s); + } + } + return b.withStatements(filtered); + } + + /** + * Walk the block's statements looking for: + * 
+                     *     Map<K, V> name = new HashMap<>();
+                     *     name.put(k1, v1);
+                     *     name.put(k2, v2);
+                     *     ...
+                     *
+ * For each such sequence with at least two puts, record + * (initializer UUID, [k1, v1, k2, v2, ...]) in {@code rewrites} and the + * absorbed put statement UUIDs in {@code absorbedPutIds}. + */ + private void identifyProseRewrites( + J.Block block, + Map> rewrites, + Set absorbedPutIds) { + List stmts = block.getStatements(); + int i = 0; + while (i < stmts.size()) { + Statement stmt = stmts.get(i); + if (!(stmt instanceof J.VariableDeclarations)) { + i++; + continue; + } + J.VariableDeclarations decl = (J.VariableDeclarations) stmt; + String targetName = matchingTargetName(decl); + if (targetName == null) { + i++; + continue; + } + J.NewClass initializer = (J.NewClass) decl.getVariables().get(0).getInitializer(); + + List args = new ArrayList<>(); + List absorbedHere = new ArrayList<>(); + int pairs = 0; + int j = i + 1; + while (j < stmts.size()) { + Statement next = stmts.get(j); + List kv = matchPutCallOn(next, targetName); + if (kv == null) { + break; + } + if (expressionReferences(kv.get(0), targetName) || + expressionReferences(kv.get(1), targetName)) { + break; + } + args.addAll(kv); + absorbedHere.add(next.getId()); + pairs++; + j++; + } + if (pairs >= 2 && initializer != null) { + rewrites.put(initializer.getId(), args); + absorbedPutIds.addAll(absorbedHere); + i = j; + } else { + i++; + } + } + } + + /** + * Returns the variable name if {@code decl} is a single-variable, parameterized + * {@code Map} declaration whose initializer is a no-arg {@code new HashMap<>()} + * with no anonymous-class body. Returns {@code null} otherwise. + */ + private String matchingTargetName(J.VariableDeclarations decl) { + if (decl.getVariables().size() != 1) { + return null; + } + if (!(decl.getTypeExpression() instanceof J.ParameterizedType)) { + return null; + } + J.VariableDeclarations.NamedVariable nv = decl.getVariables().get(0); + if (!(nv.getInitializer() instanceof J.NewClass)) { + return null; + } + J.NewClass nc = (J.NewClass) nv.getInitializer(); + if (!NEW_HASH_MAP.matches(nc)) { + return null; + } + if (nc.getBody() != null) { + return null; + } + return nv.getSimpleName(); + } + + /** + * If {@code stmt} is {@code targetName.put(k, v)} matching {@link #MAP_PUT}, + * returns [key, value] as a list; otherwise {@code null}. Returns {@code null} + * if either argument is the {@code null} literal, since {@code Map.of(..)} and + * {@code Map.entry(..)} reject nulls. + */ + private List matchPutCallOn(Statement stmt, String targetName) { + if (!(stmt instanceof J.MethodInvocation)) { + return null; + } + J.MethodInvocation mi = (J.MethodInvocation) stmt; + if (!MAP_PUT.matches(mi)) { + return null; + } + if (mi.getArguments().size() != 2) { + return null; + } + if (!(mi.getSelect() instanceof J.Identifier)) { + return null; + } + if (!targetName.equals(((J.Identifier) mi.getSelect()).getSimpleName())) { + return null; + } + for (Expression arg : mi.getArguments()) { + if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { + return null; + } + } + return mi.getArguments(); + } + + private boolean expressionReferences(Expression expr, String name) { + AtomicBoolean found = new AtomicBoolean(false); + new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { + if (name.equals(id.getSimpleName())) { + f.set(true); + } + return id; + } + }.visit(expr, found); + return found.get(); + } }); } diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java index 3f71e5091b..80ea2a66f8 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java @@ -20,6 +20,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; @@ -30,19 +31,31 @@ import org.openrewrite.java.tree.Statement; import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.StringJoiner; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; public class UseSetOf extends Recipe { private static final MethodMatcher NEW_HASH_SET = new MethodMatcher("java.util.HashSet ()", true); private static final MethodMatcher SET_ADD = new MethodMatcher("java.util.Set add(..)", true); + private static final String PROSE_REWRITES_KEY = "use-set-of.prose-rewrites"; + @Getter final String displayName = "Prefer `Set.of(..)`"; @Getter - final String description = "Prefer `Set.of(..)` instead of using `java.util.Set#add(..)` in anonymous HashSet initializers in Java 10 or higher. " + - "This recipe will not modify code where the Set is later mutated since `Set.of` returns an immutable set."; + final String description = "Prefer `Set.of(..)` in Java 10 or higher. Two input shapes are recognised:\n\n" + + "- Anonymous-class initialization (`new HashSet<>() {{ add(\"a\"); add(\"b\"); }}`), " + + "which is replaced wholesale with `Set.of(\"a\", \"b\")` (immutable result, matching the " + + "anonymous-class idiom's typical intent).\n" + + "- A `new HashSet<>()` declaration followed by a chain of `target.add(..)` statements, " + + "which is collapsed to `new HashSet<>(Set.of(..))` (preserving the mutable `HashSet`)."; @Override public TreeVisitor getVisitor() { @@ -54,6 +67,27 @@ public TreeVisitor getVisitor() { @Override public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { J.NewClass n = (J.NewClass) super.visitNewClass(newClass, ctx); + + // Prose-pattern: see if visitBlock (above us on the cursor) decided this + // initializer should be wrapped with `new HashSet<>(Set.of(..))`. + Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); + if (rewrites != null) { + List proseArgs = rewrites.get(n.getId()); + if (proseArgs != null) { + StringJoiner joiner = new StringJoiner(", ", "new HashSet<>(Set.of(", "))"); + for (int k = 0; k < proseArgs.size(); k++) { + joiner.add("#{any()}"); + } + maybeAddImport("java.util.Set"); + return JavaTemplate.builder(joiner.toString()) + .contextSensitive() + .imports("java.util.HashSet", "java.util.Set") + .build() + .apply(updateCursor(n), n.getCoordinates().replace(), proseArgs.toArray()); + } + } + + // Anonymous-class form (original UseSetOf logic, unchanged). J.Block body = n.getBody(); if (NEW_HASH_SET.matches(n) && body != null && body.getStatements().size() == 1) { Statement statement = body.getStatements().get(0); @@ -65,7 +99,6 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } J.MethodInvocation add = (J.MethodInvocation) stat; - // Set.add() takes only one argument if (add.getArguments().size() != 1) { return n; } @@ -85,7 +118,137 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + + @Override + public J visitBlock(J.Block block, ExecutionContext ctx) { + Map> rewrites = new HashMap<>(); + Set absorbedAddIds = new HashSet<>(); + identifyProseRewrites(block, rewrites, absorbedAddIds); + + if (!rewrites.isEmpty()) { + getCursor().putMessage(PROSE_REWRITES_KEY, rewrites); + } + + J.Block b = (J.Block) super.visitBlock(block, ctx); + + if (absorbedAddIds.isEmpty()) { + return b; + } + List filtered = new ArrayList<>(b.getStatements().size()); + for (Statement s : b.getStatements()) { + if (!absorbedAddIds.contains(s.getId())) { + filtered.add(s); + } + } + return b.withStatements(filtered); + } + + private void identifyProseRewrites( + J.Block block, + Map> rewrites, + Set absorbedAddIds) { + List stmts = block.getStatements(); + int i = 0; + while (i < stmts.size()) { + Statement stmt = stmts.get(i); + if (!(stmt instanceof J.VariableDeclarations)) { + i++; + continue; + } + J.VariableDeclarations decl = (J.VariableDeclarations) stmt; + String targetName = matchingTargetName(decl); + if (targetName == null) { + i++; + continue; + } + J.NewClass initializer = (J.NewClass) decl.getVariables().get(0).getInitializer(); + + List args = new ArrayList<>(); + List absorbedHere = new ArrayList<>(); + int j = i + 1; + while (j < stmts.size()) { + Statement next = stmts.get(j); + Expression arg = matchAddCallOn(next, targetName); + if (arg == null || expressionReferences(arg, targetName)) { + break; + } + args.add(arg); + absorbedHere.add(next.getId()); + j++; + } + if (args.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), args); + absorbedAddIds.addAll(absorbedHere); + i = j; + } else { + i++; + } + } + } + + /** + * Returns the variable name if {@code decl} is a single-variable, parameterized + * {@code Set} declaration whose initializer is a no-arg {@code new HashSet<>()} + * with no anonymous-class body. Returns {@code null} otherwise. + */ + private String matchingTargetName(J.VariableDeclarations decl) { + if (decl.getVariables().size() != 1) { + return null; + } + if (!(decl.getTypeExpression() instanceof J.ParameterizedType)) { + return null; + } + J.VariableDeclarations.NamedVariable nv = decl.getVariables().get(0); + if (!(nv.getInitializer() instanceof J.NewClass)) { + return null; + } + J.NewClass nc = (J.NewClass) nv.getInitializer(); + if (!NEW_HASH_SET.matches(nc)) { + return null; + } + if (nc.getBody() != null) { + return null; + } + return nv.getSimpleName(); + } + + private Expression matchAddCallOn(Statement stmt, String targetName) { + if (!(stmt instanceof J.MethodInvocation)) { + return null; + } + J.MethodInvocation mi = (J.MethodInvocation) stmt; + if (!SET_ADD.matches(mi)) { + return null; + } + if (mi.getArguments().size() != 1) { + return null; + } + if (!(mi.getSelect() instanceof J.Identifier)) { + return null; + } + if (!targetName.equals(((J.Identifier) mi.getSelect()).getSimpleName())) { + return null; + } + Expression arg = mi.getArguments().get(0); + if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { + return null; + } + return arg; + } + + private boolean expressionReferences(Expression expr, String name) { + AtomicBoolean found = new AtomicBoolean(false); + new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { + if (name.equals(id.getSimpleName())) { + f.set(true); + } + return id; + } + }.visit(expr, found); + return found.get(); + } }); } - } diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 0da138850c..2bdaf98d36 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -486,9 +486,18 @@ maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.u maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.SequencedCollection,Adopt `SequencedCollection`,"Replace older code patterns with `SequencedCollection` methods, as per https://openjdk.org/jeps/431.",7,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.StreamFindFirst,Use `getFirst()` instead of `stream().findFirst().orElseThrow()`,"For SequencedCollections, use `collection.getFirst()` instead of `collection.stream().findFirst().orElseThrow()`.",1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseEnumSetOf,Prefer `EnumSet of(..)`,Prefer `EnumSet of(..)` instead of using `Set of(..)` when the arguments are enums in Java 9 or higher.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""convertEmptySet"",""type"":""Boolean"",""displayName"":""Convert empty `Set.of()` to `EnumSet.noneOf()`"",""description"":""When true, converts `Set.of()` with no arguments to `EnumSet.noneOf()`. Default true."",""example"":""true""}]", -maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseListOf,Prefer `List.of(..)`,Prefer `List.of(..)` instead of using `java.util.List#add(..)` in anonymous ArrayList initializers in Java 10 or higher. This recipe will not modify code where the List is later mutated since `List.of` returns an immutable list.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseListOf,Prefer `List.of(..)`,"Prefer `List.of(..)` in Java 10 or higher. Two input shapes are recognised: + +- Anonymous-class initialization (`new ArrayList<>() {{ add(""a""); add(""b""); }}`), which is replaced wholesale with `List.of(""a"", ""b"")` (immutable result, matching the anonymous-class idiom's typical intent). +- A `new ArrayList<>()` declaration followed by a chain of `target.add(..)` statements, which is collapsed to `new ArrayList<>(List.of(..))` (preserving the mutable `ArrayList`).",1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseLocaleOf,Prefer `Locale.of(..)` over `new Locale(..)`,Prefer `Locale.of(..)` over `new Locale(..)` in Java 19 or higher.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, -maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseMapOf,Prefer `Map.of(..)`,Prefer `Map.of(..)` instead of using `java.util.Map#put(..)` in Java 10 or higher.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseMapOf,Prefer `Map.of(..)`,"Prefer `Map.of(..)` instead of using `java.util.Map#put(..)` in Java 10 or higher. Two input shapes are recognised: + +- Anonymous-class initialization (`new HashMap<>() {{ put(k, v); ... }}`), which is replaced wholesale with `Map.of(k, v, ...)` (or `Map.ofEntries(...)` past ten entries) — immutable result. +- A `new HashMap<>()` declaration followed by a chain of `target.put(k, v)` statements, which is collapsed to `new HashMap<>(Map.of(..))` (or `new HashMap<>(Map.ofEntries(..))`) — preserving the mutable `HashMap`.",1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UsePredicateNot,Prefer `Predicate.not(..)` over casting to `Predicate` and calling `negate()`,Replace `((Predicate) lambdaOrMethodRef).negate()` with `Predicate.not(lambdaOrMethodRef)` as of Java 11.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, -maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseSetOf,Prefer `Set.of(..)`,Prefer `Set.of(..)` instead of using `java.util.Set#add(..)` in anonymous HashSet initializers in Java 10 or higher. This recipe will not modify code where the Set is later mutated since `Set.of` returns an immutable set.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.UseSetOf,Prefer `Set.of(..)`,"Prefer `Set.of(..)` in Java 10 or higher. Two input shapes are recognised: + +- Anonymous-class initialization (`new HashSet<>() {{ add(""a""); add(""b""); }}`), which is replaced wholesale with `Set.of(""a"", ""b"")` (immutable result, matching the anonymous-class idiom's typical intent). +- A `new HashSet<>()` declaration followed by a chain of `target.add(..)` statements, which is collapsed to `new HashSet<>(Set.of(..))` (preserving the mutable `HashSet`).",1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.scala.migrate.UpgradeScala_2_12,Migrate to Scala 2.12.+,Upgrade the Scala version for compatibility with newer Java versions.,2,,,Migrate,Scala,,,,,, diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java index 952a7d0d92..8be94778ac 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java @@ -226,4 +226,173 @@ void foo() { ) ); } + + @Test + void proseAddChainCollapsedIntoArrayListConstructor() { + // The mutable-ArrayList output preserves the ability to .add() / .sort() / etc. afterwards. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(); + names.add("Bob"); + names.add("alice"); + names.add("Charlie"); + names.sort(null); + } + } + """, + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(List.of("Bob", "alice", "Charlie")); + names.sort(null); + } + } + """ + ) + ); + } + + @Test + void proseSingleAddBelowThresholdLeftAlone() { + // A single add is below the threshold — the rewrite would be more noise than benefit. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(); + names.add("Bob"); + names.sort(null); + } + } + """ + ) + ); + } + + @Test + void proseInterveningStatementStopsAccumulation() { + // Anything other than a recognised add(x) between the decl and a later add breaks the chain. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(); + names.add("Bob"); + System.out.println("debug"); + names.add("alice"); + } + } + """ + ) + ); + } + + @Test + void proseArgReferencingTargetIsBail() { + // names.add(names.size() + "") depends on names being mutated step-by-step. + // Collapsing into List.of(...) would evaluate all args against the pre-add state and break semantics. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(); + names.add("first"); + names.add("size:" + names.size()); + } + } + """ + ) + ); + } + + @Test + void proseNullArgIsBail() { + // List.of(...) rejects null at runtime. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(); + names.add("Bob"); + names.add(null); + } + } + """ + ) + ); + } + + @Test + void proseRawListLeftAlone() { + // Raw LHS — ParameterizeRawCollection (or hand-edit) should run first. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList(); + names.add("Bob"); + names.add("alice"); + } + } + """ + ) + ); + } + + @Test + void proseArrayListWithCapacityArgLeftAlone() { + // new ArrayList<>(10) has a capacity hint that we'd be silently dropping. + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(10); + names.add("Bob"); + names.add("alice"); + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java index ce4f047680..1e3e567329 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java @@ -282,4 +282,181 @@ void foo() { ) ); } + + @Test + void proseChainCollapsedIntoHashMapConstructorMapOf() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(); + ages.put("Bob", 42); + ages.put("alice", 30); + ages.put("Charlie", 51); + } + } + """, + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(Map.of("Bob", 42, "alice", 30, "Charlie", 51)); + } + } + """ + ) + ); + } + + @Test + void proseChainOverTenPairsUsesMapOfEntries() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map codes = new HashMap<>(); + codes.put("a", 1); + codes.put("b", 2); + codes.put("c", 3); + codes.put("d", 4); + codes.put("e", 5); + codes.put("f", 6); + codes.put("g", 7); + codes.put("h", 8); + codes.put("i", 9); + codes.put("j", 10); + codes.put("k", 11); + } + } + """, + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map codes = new HashMap<>(Map.ofEntries(Map.entry("a", 1), Map.entry("b", 2), Map.entry("c", 3), Map.entry("d", 4), Map.entry("e", 5), Map.entry("f", 6), Map.entry("g", 7), Map.entry("h", 8), Map.entry("i", 9), Map.entry("j", 10), Map.entry("k", 11))); + } + } + """ + ) + ); + } + + @Test + void proseSinglePutBelowThresholdLeftAlone() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(); + ages.put("Bob", 42); + } + } + """ + ) + ); + } + + @Test + void proseInterveningStatementStopsAccumulation() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(); + ages.put("Bob", 42); + System.out.println("debug"); + ages.put("alice", 30); + } + } + """ + ) + ); + } + + @Test + void proseValueReferencingTargetIsBail() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map counts = new HashMap<>(); + counts.put("first", 1); + counts.put("size", counts.size()); + } + } + """ + ) + ); + } + + @Test + void proseNullKeyOrValueIsBail() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(); + ages.put("Bob", 42); + ages.put("alice", null); + } + } + """ + ) + ); + } + + @Test + void proseRawMapLeftAlone() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap(); + ages.put("Bob", 42); + ages.put("alice", 30); + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java index 02922a864f..affaa4d32e 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java @@ -226,4 +226,141 @@ void foo() { ) ); } + + @Test + void proseAddChainCollapsedIntoHashSetConstructor() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(); + tags.add("alpha"); + tags.add("beta"); + tags.add("gamma"); + } + } + """, + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(Set.of("alpha", "beta", "gamma")); + } + } + """ + ) + ); + } + + @Test + void proseSingleAddBelowThresholdLeftAlone() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(); + tags.add("alpha"); + } + } + """ + ) + ); + } + + @Test + void proseInterveningStatementStopsAccumulation() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(); + tags.add("alpha"); + System.out.println("debug"); + tags.add("beta"); + } + } + """ + ) + ); + } + + @Test + void proseArgReferencingTargetIsBail() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(); + tags.add("first"); + tags.add("size:" + tags.size()); + } + } + """ + ) + ); + } + + @Test + void proseNullArgIsBail() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(); + tags.add("alpha"); + tags.add(null); + } + } + """ + ) + ); + } + + @Test + void proseRawSetLeftAlone() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet(); + tags.add("alpha"); + tags.add("beta"); + } + } + """ + ) + ); + } } From e0cc381ca0d3f2b9e9be210c06c25dfb6435b346 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jun 2026 12:13:42 +0200 Subject: [PATCH 2/4] Simplify Use{List,Set,Map}Of prose helpers with ListUtils.filter and reduce --- .../java/migrate/util/UseListOf.java | 18 ++++-------------- .../java/migrate/util/UseMapOf.java | 19 +++++-------------- .../java/migrate/util/UseSetOf.java | 19 +++++-------------- 3 files changed, 14 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java index 6a79ff2dce..a0ea8d9be9 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java @@ -20,6 +20,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; @@ -136,17 +137,8 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = (J.Block) super.visitBlock(block, ctx); - if (absorbedAddIds.isEmpty()) { - return b; - } // Post-pass: drop the now-absorbed `add(..)` statements from the block. - List filtered = new ArrayList<>(b.getStatements().size()); - for (Statement s : b.getStatements()) { - if (!absorbedAddIds.contains(s.getId())) { - filtered.add(s); - } - } - return b.withStatements(filtered); + return b.withStatements(ListUtils.filter(b.getStatements(), s -> !absorbedAddIds.contains(s.getId()))); } /** @@ -264,8 +256,7 @@ private Expression matchAddCallOn(Statement stmt, String targetName) { } private boolean expressionReferences(Expression expr, String name) { - AtomicBoolean found = new AtomicBoolean(false); - new JavaIsoVisitor() { + return new JavaIsoVisitor() { @Override public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { if (name.equals(id.getSimpleName())) { @@ -273,8 +264,7 @@ public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { } return id; } - }.visit(expr, found); - return found.get(); + }.reduce(expr, new AtomicBoolean(false)).get(); } }); } diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java index a0f7381ecb..8e3ba91e5b 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java @@ -20,6 +20,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; @@ -171,16 +172,8 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = (J.Block) super.visitBlock(block, ctx); - if (absorbedPutIds.isEmpty()) { - return b; - } - List filtered = new ArrayList<>(b.getStatements().size()); - for (Statement s : b.getStatements()) { - if (!absorbedPutIds.contains(s.getId())) { - filtered.add(s); - } - } - return b.withStatements(filtered); + // Post-pass: drop the now-absorbed `put(..)` statements from the block. + return b.withStatements(ListUtils.filter(b.getStatements(), s -> !absorbedPutIds.contains(s.getId()))); } /** @@ -302,8 +295,7 @@ private List matchPutCallOn(Statement stmt, String targetName) { } private boolean expressionReferences(Expression expr, String name) { - AtomicBoolean found = new AtomicBoolean(false); - new JavaIsoVisitor() { + return new JavaIsoVisitor() { @Override public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { if (name.equals(id.getSimpleName())) { @@ -311,8 +303,7 @@ public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { } return id; } - }.visit(expr, found); - return found.get(); + }.reduce(expr, new AtomicBoolean(false)).get(); } }); } diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java index 80ea2a66f8..e0199ff95d 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java @@ -20,6 +20,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; @@ -131,16 +132,8 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = (J.Block) super.visitBlock(block, ctx); - if (absorbedAddIds.isEmpty()) { - return b; - } - List filtered = new ArrayList<>(b.getStatements().size()); - for (Statement s : b.getStatements()) { - if (!absorbedAddIds.contains(s.getId())) { - filtered.add(s); - } - } - return b.withStatements(filtered); + // Post-pass: drop the now-absorbed `add(..)` statements from the block. + return b.withStatements(ListUtils.filter(b.getStatements(), s -> !absorbedAddIds.contains(s.getId()))); } private void identifyProseRewrites( @@ -237,8 +230,7 @@ private Expression matchAddCallOn(Statement stmt, String targetName) { } private boolean expressionReferences(Expression expr, String name) { - AtomicBoolean found = new AtomicBoolean(false); - new JavaIsoVisitor() { + return new JavaIsoVisitor() { @Override public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { if (name.equals(id.getSimpleName())) { @@ -246,8 +238,7 @@ public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { } return id; } - }.visit(expr, found); - return found.get(); + }.reduce(expr, new AtomicBoolean(false)).get(); } }); } From 99973be6d62508e1ae223974d3f5e3ccc96255e2 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jun 2026 12:33:41 +0200 Subject: [PATCH 3/4] Preserve comments and one-per-line formatting in UseMapOf prose chains --- .../java/migrate/util/UseMapOf.java | 70 ++++++++++++++----- .../java/migrate/util/UseMapOfTest.java | 53 +++++++++++++- 2 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java index 8e3ba91e5b..6b2e469307 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java @@ -33,6 +33,7 @@ import org.openrewrite.java.tree.TypeUtils; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -73,17 +74,17 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { // Prose-pattern: see if visitBlock decided this initializer should be wrapped // with `new HashMap<>(Map.of(..))` or `new HashMap<>(Map.ofEntries(..))`. - Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); + Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); if (rewrites != null) { - List proseArgs = rewrites.get(n.getId()); - if (proseArgs != null) { - // proseArgs is [k1, v1, k2, v2, ...] - int pairCount = proseArgs.size() / 2; - boolean useEntries = pairCount > 10; + List puts = rewrites.get(n.getId()); + if (puts != null) { + boolean useEntries = puts.size() > 10; + List args = new ArrayList<>(); StringJoiner inner = useEntries ? new StringJoiner(", ", "Map.ofEntries(", ")") : new StringJoiner(", ", "Map.of(", ")"); - for (int p = 0; p < pairCount; p++) { + for (J.MethodInvocation put : puts) { + args.addAll(put.getArguments()); if (useEntries) { inner.add("Map.entry(#{any()}, #{any()})"); } else { @@ -93,11 +94,14 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { } String src = "new HashMap<>(" + inner + ")"; maybeAddImport("java.util.Map"); - return JavaTemplate.builder(src) + J applied = JavaTemplate.builder(src) .contextSensitive() .imports("java.util.HashMap", "java.util.Map") .build() - .apply(updateCursor(n), n.getCoordinates().replace(), proseArgs.toArray()); + .apply(updateCursor(n), n.getCoordinates().replace(), args.toArray()); + // Reattach each put's prefix so the entries land one-per-line and any + // leading comments survive, then autoformat to nest the indentation. + return autoFormat(reattachPairPrefixes(applied, puts, useEntries), ctx); } } @@ -160,9 +164,39 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + /** + * Re-applies the absorbed put statements' prefixes to the generated + * {@code new HashMap<>(Map.of(..))} / {@code new HashMap<>(Map.ofEntries(..))} so each + * pair keeps its own line and any leading comments. {@code puts} holds one invocation + * per pair, in order; for {@code Map.of} every other argument (the keys, at even + * indices) takes a prefix, while for {@code Map.ofEntries} every {@code Map.entry(..)} + * argument does. + */ + private J reattachPairPrefixes(J applied, List puts, boolean useEntries) { + if (!(applied instanceof J.NewClass)) { + return applied; + } + J.NewClass nc = (J.NewClass) applied; + if (nc.getArguments().size() != 1 || !(nc.getArguments().get(0) instanceof J.MethodInvocation)) { + return applied; + } + J.MethodInvocation mapCall = (J.MethodInvocation) nc.getArguments().get(0); + List mapArgs = mapCall.getArguments(); + int step = useEntries ? 1 : 2; + List withPrefixes = new ArrayList<>(mapArgs.size()); + for (int i = 0; i < mapArgs.size(); i++) { + Expression arg = mapArgs.get(i); + if (i % step == 0) { + arg = arg.withPrefix(puts.get(i / step).getPrefix()); + } + withPrefixes.add(arg); + } + return nc.withArguments(Collections.singletonList(mapCall.withArguments(withPrefixes))); + } + @Override public J visitBlock(J.Block block, ExecutionContext ctx) { - Map> rewrites = new HashMap<>(); + Map> rewrites = new HashMap<>(); Set absorbedPutIds = new HashSet<>(); identifyProseRewrites(block, rewrites, absorbedPutIds); @@ -185,12 +219,12 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { * ... * * For each such sequence with at least two puts, record - * (initializer UUID, [k1, v1, k2, v2, ...]) in {@code rewrites} and the - * absorbed put statement UUIDs in {@code absorbedPutIds}. + * (initializer UUID, the absorbed {@code put(..)} invocations in order) in + * {@code rewrites} and the absorbed put statement UUIDs in {@code absorbedPutIds}. */ private void identifyProseRewrites( J.Block block, - Map> rewrites, + Map> rewrites, Set absorbedPutIds) { List stmts = block.getStatements(); int i = 0; @@ -208,9 +242,8 @@ private void identifyProseRewrites( } J.NewClass initializer = (J.NewClass) decl.getVariables().get(0).getInitializer(); - List args = new ArrayList<>(); + List puts = new ArrayList<>(); List absorbedHere = new ArrayList<>(); - int pairs = 0; int j = i + 1; while (j < stmts.size()) { Statement next = stmts.get(j); @@ -222,13 +255,12 @@ private void identifyProseRewrites( expressionReferences(kv.get(1), targetName)) { break; } - args.addAll(kv); + puts.add((J.MethodInvocation) next); absorbedHere.add(next.getId()); - pairs++; j++; } - if (pairs >= 2 && initializer != null) { - rewrites.put(initializer.getId(), args); + if (puts.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), puts); absorbedPutIds.addAll(absorbedHere); i = j; } else { diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java index 1e3e567329..5521eb965f 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java @@ -307,7 +307,45 @@ void m() { class Test { void m() { - Map ages = new HashMap<>(Map.of("Bob", 42, "alice", 30, "Charlie", 51)); + Map ages = new HashMap<>(Map.of( + "Bob", 42, + "alice", 30, + "Charlie", 51)); + } + } + """ + ) + ); + } + + @Test + void prosePreservesCommentsOnPutStatements() { + //language=java + rewriteRun( + java( + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(); + // Bob is the boss + ages.put("Bob", 42); + ages.put("alice", 30); + } + } + """, + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + void m() { + Map ages = new HashMap<>(Map.of( + // Bob is the boss + "Bob", 42, + "alice", 30)); } } """ @@ -347,7 +385,18 @@ void m() { class Test { void m() { - Map codes = new HashMap<>(Map.ofEntries(Map.entry("a", 1), Map.entry("b", 2), Map.entry("c", 3), Map.entry("d", 4), Map.entry("e", 5), Map.entry("f", 6), Map.entry("g", 7), Map.entry("h", 8), Map.entry("i", 9), Map.entry("j", 10), Map.entry("k", 11))); + Map codes = new HashMap<>(Map.ofEntries( + Map.entry("a", 1), + Map.entry("b", 2), + Map.entry("c", 3), + Map.entry("d", 4), + Map.entry("e", 5), + Map.entry("f", 6), + Map.entry("g", 7), + Map.entry("h", 8), + Map.entry("i", 9), + Map.entry("j", 10), + Map.entry("k", 11))); } } """ From 3a497b9f5029267a6f079994c592fc42e45deb1d Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jun 2026 12:40:24 +0200 Subject: [PATCH 4/4] Extend prose comment + one-per-line formatting to UseListOf and UseSetOf --- .../java/migrate/util/UseListOf.java | 56 ++++++++++++++----- .../java/migrate/util/UseSetOf.java | 52 +++++++++++++---- .../java/migrate/util/UseListOfTest.java | 40 ++++++++++++- .../java/migrate/util/UseSetOfTest.java | 40 ++++++++++++- 4 files changed, 160 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java index a0ea8d9be9..608caad3f1 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseListOf.java @@ -32,6 +32,7 @@ import org.openrewrite.java.tree.Statement; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -71,20 +72,25 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { // Prose-pattern: see if visitBlock (above us on the cursor) decided this // initializer should be wrapped with `new ArrayList<>(List.of(..))`. - Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); + Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); if (rewrites != null) { - List proseArgs = rewrites.get(n.getId()); - if (proseArgs != null) { + List adds = rewrites.get(n.getId()); + if (adds != null) { + List args = new ArrayList<>(); StringJoiner joiner = new StringJoiner(", ", "new ArrayList<>(List.of(", "))"); - for (int k = 0; k < proseArgs.size(); k++) { + for (J.MethodInvocation add : adds) { + args.add(add.getArguments().get(0)); joiner.add("#{any()}"); } maybeAddImport("java.util.List"); - return JavaTemplate.builder(joiner.toString()) + J applied = JavaTemplate.builder(joiner.toString()) .contextSensitive() .imports("java.util.ArrayList", "java.util.List") .build() - .apply(updateCursor(n), n.getCoordinates().replace(), proseArgs.toArray()); + .apply(updateCursor(n), n.getCoordinates().replace(), args.toArray()); + // Reattach each add's prefix so the elements land one-per-line and any + // leading comments survive, then autoformat to nest the indentation. + return autoFormat(reattachElementPrefixes(applied, adds), ctx); } } @@ -120,6 +126,28 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + /** + * Re-applies the absorbed add statements' prefixes to the generated + * {@code new ArrayList<>(List.of(..))} so each element keeps its own line and any + * leading comments. {@code adds} holds one invocation per element, in order. + */ + private J reattachElementPrefixes(J applied, List adds) { + if (!(applied instanceof J.NewClass)) { + return applied; + } + J.NewClass nc = (J.NewClass) applied; + if (nc.getArguments().size() != 1 || !(nc.getArguments().get(0) instanceof J.MethodInvocation)) { + return applied; + } + J.MethodInvocation listCall = (J.MethodInvocation) nc.getArguments().get(0); + List listArgs = listCall.getArguments(); + List withPrefixes = new ArrayList<>(listArgs.size()); + for (int i = 0; i < listArgs.size(); i++) { + withPrefixes.add(listArgs.get(i).withPrefix(adds.get(i).getPrefix())); + } + return nc.withArguments(Collections.singletonList(listCall.withArguments(withPrefixes))); + } + @Override public J visitBlock(J.Block block, ExecutionContext ctx) { // Pre-pass: scan the ORIGINAL block to identify which initializers to @@ -127,7 +155,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { // through super.visitBlock unless a child visitor rebuilds the node, // and nothing else in this recipe touches the targeted initializers // before visitNewClass fires. - Map> rewrites = new HashMap<>(); + Map> rewrites = new HashMap<>(); Set absorbedAddIds = new HashSet<>(); identifyProseRewrites(block, rewrites, absorbedAddIds); @@ -150,12 +178,12 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { * ... * * For each such sequence with at least two adds, record - * (initializer UUID, [args]) in {@code rewrites} and the absorbed add - * statement UUIDs in {@code absorbedAddIds}. + * (initializer UUID, the absorbed {@code add(..)} invocations in order) in + * {@code rewrites} and the absorbed add statement UUIDs in {@code absorbedAddIds}. */ private void identifyProseRewrites( J.Block block, - Map> rewrites, + Map> rewrites, Set absorbedAddIds) { List stmts = block.getStatements(); int i = 0; @@ -174,7 +202,7 @@ private void identifyProseRewrites( J.NewClass initializer = (J.NewClass) decl.getVariables().get(0).getInitializer(); // (matchingTargetName already verified the initializer is a J.NewClass) - List args = new ArrayList<>(); + List adds = new ArrayList<>(); List absorbedHere = new ArrayList<>(); int j = i + 1; while (j < stmts.size()) { @@ -183,12 +211,12 @@ private void identifyProseRewrites( if (arg == null || expressionReferences(arg, targetName)) { break; } - args.add(arg); + adds.add((J.MethodInvocation) next); absorbedHere.add(next.getId()); j++; } - if (args.size() >= 2 && initializer != null) { - rewrites.put(initializer.getId(), args); + if (adds.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), adds); absorbedAddIds.addAll(absorbedHere); i = j; } else { diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java index e0199ff95d..88443fcea6 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseSetOf.java @@ -32,6 +32,7 @@ import org.openrewrite.java.tree.Statement; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -71,20 +72,25 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { // Prose-pattern: see if visitBlock (above us on the cursor) decided this // initializer should be wrapped with `new HashSet<>(Set.of(..))`. - Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); + Map> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY); if (rewrites != null) { - List proseArgs = rewrites.get(n.getId()); - if (proseArgs != null) { + List adds = rewrites.get(n.getId()); + if (adds != null) { + List args = new ArrayList<>(); StringJoiner joiner = new StringJoiner(", ", "new HashSet<>(Set.of(", "))"); - for (int k = 0; k < proseArgs.size(); k++) { + for (J.MethodInvocation add : adds) { + args.add(add.getArguments().get(0)); joiner.add("#{any()}"); } maybeAddImport("java.util.Set"); - return JavaTemplate.builder(joiner.toString()) + J applied = JavaTemplate.builder(joiner.toString()) .contextSensitive() .imports("java.util.HashSet", "java.util.Set") .build() - .apply(updateCursor(n), n.getCoordinates().replace(), proseArgs.toArray()); + .apply(updateCursor(n), n.getCoordinates().replace(), args.toArray()); + // Reattach each add's prefix so the elements land one-per-line and any + // leading comments survive, then autoformat to nest the indentation. + return autoFormat(reattachElementPrefixes(applied, adds), ctx); } } @@ -120,9 +126,31 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + /** + * Re-applies the absorbed add statements' prefixes to the generated + * {@code new HashSet<>(Set.of(..))} so each element keeps its own line and any + * leading comments. {@code adds} holds one invocation per element, in order. + */ + private J reattachElementPrefixes(J applied, List adds) { + if (!(applied instanceof J.NewClass)) { + return applied; + } + J.NewClass nc = (J.NewClass) applied; + if (nc.getArguments().size() != 1 || !(nc.getArguments().get(0) instanceof J.MethodInvocation)) { + return applied; + } + J.MethodInvocation setCall = (J.MethodInvocation) nc.getArguments().get(0); + List setArgs = setCall.getArguments(); + List withPrefixes = new ArrayList<>(setArgs.size()); + for (int i = 0; i < setArgs.size(); i++) { + withPrefixes.add(setArgs.get(i).withPrefix(adds.get(i).getPrefix())); + } + return nc.withArguments(Collections.singletonList(setCall.withArguments(withPrefixes))); + } + @Override public J visitBlock(J.Block block, ExecutionContext ctx) { - Map> rewrites = new HashMap<>(); + Map> rewrites = new HashMap<>(); Set absorbedAddIds = new HashSet<>(); identifyProseRewrites(block, rewrites, absorbedAddIds); @@ -138,7 +166,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { private void identifyProseRewrites( J.Block block, - Map> rewrites, + Map> rewrites, Set absorbedAddIds) { List stmts = block.getStatements(); int i = 0; @@ -156,7 +184,7 @@ private void identifyProseRewrites( } J.NewClass initializer = (J.NewClass) decl.getVariables().get(0).getInitializer(); - List args = new ArrayList<>(); + List adds = new ArrayList<>(); List absorbedHere = new ArrayList<>(); int j = i + 1; while (j < stmts.size()) { @@ -165,12 +193,12 @@ private void identifyProseRewrites( if (arg == null || expressionReferences(arg, targetName)) { break; } - args.add(arg); + adds.add((J.MethodInvocation) next); absorbedHere.add(next.getId()); j++; } - if (args.size() >= 2 && initializer != null) { - rewrites.put(initializer.getId(), args); + if (adds.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), adds); absorbedAddIds.addAll(absorbedHere); i = j; } else { diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java index 8be94778ac..92ca15e3f6 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java @@ -227,6 +227,41 @@ void foo() { ); } + @Test + void prosePreservesCommentsOnAddStatements() { + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(); + // the boss + names.add("Bob"); + names.add("alice"); + } + } + """, + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + void m() { + List names = new ArrayList<>(List.of( + // the boss + "Bob", + "alice")); + } + } + """ + ) + ); + } + @Test void proseAddChainCollapsedIntoArrayListConstructor() { // The mutable-ArrayList output preserves the ability to .add() / .sort() / etc. afterwards. @@ -253,7 +288,10 @@ void m() { class Test { void m() { - List names = new ArrayList<>(List.of("Bob", "alice", "Charlie")); + List names = new ArrayList<>(List.of( + "Bob", + "alice", + "Charlie")); names.sort(null); } } diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java index affaa4d32e..b3152d37e1 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseSetOfTest.java @@ -227,6 +227,41 @@ void foo() { ); } + @Test + void prosePreservesCommentsOnAddStatements() { + //language=java + rewriteRun( + java( + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(); + // primary + tags.add("alpha"); + tags.add("beta"); + } + } + """, + """ + import java.util.HashSet; + import java.util.Set; + + class Test { + void m() { + Set tags = new HashSet<>(Set.of( + // primary + "alpha", + "beta")); + } + } + """ + ) + ); + } + @Test void proseAddChainCollapsedIntoHashSetConstructor() { //language=java @@ -251,7 +286,10 @@ void m() { class Test { void m() { - Set tags = new HashSet<>(Set.of("alpha", "beta", "gamma")); + Set tags = new HashSet<>(Set.of( + "alpha", + "beta", + "gamma")); } } """