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..608caad3f1 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,8 @@ 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; import org.openrewrite.java.MethodMatcher; @@ -30,19 +32,32 @@ 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; +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 +69,32 @@ 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 adds = rewrites.get(n.getId()); + if (adds != null) { + List args = new ArrayList<>(); + StringJoiner joiner = new StringJoiner(", ", "new ArrayList<>(List.of(", "))"); + for (J.MethodInvocation add : adds) { + args.add(add.getArguments().get(0)); + joiner.add("#{any()}"); + } + maybeAddImport("java.util.List"); + J applied = JavaTemplate.builder(joiner.toString()) + .contextSensitive() + .imports("java.util.ArrayList", "java.util.List") + .build() + .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); + } + } + + // 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 +106,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 +125,175 @@ 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 + // 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); + + // Post-pass: drop the now-absorbed `add(..)` statements from the block. + return b.withStatements(ListUtils.filter(b.getStatements(), s -> !absorbedAddIds.contains(s.getId()))); + } + + /** + * 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, 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, + 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 adds = 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; + } + adds.add((J.MethodInvocation) next); + absorbedHere.add(next.getId()); + j++; + } + if (adds.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), adds); + 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) { + return new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { + if (name.equals(id.getSimpleName())) { + f.set(true); + } + return id; + } + }.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 9da115c789..6b2e469307 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,8 @@ 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; import org.openrewrite.java.MethodMatcher; @@ -31,18 +33,33 @@ 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; +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 +71,41 @@ 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 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 (J.MethodInvocation put : puts) { + args.addAll(put.getArguments()); + if (useEntries) { + inner.add("Map.entry(#{any()}, #{any()})"); + } else { + inner.add("#{any()}"); + inner.add("#{any()}"); + } + } + String src = "new HashMap<>(" + inner + ")"; + maybeAddImport("java.util.Map"); + J applied = JavaTemplate.builder(src) + .contextSensitive() + .imports("java.util.HashMap", "java.util.Map") + .build() + .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); + } + } + + // 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 +163,180 @@ 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<>(); + 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); + + // Post-pass: drop the now-absorbed `put(..)` statements from the block. + return b.withStatements(ListUtils.filter(b.getStatements(), s -> !absorbedPutIds.contains(s.getId()))); + } + + /** + * 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, 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, + 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 puts = new ArrayList<>(); + List absorbedHere = new ArrayList<>(); + 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; + } + puts.add((J.MethodInvocation) next); + absorbedHere.add(next.getId()); + j++; + } + if (puts.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), puts); + 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) { + return new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { + if (name.equals(id.getSimpleName())) { + f.set(true); + } + return id; + } + }.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 3f71e5091b..88443fcea6 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,8 @@ 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; import org.openrewrite.java.MethodMatcher; @@ -30,19 +32,32 @@ 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; +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 +69,32 @@ 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 adds = rewrites.get(n.getId()); + if (adds != null) { + List args = new ArrayList<>(); + StringJoiner joiner = new StringJoiner(", ", "new HashSet<>(Set.of(", "))"); + for (J.MethodInvocation add : adds) { + args.add(add.getArguments().get(0)); + joiner.add("#{any()}"); + } + maybeAddImport("java.util.Set"); + J applied = JavaTemplate.builder(joiner.toString()) + .contextSensitive() + .imports("java.util.HashSet", "java.util.Set") + .build() + .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); + } + } + + // 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 +106,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 +125,149 @@ 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<>(); + 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); + + // 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( + 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 adds = 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; + } + adds.add((J.MethodInvocation) next); + absorbedHere.add(next.getId()); + j++; + } + if (adds.size() >= 2 && initializer != null) { + rewrites.put(initializer.getId(), adds); + 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) { + return new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier id, AtomicBoolean f) { + if (name.equals(id.getSimpleName())) { + f.set(true); + } + return id; + } + }.reduce(expr, new AtomicBoolean(false)).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..92ca15e3f6 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,211 @@ 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. + //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..5521eb965f 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,230 @@ 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 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)); + } + } + """ + ) + ); + } + + @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..b3152d37e1 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,179 @@ 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 + 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"); + } + } + """ + ) + ); + } }