From 2a944642f5a6566a6012702dad00f2285de3fc15 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 15 Dec 2025 20:51:55 +0100 Subject: [PATCH] Avoid opinionated empty EnumSet by default --- .../java/migrate/util/UseEnumSetOf.java | 124 +++++---- .../META-INF/rewrite/java-version-6.yml | 3 +- .../java/migrate/util/UseEnumSetOfTest.java | 240 +++++++++--------- 3 files changed, 198 insertions(+), 169 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/UseEnumSetOf.java b/src/main/java/org/openrewrite/java/migrate/util/UseEnumSetOf.java index 34c37584f2..17f1d5d58f 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseEnumSetOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseEnumSetOf.java @@ -15,6 +15,8 @@ */ package org.openrewrite.java.migrate.util; +import lombok.EqualsAndHashCode; +import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.java.JavaTemplate; @@ -31,10 +33,21 @@ import java.util.List; import java.util.StringJoiner; +@EqualsAndHashCode(callSuper = false) +@Value public class UseEnumSetOf extends Recipe { private static final MethodMatcher SET_OF = new MethodMatcher("java.util.Set of(..)", true); private static final String METHOD_TYPE = "java.util.EnumSet"; + @Option( + displayName = "Convert empty `Set.of()` to `EnumSet.noneOf()`", + description = "When true, converts `Set.of()` with no arguments to `EnumSet.noneOf()`. Default true.", + example = "true", + required = false + ) + @Nullable + Boolean convertEmptySet; + @Override public String getDisplayName() { return "Prefer `EnumSet of(..)`"; @@ -52,73 +65,78 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(Preconditions.and(new UsesJavaVersion<>(9), - new UsesMethod<>(SET_OF)), new UseEnumSetOfVisitor()); - } + return Preconditions.check(Preconditions.and(new UsesJavaVersion<>(9), new UsesMethod<>(SET_OF)), new JavaVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) { + J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(methodInvocation, ctx); - private static class UseEnumSetOfVisitor extends JavaVisitor { - @Override - public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) { - J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(methodInvocation, ctx); - - if (SET_OF.matches(mi) && - mi.getType() instanceof JavaType.Parameterized && - !TypeUtils.isOfClassType(mi.getType(), METHOD_TYPE)) { - Cursor parent = getCursor().dropParentUntil(is -> is instanceof J.Assignment || is instanceof J.VariableDeclarations || is instanceof J.Block); - if (!(parent.getValue() instanceof J.Block)) { - JavaType type = parent.getValue() instanceof J.Assignment ? - ((J.Assignment) parent.getValue()).getType() : ((J.VariableDeclarations) parent.getValue()).getVariables().get(0).getType(); - if (isAssignmentSetOfEnum(type)) { - maybeAddImport(METHOD_TYPE); - - List args = mi.getArguments(); - if (isArrayParameter(args)) { - return mi; - } + if (SET_OF.matches(mi) && + mi.getType() instanceof JavaType.Parameterized && + !TypeUtils.isOfClassType(mi.getType(), METHOD_TYPE) && + convertEmptySet(mi)) { + Cursor parent = getCursor().dropParentUntil(is -> is instanceof J.Assignment || is instanceof J.VariableDeclarations || is instanceof J.Block); + if (!(parent.getValue() instanceof J.Block)) { + JavaType type = parent.getValue() instanceof J.Assignment ? + ((J.Assignment) parent.getValue()).getType() : ((J.VariableDeclarations) parent.getValue()).getVariables().get(0).getType(); + if (isAssignmentSetOfEnum(type)) { + maybeAddImport(METHOD_TYPE); - if (args.get(0) instanceof J.Empty) { - JavaType firstTypeParameter = ((JavaType.Parameterized) type).getTypeParameters().get(0); - JavaType.ShallowClass shallowClass = JavaType.ShallowClass.build(firstTypeParameter.toString()); - return JavaTemplate.builder("EnumSet.noneOf(" + shallowClass.getClassName() + ".class)") + List args = mi.getArguments(); + if (isArrayParameter(args)) { + return mi; + } + + if (args.get(0) instanceof J.Empty) { + JavaType firstTypeParameter = ((JavaType.Parameterized) type).getTypeParameters().get(0); + JavaType.ShallowClass shallowClass = JavaType.ShallowClass.build(firstTypeParameter.toString()); + return JavaTemplate.builder("EnumSet.noneOf(" + shallowClass.getClassName() + ".class)") + .contextSensitive() + .imports(METHOD_TYPE) + .build() + .apply(updateCursor(mi), mi.getCoordinates().replace()); + } + + StringJoiner setOf = new StringJoiner(", ", "EnumSet.of(", ")"); + args.forEach(o -> setOf.add("#{any()}")); + return JavaTemplate.builder(setOf.toString()) .contextSensitive() .imports(METHOD_TYPE) .build() - .apply(updateCursor(mi), mi.getCoordinates().replace()); + .apply(updateCursor(mi), mi.getCoordinates().replace(), args.toArray()); } - - StringJoiner setOf = new StringJoiner(", ", "EnumSet.of(", ")"); - args.forEach(o -> setOf.add("#{any()}")); - return JavaTemplate.builder(setOf.toString()) - .contextSensitive() - .imports(METHOD_TYPE) - .build() - .apply(updateCursor(mi), mi.getCoordinates().replace(), args.toArray()); } } + return mi; } - return mi; - } - - private boolean isAssignmentSetOfEnum(@Nullable JavaType type) { - if (type instanceof JavaType.Parameterized) { - JavaType.Parameterized parameterized = (JavaType.Parameterized) type; - if (TypeUtils.isOfClassType(parameterized.getType(), "java.util.Set")) { - return ((JavaType.Parameterized) type).getTypeParameters().stream() - .filter(org.openrewrite.java.tree.JavaType.Class.class::isInstance) - .map(org.openrewrite.java.tree.JavaType.Class.class::cast) - .anyMatch(o -> o.getKind() == JavaType.FullyQualified.Kind.Enum); + + private boolean convertEmptySet(J.MethodInvocation mi) { + if (convertEmptySet == null || convertEmptySet) { + return true; } + return !mi.getArguments().isEmpty() && !(mi.getArguments().get(0) instanceof J.Empty); } - return false; - } - private boolean isArrayParameter(final List args) { - if (args.size() != 1) { + private boolean isAssignmentSetOfEnum(@Nullable JavaType type) { + if (type instanceof JavaType.Parameterized) { + JavaType.Parameterized parameterized = (JavaType.Parameterized) type; + if (TypeUtils.isOfClassType(parameterized.getType(), "java.util.Set")) { + return ((JavaType.Parameterized) type).getTypeParameters().stream() + .filter(JavaType.Class.class::isInstance) + .map(JavaType.Class.class::cast) + .anyMatch(o -> o.getKind() == JavaType.FullyQualified.Kind.Enum); + } + } return false; } - JavaType type = args.get(0).getType(); - return TypeUtils.asArray(type) != null; - } + + private boolean isArrayParameter(final List args) { + if (args.size() != 1) { + return false; + } + JavaType type = args.get(0).getType(); + return TypeUtils.asArray(type) != null; + } + }); } } diff --git a/src/main/resources/META-INF/rewrite/java-version-6.yml b/src/main/resources/META-INF/rewrite/java-version-6.yml index 499a3cb40d..eb690fc8c7 100644 --- a/src/main/resources/META-INF/rewrite/java-version-6.yml +++ b/src/main/resources/META-INF/rewrite/java-version-6.yml @@ -25,7 +25,8 @@ tags: - java6 recipeList: - org.openrewrite.java.migrate.jacoco.UpgradeJaCoCo - - org.openrewrite.java.migrate.util.UseEnumSetOf + - org.openrewrite.java.migrate.util.UseEnumSetOf: + convertEmptySet: false - org.openrewrite.java.migrate.JREWrapperInterface --- type: specs.openrewrite.org/v1beta/recipe diff --git a/src/test/java/org/openrewrite/java/migrate/util/UseEnumSetOfTest.java b/src/test/java/org/openrewrite/java/migrate/util/UseEnumSetOfTest.java index 7dd3f1bed0..ba78e21246 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseEnumSetOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseEnumSetOfTest.java @@ -22,14 +22,16 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; -import static org.openrewrite.java.Assertions.version; +import static org.openrewrite.java.Assertions.javaVersion; @SuppressWarnings("UnusedAssignment") class UseEnumSetOfTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new UseEnumSetOf()); + spec + .recipe(new UseEnumSetOf(null)) + .allSources(s -> s.markers(javaVersion(9))); } @DocumentExample @@ -37,35 +39,32 @@ public void defaults(RecipeSpec spec) { void changeDeclaration() { //language=java rewriteRun( - version( - java( - """ - import java.util.Set; - - class Test { - public enum Color { - RED, GREEN, BLUE - } - public void method() { - Set warm = Set.of(Color.RED, Color.GREEN); - } - } - """, + java( + """ + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method() { + Set warm = Set.of(Color.RED, Color.GREEN); + } + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method() { + Set warm = EnumSet.of(Color.RED, Color.GREEN); + } + } """ - import java.util.EnumSet; - import java.util.Set; - - class Test { - public enum Color { - RED, GREEN, BLUE - } - public void method() { - Set warm = EnumSet.of(Color.RED, Color.GREEN); - } - } - """ - ), - 9 ) ); } @@ -74,37 +73,34 @@ public void method() { void changeAssignment() { //language=java rewriteRun( - version( - java( - """ - import java.util.Set; - - class Test { - public enum Color { - RED, GREEN, BLUE - } - public void method() { - Set warm; - warm = Set.of(Color.RED); - } - } - """, + java( + """ + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method() { + Set warm; + warm = Set.of(Color.RED); + } + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method() { + Set warm; + warm = EnumSet.of(Color.RED); + } + } """ - import java.util.EnumSet; - import java.util.Set; - - class Test { - public enum Color { - RED, GREEN, BLUE - } - public void method() { - Set warm; - warm = EnumSet.of(Color.RED); - } - } - """ - ), - 9 ) ); } @@ -114,22 +110,19 @@ public void method() { void dontChangeVarargs() { //language=java rewriteRun( - version( - java( + java( + """ + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method(final Color... colors) { + Set s = Set.of(colors); + } + } """ - import java.util.Set; - - class Test { - public enum Color { - RED, GREEN, BLUE - } - public void method(final Color... colors) { - Set s = Set.of(colors); - } - } - """ - ), - 9 ) ); } @@ -139,23 +132,20 @@ public void method(final Color... colors) { void dontChangeArray() { //language=java rewriteRun( - version( - java( + java( + """ + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method() { + Color[] colors = {}; + Set s = Set.of(colors); + } + } """ - import java.util.Set; - - class Test { - public enum Color { - RED, GREEN, BLUE - } - public void method() { - Color[] colors = {}; - Set s = Set.of(colors); - } - } - """ - ), - 9 ) ); } @@ -165,32 +155,52 @@ public void method() { void dontHaveArgs() { //language=java rewriteRun( - version( - java( + java( + """ + import java.util.Set; + import java.util.concurrent.TimeUnit; + class Test { + + public void method() { + Set warm = Set.of(); + } + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + import java.util.concurrent.TimeUnit; + + class Test { + + public void method() { + Set warm = EnumSet.noneOf(TimeUnit.class); + } + } """ - import java.util.Set; - import java.util.concurrent.TimeUnit; - class Test { - - public void method() { - Set warm = Set.of(); - } - } - """, + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/958") + @Test + void retainEmptySetOf() { + //language=java + rewriteRun( + spec -> spec.recipe(new UseEnumSetOf(false)), + java( + """ + import java.util.Set; + + class Test { + public enum Color { + RED, GREEN, BLUE + } + public void method() { + Set warm = Set.of(); + } + } """ - import java.util.EnumSet; - import java.util.Set; - import java.util.concurrent.TimeUnit; - - class Test { - - public void method() { - Set warm = EnumSet.noneOf(TimeUnit.class); - } - } - """ - ), - 9 ) ); }