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 07bf1a8b70..02d692f0af 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/UseEnumSetOf.java +++ b/src/main/java/org/openrewrite/java/migrate/util/UseEnumSetOf.java @@ -22,16 +22,22 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.VariableNameUtils; import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.service.ImportService; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; import java.time.Duration; +import java.util.Iterator; import java.util.List; import java.util.StringJoiner; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.util.Objects.requireNonNull; @EqualsAndHashCode(callSuper = false) @Value @@ -40,8 +46,8 @@ public class UseEnumSetOf extends Recipe { 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.", + displayName = "Convert empty `Set.of()` to an unmodifiable `EnumSet.noneOf()`", + description = "When true, converts `Set.of()` with no arguments to an unmodifiable `EnumSet.noneOf()`. Default true.", example = "true", required = false ) @@ -50,7 +56,7 @@ public class UseEnumSetOf extends Recipe { String displayName = "Prefer `EnumSet of(..)`"; - String description = "Prefer `EnumSet of(..)` instead of using `Set of(..)` when the arguments are enums in Java 9 or higher."; + String description = "Prefer an unmodifiable `EnumSet` instead of using `Set.of(..)` when the arguments are enums in Java 9 or higher."; Duration estimatedEffortPerOccurrence = Duration.ofMinutes( 2 ); @@ -70,6 +76,16 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat JavaType type = parent.getValue() instanceof J.Assignment ? ((J.Assignment) parent.getValue()).getType() : ((J.VariableDeclarations) parent.getValue()).getVariables().get(0).getType(); if (isAssignmentSetOfEnum(type)) { + boolean collectionsUnavailable = isNameUnavailable("Collections", "java.util.Collections"); + // a declaration named `java` in scope breaks the fully qualified `java.util.Collections` fallback + boolean fullyQualifiedFallbackShadowed = isNameUnavailable("java", null); + if (collectionsUnavailable && fullyQualifiedFallbackShadowed) { + return mi; + } + String collections = fullyQualifiedFallbackShadowed ? "Collections" : "java.util.Collections"; + if (fullyQualifiedFallbackShadowed) { + maybeAddImport("java.util.Collections"); + } maybeAddImport(METHOD_TYPE); List args = mi.getArguments(); @@ -83,20 +99,31 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat } JavaType firstTypeParameter = ((JavaType.Parameterized) type).getTypeParameters().get(0); JavaType.ShallowClass shallowClass = JavaType.ShallowClass.build(firstTypeParameter.toString()); - return JavaTemplate.builder("EnumSet.noneOf(" + shallowClass.getClassName() + ".class)") + J.MethodInvocation replacement = JavaTemplate.builder( + unmodifiableSet(collections, "EnumSet.noneOf(" + shallowClass.getClassName() + ".class)")) .contextSensitive() - .imports(METHOD_TYPE) + .imports("java.util.Collections", METHOD_TYPE) .build() .apply(updateCursor(mi), mi.getCoordinates().replace()); + if (!collectionsUnavailable) { + doAfterVisit(service(ImportService.class).shortenFullyQualifiedTypeReferencesIn( + requireNonNull(replacement.getSelect()))); + } + return replacement; } - StringJoiner setOf = new StringJoiner(", ", "EnumSet.of(", ")"); - args.forEach(o -> setOf.add("#{any()}")); - return JavaTemplate.builder(setOf.toString()) + StringJoiner setOfArguments = new StringJoiner(", "); + args.forEach(o -> setOfArguments.add("#{any()}")); + J.MethodInvocation replacement = JavaTemplate.builder(unmodifiableSet(collections, "EnumSet.of(" + setOfArguments + ")")) .contextSensitive() - .imports(METHOD_TYPE) + .imports("java.util.Collections", METHOD_TYPE) .build() .apply(updateCursor(mi), mi.getCoordinates().replace(), args.toArray()); + if (!collectionsUnavailable) { + doAfterVisit(service(ImportService.class).shortenFullyQualifiedTypeReferencesIn( + requireNonNull(replacement.getSelect()))); + } + return replacement; } } } @@ -135,7 +162,70 @@ private boolean isArrayParameter(final List args) { JavaType type = args.get(0).getType(); return TypeUtils.asArray(type) != null; } + + private boolean isNameUnavailable(String name, @Nullable String allowedImport) { + J.CompilationUnit compilationUnit = getCursor().firstEnclosingOrThrow(J.CompilationUnit.class); + boolean conflictingImport = compilationUnit.getImports().stream() + .filter(anImport -> !anImport.isStatic()) + .filter(anImport -> name.equals(anImport.getQualid().getSimpleName())) + .anyMatch(anImport -> allowedImport == null || !allowedImport.equals(anImport.getTypeName())); + if (conflictingImport) { + return true; + } + + AtomicBoolean typeDeclared = new AtomicBoolean(); + new JavaVisitor() { + @Override + public J visitClassDeclaration(J.ClassDeclaration classDecl, AtomicBoolean found) { + if (name.equals(classDecl.getSimpleName())) { + found.set(true); + return classDecl; + } + return super.visitClassDeclaration(classDecl, found); + } + + @Override + public J visitTypeParameter(J.TypeParameter typeParameter, AtomicBoolean found) { + if (typeParameter.getName() instanceof J.Identifier && + name.equals(((J.Identifier) typeParameter.getName()).getSimpleName())) { + found.set(true); + return typeParameter; + } + return super.visitTypeParameter(typeParameter, found); + } + }.visit(compilationUnit, typeDeclared); + + boolean visibleMember = false; + Iterator enclosingClasses = getCursor().getPathAsStream() + .filter(J.ClassDeclaration.class::isInstance) + .map(J.ClassDeclaration.class::cast) + .iterator(); + while (enclosingClasses.hasNext() && !visibleMember) { + JavaType.FullyQualified classType = TypeUtils.asFullyQualified(enclosingClasses.next().getType()); + if (classType != null) { + Iterator members = classType.getVisibleMembers(); + while (members.hasNext()) { + if (name.equals(members.next().getName())) { + visibleMember = true; + break; + } + } + } + } + + return typeDeclared.get() || visibleMember || + VariableNameUtils.findNamesInScope(getCursor()).contains(name) || + getCursor().getPathAsStream() + .filter(J.ClassDeclaration.class::isInstance) + .map(J.ClassDeclaration.class::cast) + .anyMatch(classDecl -> name.equals(classDecl.getSimpleName())); + } }); } + // Wraps the EnumSet expression so every paren opens and closes in one place + private static String unmodifiableSet(String collections, String enumSetExpression) { + return collections + ".unmodifiableSet(" + enumSetExpression + ")"; + } + } diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 1dfc38ed48..7e47f25bd1 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -9757,6 +9757,7 @@ examples: } } after: | + import java.util.Collections; import java.util.EnumSet; import java.util.Set; @@ -9765,7 +9766,7 @@ examples: RED, GREEN, BLUE } public void method() { - Set warm = EnumSet.of(Color.RED, Color.GREEN); + Set warm = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); } } language: java diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index dbb7a45637..9194bd4aca 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -494,7 +494,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.ReplaceStreamCollectWithToList,Replace `Stream.collect(Collectors.toUnmodifiableList())` with `Stream.toList()`,Replace `Stream.collect(Collectors.toUnmodifiableList())` with Java 16+ `Stream.toList()`. Also replaces `Stream.collect(Collectors.toList())` if `convertToList` is set to `true`.,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"":""convertToList"",""type"":""Boolean"",""displayName"":""Convert mutable `Collectors.toList()` to immutable"",""description"":""Also replace `Stream.collect(Collectors.toList())` with `Stream.toList()`. *BEWARE*: Attempts to modify the returned list, result in an `UnsupportedOperationException`!""}]", 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.UseEnumSetOf,Prefer `EnumSet of(..)`,Prefer an unmodifiable `EnumSet` 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 an unmodifiable `EnumSet.noneOf()`"",""description"":""When true, converts `Set.of()` with no arguments to an unmodifiable `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(..)` 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). 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 0dd3f47357..f5634d81ff 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/UseEnumSetOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/UseEnumSetOfTest.java @@ -53,6 +53,7 @@ public void method() { } """, """ + import java.util.Collections; import java.util.EnumSet; import java.util.Set; @@ -61,7 +62,7 @@ public enum Color { RED, GREEN, BLUE } public void method() { - Set warm = EnumSet.of(Color.RED, Color.GREEN); + Set warm = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); } } """ @@ -88,6 +89,7 @@ public void method() { } """, """ + import java.util.Collections; import java.util.EnumSet; import java.util.Set; @@ -97,7 +99,7 @@ public enum Color { } public void method() { Set warm; - warm = EnumSet.of(Color.RED); + warm = Collections.unmodifiableSet(EnumSet.of(Color.RED)); } } """ @@ -167,6 +169,7 @@ public void method() { } """, """ + import java.util.Collections; import java.util.EnumSet; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -174,7 +177,7 @@ public void method() { class Test { public void method() { - Set warm = EnumSet.noneOf(TimeUnit.class); + Set warm = Collections.unmodifiableSet(EnumSet.noneOf(TimeUnit.class)); } } """ @@ -243,4 +246,347 @@ public void method() { ) ); } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/958") + @Test + void retainNonEmptyImmutableSetOf() { + rewriteRun( + java( + """ + import java.util.Set; + + class Test { + enum Color { + RED, GREEN, BLUE + } + + static final Set CONSTANT = Set.of(Color.RED, Color.GREEN); + + Set local() { + Set colors = Set.of(Color.RED, Color.GREEN); + return colors; + } + } + """, + """ + import java.util.Collections; + import java.util.EnumSet; + import java.util.Set; + + class Test { + enum Color { + RED, GREEN, BLUE + } + + static final Set CONSTANT = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + + Set local() { + Set colors = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + return colors; + } + } + """ + ) + ); + } + + @Test + void fullyQualifyCollectionsWhenSimpleNameIsShadowed() { + rewriteRun( + java( + """ + import java.util.Set; + + class Collections { + } + + class Test { + enum Color { + RED, GREEN + } + + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + + class Collections { + } + + class Test { + enum Color { + RED, GREEN + } + + Set colors = java.util.Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ) + ); + } + + @Test + void fullyQualifyCollectionsWhenVariableWithSameNameIsInScope() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + + class Test { + enum Color { + RED, GREEN + } + + Object Collections; + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + + class Test { + enum Color { + RED, GREEN + } + + Object Collections; + Set colors = java.util.Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ) + ); + } + + @Test + void importCollectionsWhenJavaIsShadowed() { + rewriteRun( + java( + """ + import java.util.Set; + + class Test { + enum Color { + RED, GREEN + } + + Object java; + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.Collections; + import java.util.EnumSet; + import java.util.Set; + + class Test { + enum Color { + RED, GREEN + } + + Object java; + Set colors = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ) + ); + } + + @Test + void importCollectionsWhenNestedJavaTypeIsInScope() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + + class Test { + enum Color { + RED, GREEN + } + + static class java { + } + + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.Collections; + import java.util.EnumSet; + import java.util.Set; + + class Test { + enum Color { + RED, GREEN + } + + static class java { + } + + Set colors = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ) + ); + } + + @Test + void handleTypeParameterNameCollisions() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + + class CollectionsTypeParameterTest { + enum Color { + RED, GREEN + } + + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + + class CollectionsTypeParameterTest { + enum Color { + RED, GREEN + } + + Set colors = java.util.Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ), + //language=java + java( + """ + import java.util.Set; + + class JavaTypeParameterTest { + enum Color { + RED, GREEN + } + + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.Collections; + import java.util.EnumSet; + import java.util.Set; + + class JavaTypeParameterTest { + enum Color { + RED, GREEN + } + + Set colors = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ) + ); + } + + @Test + void handleInheritedFieldNameCollisions() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + + interface CollectionsShadow { + Object Collections = null; + } + + class CollectionsFieldTest implements CollectionsShadow { + enum Color { + RED, GREEN + } + + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.EnumSet; + import java.util.Set; + + interface CollectionsShadow { + Object Collections = null; + } + + class CollectionsFieldTest implements CollectionsShadow { + enum Color { + RED, GREEN + } + + Set colors = java.util.Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ), + //language=java + java( + """ + import java.util.Set; + + interface JavaShadow { + Object java = null; + } + + class JavaFieldTest implements JavaShadow { + enum Color { + RED, GREEN + } + + Set colors = Set.of(Color.RED, Color.GREEN); + } + """, + """ + import java.util.Collections; + import java.util.EnumSet; + import java.util.Set; + + interface JavaShadow { + Object java = null; + } + + class JavaFieldTest implements JavaShadow { + enum Color { + RED, GREEN + } + + Set colors = Collections.unmodifiableSet(EnumSet.of(Color.RED, Color.GREEN)); + } + """ + ) + ); + } + + @Test + void retainSetOfWhenBothCollectionsSpellingsAreShadowed() { + rewriteRun( + java( + """ + import java.util.Set; + + class Collections { + } + + class Test { + enum Color { + RED, GREEN + } + + Object java; + Set colors = Set.of(Color.RED, Color.GREEN); + } + """ + ) + ); + } }