Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 49 additions & 18 deletions src/main/java/org/openrewrite/java/migrate/util/UseListOf.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.openrewrite.java.migrate.util;

import lombok.Getter;
import org.jspecify.annotations.Nullable;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
Expand All @@ -29,6 +30,7 @@
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.Statement;
import org.openrewrite.java.tree.TypeUtils;

Expand All @@ -39,7 +41,15 @@

public class UseListOf extends Recipe {
private static final MethodMatcher NEW_ARRAY_LIST = new MethodMatcher("java.util.ArrayList <constructor>()", true);
private static final MethodMatcher NEW_LINKED_HASH_SET = new MethodMatcher("java.util.LinkedHashSet <constructor>()", true);
private static final MethodMatcher LIST_ADD = new MethodMatcher("java.util.List add(..)", true);
private static final MethodMatcher COLLECTION_ADD = new MethodMatcher("java.util.Collection add(..)", true);

/**
* Concrete collection types the prose pattern may keep: each preserves insertion order and has a
* {@code Collection} constructor, so wrapping an ordered {@code List.of(..)} is behavior preserving.
*/
private static final List<String> ORDERED_COLLECTIONS = Arrays.asList("java.util.ArrayList", "java.util.LinkedHashSet");

private static final String PROSE_REWRITES_KEY = "use-list-of.prose-rewrites";

Expand All @@ -51,36 +61,42 @@ public class UseListOf extends Recipe {
"- 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`).";
"- A `new ArrayList<>()` or `new LinkedHashSet<>()` declaration followed by a chain of " +
"`target.add(..)` statements, which is collapsed to `new ArrayList<>(List.of(..))` or " +
"`new LinkedHashSet<>(List.of(..))` (preserving both the mutable collection and its iteration order).";

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(
Preconditions.and(
new UsesJavaVersion<>(10),
new UsesMethod<>(NEW_ARRAY_LIST)),
Preconditions.or(
new UsesMethod<>(NEW_ARRAY_LIST),
new UsesMethod<>(NEW_LINKED_HASH_SET))),
new JavaVisitor<ExecutionContext>() {
@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(..))`.
// initializer should keep its type and wrap a `List.of(..)`.
Map<UUID, List<J.MethodInvocation>> rewrites = getCursor().getNearestMessage(PROSE_REWRITES_KEY);
if (rewrites != null) {
String orderedCollection = orderedCollectionType(n);
if (rewrites != null && orderedCollection != null) {
List<J.MethodInvocation> adds = rewrites.get(n.getId());
if (adds != null) {
String simpleName = orderedCollection.substring(orderedCollection.lastIndexOf('.') + 1);
List<Expression> args = new ArrayList<>();
StringJoiner joiner = new StringJoiner(", ", "new ArrayList<>(List.of(", "))");
StringJoiner joiner = new StringJoiner(", ", "new " + simpleName + "<>(List.of(", "))");
for (J.MethodInvocation add : adds) {
args.add(add.getArguments().get(0));
joiner.add("#{any()}");
}
maybeAddImport(orderedCollection);
maybeAddImport("java.util.List");
J applied = JavaTemplate.builder(joiner.toString())
.contextSensitive()
.imports("java.util.ArrayList", "java.util.List")
.imports(orderedCollection, "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
Expand Down Expand Up @@ -124,7 +140,7 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {

/**
* Re-applies the absorbed add statements' prefixes to the generated
* {@code new ArrayList<>(List.of(..))} so each element keeps its own line and any
* {@code new ArrayList<>(List.of(..))} constructor call 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<J.MethodInvocation> adds) {
Expand Down Expand Up @@ -223,14 +239,14 @@ private void identifyProseRewrites(

/**
* Returns the variable name if {@code decl} is a single-variable, parameterized
* {@code List<T>} declaration whose initializer is a no-arg {@code new ArrayList<>()}
* with no anonymous-class body. Returns {@code null} otherwise.
* declaration whose initializer is a no-arg {@code new ArrayList<>()} or
* {@code new LinkedHashSet<>()} 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.
// Require parameterized LHS; for a raw `List` we'd be guessing at a type argument.
if (!(decl.getTypeExpression() instanceof J.ParameterizedType)) {
return null;
}
Expand All @@ -239,10 +255,10 @@ private String matchingTargetName(J.VariableDeclarations decl) {
return null;
}
J.NewClass nc = (J.NewClass) nv.getInitializer();
if (!NEW_ARRAY_LIST.matches(nc)) {
if (!NEW_ARRAY_LIST.matches(nc) && !NEW_LINKED_HASH_SET.matches(nc)) {
return null;
}
if (!isExactlyArrayList(nc)) {
if (orderedCollectionType(nc) == null) {
return null;
}
// A body would put us in the anonymous-class case handled by visitNewClass directly.
Expand All @@ -253,16 +269,31 @@ private String matchingTargetName(J.VariableDeclarations decl) {
}

/**
* Skip `ArrayList` subclasses: `new ArrayList<>(..)` is not assignable to a subclass
* declared type, and the subclass may carry behavior beyond a plain `ArrayList`
* (issue #1181, matching #1113).
* Returns the constructed type if it is exactly one of {@link #ORDERED_COLLECTIONS}, which the
* prose rewrite retains rather than replacing. Subclasses return {@code null}: they may carry
* behavior beyond the collection they extend, and the constructor we'd generate might not exist
* on them (issue #1181, matching #1113).
*/
private @Nullable String orderedCollectionType(J.NewClass nc) {
JavaType type = nc.getClazz() != null ? nc.getClazz().getType() : null;
for (String fqn : ORDERED_COLLECTIONS) {
if (TypeUtils.isOfClassType(type, fqn)) {
return fqn;
}
}
return null;
}

/**
* Skip `ArrayList` subclasses: the subclass may carry behavior beyond a plain `ArrayList`
* that `List.of(..)` would drop (issue #1181, matching #1113).
*/
private boolean isExactlyArrayList(J.NewClass nc) {
return TypeUtils.isOfClassType(nc.getClazz() != null ? nc.getClazz().getType() : null, "java.util.ArrayList");
}

/**
* If {@code stmt} is {@code targetName.add(arg)} matching {@link #LIST_ADD},
* If {@code stmt} is {@code targetName.add(arg)} matching {@link #COLLECTION_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.
Expand All @@ -272,7 +303,7 @@ private Expression matchAddCallOn(Statement stmt, String targetName) {
return null;
}
J.MethodInvocation mi = (J.MethodInvocation) stmt;
if (!LIST_ADD.matches(mi)) {
if (!COLLECTION_ADD.matches(mi)) {
return null;
}
if (mi.getArguments().size() != 1) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/META-INF/rewrite/recipes.csv
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.u
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.,,
- A `new ArrayList<>()` or `new LinkedHashSet<>()` declaration followed by a chain of `target.add(..)` statements, which is collapsed to `new ArrayList<>(List.of(..))` or `new LinkedHashSet<>(List.of(..))` (preserving both the mutable collection and its iteration order).",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. Two input shapes are recognised:

Expand Down
99 changes: 99 additions & 0 deletions src/test/java/org/openrewrite/java/migrate/util/UseListOfTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,105 @@ void m() {
);
}

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/1181")
@Test
void convertLinkedHashSetToUseListOf() {
//language=java
rewriteRun(
java(
"""
import java.util.LinkedHashSet;
import java.util.Set;

class Test {
void m() {
Set<String> tags = new LinkedHashSet<>();
tags.add("alpha");
tags.add("beta");
tags.add("gamma");
}
}
""",
"""
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

class Test {
void m() {
Set<String> tags = new LinkedHashSet<>(List.of(
"alpha",
"beta",
"gamma"));
}
}
"""
)
);
}

@Test
void proseAddChainOnCollectionDeclaredType() {
//language=java
rewriteRun(
java(
"""
import java.util.ArrayList;
import java.util.Collection;

class Test {
void m() {
Collection<String> names = new ArrayList<>();
names.add("Bob");
names.add("alice");
}
}
""",
"""
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

class Test {
void m() {
Collection<String> names = new ArrayList<>(List.of(
"Bob",
"alice"));
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/1181")
@Test
void doNotChangeLinkedHashSetSubclass() {
//language=java
rewriteRun(
java(
"""
import java.util.LinkedHashSet;

class Tags extends LinkedHashSet<String> {}
"""
),
java(
"""
import java.util.Set;

class Test {
void m() {
Set<String> tags = new Tags();
tags.add("alpha");
tags.add("beta");
}
}
"""
)
);
}

@Test
void proseSingleAddBelowThresholdLeftAlone() {
// A single add is below the threshold — the rewrite would be more noise than benefit.
Expand Down
Loading