8209003: Consolidate use of empty collections in java.lang.module

Reviewed-by: alanb, mchung

src/java.base/share/classes/java/lang/Module.java

@@ -40,7 +40,6 @@ import java.net.URI;
 import java.net.URL;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -1123,7 +1122,7 @@ public final class Module implements AnnotatedElement {
             Set<Module> reads = new HashSet<>();
 
             // name -> source Module when in parent layer
-            Map<String, Module> nameToSource = Collections.emptyMap();
+            Map<String, Module> nameToSource = Map.of();
 
             for (ResolvedModule other : resolvedModule.reads()) {
                 Module m2 = null;

src/java.base/share/classes/java/lang/ModuleLayer.java

@@ -173,7 +173,7 @@ public final class ModuleLayer {
 
         Map<String, Module> map;
         if (parents.isEmpty()) {
-            map = Collections.emptyMap();
+            map = Map.of();
         } else {
             map = Module.defineModules(cf, clf, this);
         }
@@ -811,8 +811,7 @@ public final class ModuleLayer {
     public Set<Module> modules() {
         Set<Module> modules = this.modules;
         if (modules == null) {
-            this.modules = modules =
+            this.modules = modules = Set.copyOf(nameToModule.values());
-                Collections.unmodifiableSet(new HashSet<>(nameToModule.values()));
         }
         return modules;
     }

src/java.base/share/classes/java/lang/module/Configuration.java

@@ -31,7 +31,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Deque;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -119,10 +118,10 @@ public final class Configuration {
     String targetPlatform() { return targetPlatform; }
 
     private Configuration() {
-        this.parents = Collections.emptyList();
+        this.parents = List.of();
-        this.graph = Collections.emptyMap();
+        this.graph = Map.of();
-        this.modules = Collections.emptySet();
+        this.modules = Set.of();
-        this.nameToModule = Collections.emptyMap();
+        this.nameToModule = Map.of();
         this.targetPlatform = null;
     }
 
@@ -140,7 +139,7 @@ public final class Configuration {
             i++;
         }
 
-        this.parents = Collections.unmodifiableList(parents);
+        this.parents = List.copyOf(parents);
         this.graph = g;
         this.modules = Set.of(moduleArray);
         this.nameToModule = Map.ofEntries(nameEntries);
@@ -554,7 +553,7 @@ public final class Configuration {
 
     Set<ModuleDescriptor> descriptors() {
         if (modules.isEmpty()) {
-            return Collections.emptySet();
+            return Set.of();
         } else {
             return modules.stream()
                     .map(ResolvedModule::reference)
@@ -596,7 +595,7 @@ public final class Configuration {
                     }
                 }
             }
-            this.allConfigurations = Collections.unmodifiableList(allConfigurations);
+            this.allConfigurations = allConfigurations; // no need to do defensive copy
         }
         return allConfigurations.stream();
     }

src/java.base/share/classes/java/lang/module/ModuleDescriptor.java

@@ -102,7 +102,7 @@ public class ModuleDescriptor
      * @since 9
      * @spec JPMS
      */
-    public static enum Modifier {
+    public enum Modifier {
         /**
          * An open module. An open module does not declare any open packages
          * but the resulting module is treated as if all packages are open.
@@ -149,7 +149,7 @@ public class ModuleDescriptor
          * @since 9
          * @spec JPMS
          */
-        public static enum Modifier {
+        public enum Modifier {
 
             /**
              * The dependence causes any module which depends on the <i>current
@@ -185,12 +185,7 @@ public class ModuleDescriptor
 
         private Requires(Set<Modifier> ms, String mn, Version v, String vs) {
             assert v == null || vs == null;
-            if (ms.isEmpty()) {
+            this.mods = Set.copyOf(ms);
-                ms = Collections.emptySet();
-            } else {
-                ms = Collections.unmodifiableSet(EnumSet.copyOf(ms));
-            }
-            this.mods = ms;
             this.name = mn;
             this.compiledVersion = v;
             this.rawCompiledVersion = vs;
@@ -384,7 +379,7 @@ public class ModuleDescriptor
          * @since 9
          * @spec JPMS
          */
-        public static enum Modifier {
+        public enum Modifier {
 
             /**
              * The export was not explicitly or implicitly declared in the
@@ -408,14 +403,9 @@ public class ModuleDescriptor
          * Constructs an export
          */
         private Exports(Set<Modifier> ms, String source, Set<String> targets) {
-            if (ms.isEmpty()) {
+            this.mods = Set.copyOf(ms);
-                ms = Collections.emptySet();
-            } else {
-                ms = Collections.unmodifiableSet(EnumSet.copyOf(ms));
-            }
-            this.mods = ms;
             this.source = source;
-            this.targets = emptyOrUnmodifiableSet(targets);
+            this.targets = Set.copyOf(targets);
         }
 
         private Exports(Set<Modifier> ms,
@@ -596,7 +586,7 @@ public class ModuleDescriptor
          * @since 9
          * @spec JPMS
          */
-        public static enum Modifier {
+        public enum Modifier {
 
             /**
              * The open package was not explicitly or implicitly declared in
@@ -620,14 +610,9 @@ public class ModuleDescriptor
          * Constructs an Opens
          */
         private Opens(Set<Modifier> ms, String source, Set<String> targets) {
-            if (ms.isEmpty()) {
+            this.mods = Set.copyOf(ms);
-                ms = Collections.emptySet();
-            } else {
-                ms = Collections.unmodifiableSet(EnumSet.copyOf(ms));
-            }
-            this.mods = ms;
             this.source = source;
-            this.targets = emptyOrUnmodifiableSet(targets);
+            this.targets = Set.copyOf(targets);
         }
 
         private Opens(Set<Modifier> ms,
@@ -800,7 +785,7 @@ public class ModuleDescriptor
 
         private Provides(String service, List<String> providers) {
             this.service = service;
-            this.providers = Collections.unmodifiableList(providers);
+            this.providers = List.copyOf(providers);
         }
 
         private Provides(String service, List<String> providers, boolean unused) {
@@ -1264,18 +1249,18 @@ public class ModuleDescriptor
         this.name = name;
         this.version = version;
         this.rawVersionString = rawVersionString;
-        this.modifiers = emptyOrUnmodifiableSet(modifiers);
+        this.modifiers = Set.copyOf(modifiers);
         this.open = modifiers.contains(Modifier.OPEN);
         this.automatic = modifiers.contains(Modifier.AUTOMATIC);
         assert (requires.stream().map(Requires::name).distinct().count()
                 == requires.size());
-        this.requires = emptyOrUnmodifiableSet(requires);
+        this.requires = Set.copyOf(requires);
-        this.exports = emptyOrUnmodifiableSet(exports);
+        this.exports = Set.copyOf(exports);
-        this.opens = emptyOrUnmodifiableSet(opens);
+        this.opens = Set.copyOf(opens);
-        this.uses = emptyOrUnmodifiableSet(uses);
+        this.uses = Set.copyOf(uses);
-        this.provides = emptyOrUnmodifiableSet(provides);
+        this.provides = Set.copyOf(provides);
 
-        this.packages = emptyOrUnmodifiableSet(packages);
+        this.packages = Set.copyOf(packages);
         this.mainClass = mainClass;
     }
 
@@ -1734,16 +1719,14 @@ public class ModuleDescriptor
                                String pn,
                                Set<String> targets)
         {
-            Exports e = new Exports(ms, pn, targets);
+            targets = new HashSet<>(targets);
-
-            // check targets
-            targets = e.targets();
             if (targets.isEmpty())
                 throw new IllegalArgumentException("Empty target set");
             if (strict) {
-                requirePackageName(e.source());
+                requirePackageName(pn);
                 targets.forEach(Checks::requireModuleName);
             }
+            Exports e = new Exports(ms, pn, targets);
             return exports(e);
         }
 
@@ -1769,7 +1752,7 @@ public class ModuleDescriptor
             if (strict) {
                 requirePackageName(pn);
             }
-            Exports e = new Exports(ms, pn, Collections.emptySet());
+            Exports e = new Exports(ms, pn, Set.of());
             return exports(e);
         }
 
@@ -1794,7 +1777,7 @@ public class ModuleDescriptor
          *         or this builder is for an automatic module
          */
         public Builder exports(String pn, Set<String> targets) {
-            return exports(Collections.emptySet(), pn, targets);
+            return exports(Set.of(), pn, targets);
         }
 
         /**
@@ -1813,7 +1796,7 @@ public class ModuleDescriptor
          *         or this builder is for an automatic module
          */
         public Builder exports(String pn) {
-            return exports(Collections.emptySet(), pn);
+            return exports(Set.of(), pn);
         }
 
         /**
@@ -1870,16 +1853,14 @@ public class ModuleDescriptor
                              String pn,
                              Set<String> targets)
         {
-            Opens opens = new Opens(ms, pn, targets);
+            targets = new HashSet<>(targets);
-
-            // check targets
-            targets = opens.targets();
             if (targets.isEmpty())
                 throw new IllegalArgumentException("Empty target set");
             if (strict) {
-                requirePackageName(opens.source());
+                requirePackageName(pn);
                 targets.forEach(Checks::requireModuleName);
             }
+            Opens opens = new Opens(ms, pn, targets);
             return opens(opens);
         }
 
@@ -1905,7 +1886,7 @@ public class ModuleDescriptor
             if (strict) {
                 requirePackageName(pn);
             }
-            Opens e = new Opens(ms, pn, Collections.emptySet());
+            Opens e = new Opens(ms, pn, Set.of());
             return opens(e);
         }
 
@@ -1929,7 +1910,7 @@ public class ModuleDescriptor
          *         builder for an open module or automatic module
          */
         public Builder opens(String pn, Set<String> targets) {
-            return opens(Collections.emptySet(), pn, targets);
+            return opens(Set.of(), pn, targets);
         }
 
         /**
@@ -1948,7 +1929,7 @@ public class ModuleDescriptor
          *         builder for an open module or automatic module
          */
         public Builder opens(String pn) {
-            return opens(Collections.emptySet(), pn);
+            return opens(Set.of(), pn);
         }
 
         /**
@@ -2021,15 +2002,12 @@ public class ModuleDescriptor
          *         declared
          */
         public Builder provides(String service, List<String> providers) {
-            Provides p = new Provides(service, providers);
+            providers = new ArrayList<>(providers);
-
+            if (providers.isEmpty())
-            // check providers after the set has been copied.
-            List<String> providerNames = p.providers();
-            if (providerNames.isEmpty())
                 throw new IllegalArgumentException("Empty providers set");
             if (strict) {
-                requireServiceTypeName(p.service());
+                requireServiceTypeName(service);
-                providerNames.forEach(Checks::requireServiceProviderName);
+                providers.forEach(Checks::requireServiceProviderName);
             } else {
                 // Disallow service/providers in unnamed package
                 String pn = packageName(service);
@@ -2037,7 +2015,7 @@ public class ModuleDescriptor
                     throw new IllegalArgumentException(service
                                                        + ": unnamed package");
                 }
-                for (String name : providerNames) {
+                for (String name : providers) {
                     pn = packageName(name);
                     if (pn.isEmpty()) {
                         throw new IllegalArgumentException(name
@@ -2045,6 +2023,7 @@ public class ModuleDescriptor
                     }
                 }
             }
+            Provides p = new Provides(service, providers);
             return provides(p);
         }
 
@@ -2574,27 +2553,6 @@ public class ModuleDescriptor
         return ModuleInfo.read(bb, null).descriptor();
     }
 
-    private static <K,V> Map<K,V> emptyOrUnmodifiableMap(Map<K,V> map) {
-        if (map.isEmpty()) {
-            return Collections.emptyMap();
-        } else if (map.size() == 1) {
-            Map.Entry<K, V> entry = map.entrySet().iterator().next();
-            return Collections.singletonMap(entry.getKey(), entry.getValue());
-        } else {
-            return Collections.unmodifiableMap(map);
-        }
-    }
-
-    private static <T> Set<T> emptyOrUnmodifiableSet(Set<T> set) {
-        if (set.isEmpty()) {
-            return Collections.emptySet();
-        } else if (set.size() == 1) {
-            return Collections.singleton(set.iterator().next());
-        } else {
-            return Collections.unmodifiableSet(set);
-        }
-    }
-
     private static String packageName(String cn) {
         int index = cn.lastIndexOf('.');
         return (index == -1) ? "" : cn.substring(0, index);
@@ -2674,7 +2632,7 @@ public class ModuleDescriptor
 
                 @Override
                 public Exports newExports(Set<Exports.Modifier> ms, String source) {
-                    return new Exports(ms, source, Collections.emptySet(), true);
+                    return new Exports(ms, source, Set.of(), true);
                 }
 
                 @Override
@@ -2693,7 +2651,7 @@ public class ModuleDescriptor
 
                 @Override
                 public Opens newOpens(Set<Opens.Modifier> ms, String source) {
-                    return new Opens(ms, source, Collections.emptySet(), true);
+                    return new Opens(ms, source, Set.of(), true);
                 }
 
                 @Override

src/java.base/share/classes/java/lang/module/ModuleFinder.java

@@ -306,7 +306,7 @@ public interface ModuleFinder {
 
                 @Override
                 public Set<ModuleReference> findAll() {
-                    return Collections.emptySet();
+                    return Set.of();
                 }
             };
         }

src/java.base/share/classes/jdk/internal/module/Builder.java

@@ -30,7 +30,6 @@ import java.lang.module.ModuleDescriptor.Opens;
 import java.lang.module.ModuleDescriptor.Provides;
 import java.lang.module.ModuleDescriptor.Requires;
 import java.lang.module.ModuleDescriptor.Version;
-import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
@@ -148,11 +147,11 @@ final class Builder {
 
     Builder(String name) {
         this.name = name;
-        this.requires = Collections.emptySet();
+        this.requires = Set.of();
-        this.exports = Collections.emptySet();
+        this.exports = Set.of();
-        this.opens = Collections.emptySet();
+        this.opens = Set.of();
-        this.provides = Collections.emptySet();
+        this.provides = Set.of();
-        this.uses = Collections.emptySet();
+        this.uses = Set.of();
     }
 
     Builder open(boolean value) {
@@ -253,7 +252,7 @@ final class Builder {
         if (synthetic) n++;
         if (mandated) n++;
         if (n == 0) {
-            return Collections.emptySet();
+            return Set.of();
         } else {
             ModuleDescriptor.Modifier[] mods = new ModuleDescriptor.Modifier[n];
             if (open) mods[--n] = ModuleDescriptor.Modifier.OPEN;

src/java.base/share/classes/jdk/internal/module/ExplodedSystemModules.java

@@ -26,7 +26,6 @@
 package jdk.internal.module;
 
 import java.lang.module.ModuleDescriptor;
-import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
 
@@ -72,11 +71,11 @@ class ExplodedSystemModules implements SystemModules {
 
     @Override
     public  Map<String, Set<String>> concealedPackagesToOpen() {
-        return Collections.emptyMap();
+        return Map.of();
     }
 
     @Override
     public  Map<String, Set<String>> exportedPackagesToOpen() {
-        return Collections.emptyMap();
+        return Map.of();
     }
 }

src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java

@@ -568,7 +568,7 @@ public final class ModuleBootstrap {
         // the system property is removed after decoding
         String value = getAndRemoveProperty(prefix + index);
         if (value == null) {
-            return Collections.emptySet();
+            return Set.of();
         } else {
             Set<String> modules = new HashSet<>();
             while (value != null) {
@@ -588,7 +588,7 @@ public final class ModuleBootstrap {
     private static Set<String> limitModules() {
         String value = getAndRemoveProperty("jdk.module.limitmods");
         if (value == null) {
-            return Collections.emptySet();
+            return Set.of();
         } else {
             Set<String> names = new HashSet<>();
             for (String name : value.split(",")) {
@@ -840,7 +840,7 @@ public final class ModuleBootstrap {
         // the system property is removed after decoding
         String value = getAndRemoveProperty(prefix + index);
         if (value == null)
-            return Collections.emptyMap();
+            return Map.of();
 
         Map<String, List<String>> map = new HashMap<>();
 

src/java.base/share/classes/jdk/internal/module/ModuleInfo.java

@@ -40,7 +40,6 @@ import java.lang.module.ModuleDescriptor.Opens;
 import java.nio.ByteBuffer;
 import java.nio.BufferUnderflowException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -374,7 +373,7 @@ public final class ModuleInfo {
             int requires_flags = in.readUnsignedShort();
             Set<Requires.Modifier> mods;
             if (requires_flags == 0) {
-                mods = Collections.emptySet();
+                mods = Set.of();
             } else {
                 mods = new HashSet<>();
                 if ((requires_flags & ACC_TRANSITIVE) != 0)
@@ -430,7 +429,7 @@ public final class ModuleInfo {
                 Set<Exports.Modifier> mods;
                 int exports_flags = in.readUnsignedShort();
                 if (exports_flags == 0) {
-                    mods = Collections.emptySet();
+                    mods = Set.of();
                 } else {
                     mods = new HashSet<>();
                     if ((exports_flags & ACC_SYNTHETIC) != 0)
@@ -470,7 +469,7 @@ public final class ModuleInfo {
                 Set<Opens.Modifier> mods;
                 int opens_flags = in.readUnsignedShort();
                 if (opens_flags == 0) {
-                    mods = Collections.emptySet();
+                    mods = Set.of();
                 } else {
                     mods = new HashSet<>();
                     if ((opens_flags & ACC_SYNTHETIC) != 0)

src/java.base/share/classes/jdk/internal/module/ModulePatcher.java

@@ -42,7 +42,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -78,7 +77,7 @@ public final class ModulePatcher {
      */
     public ModulePatcher(Map<String, List<String>> input) {
         if (input.isEmpty()) {
-            this.map = Collections.emptyMap();
+            this.map = Map.of();
         } else {
             Map<String, List<Path>> map = new HashMap<>();
             for (Map.Entry<String, List<String>> e : input.entrySet()) {

src/java.base/share/classes/jdk/internal/module/ModulePath.java

@@ -45,7 +45,6 @@ import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -217,7 +216,7 @@ public class ModulePath implements ModuleFinder {
         try {
             attrs = Files.readAttributes(entry, BasicFileAttributes.class);
         } catch (NoSuchFileException e) {
-            return Collections.emptyMap();
+            return Map.of();
         } catch (IOException ioe) {
             throw new FindException(ioe);
         }
@@ -236,7 +235,7 @@ public class ModulePath implements ModuleFinder {
             ModuleReference mref = readModule(entry, attrs);
             if (mref != null) {
                 String name = mref.descriptor().name();
-                return Collections.singletonMap(name, mref);
+                return Map.of(name, mref);
             }
 
             // not recognized

src/java.base/share/classes/jdk/internal/module/ServicesCatalog.java

@@ -28,7 +28,6 @@ package jdk.internal.module;
 import java.lang.module.ModuleDescriptor;
 import java.lang.module.ModuleDescriptor.Provides;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -147,7 +146,7 @@ public final class ServicesCatalog {
      * the given service type.
      */
     public List<ServiceProvider> findServices(String service) {
-        return map.getOrDefault(service, Collections.emptyList());
+        return map.getOrDefault(service, List.of());
     }
 
     /**
@@ -175,4 +174,4 @@ public final class ServicesCatalog {
 
     // the ServicesCatalog registered to a class loader
     private static final ClassLoaderValue<ServicesCatalog> CLV = new ClassLoaderValue<>();
-}
+}

src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java

@@ -282,8 +282,8 @@ public final class SystemModuleFinders {
 
         SystemModuleFinder(Set<ModuleReference> mrefs,
                            Map<String, ModuleReference> nameToModule) {
-            this.mrefs = Collections.unmodifiableSet(mrefs);
+            this.mrefs = Set.copyOf(mrefs);
-            this.nameToModule = Collections.unmodifiableMap(nameToModule);
+            this.nameToModule = Map.copyOf(nameToModule);
         }
 
         @Override
@@ -353,7 +353,7 @@ public final class SystemModuleFinders {
                 }
             }
         }
-        return (nameToHash != null) ? nameToHash : Collections.emptyMap();
+        return (nameToHash != null) ? nameToHash : Map.of();
     }
 
     /**

test/jdk/java/lang/module/ModuleDescriptorTest.java

@@ -147,7 +147,12 @@ public class ModuleDescriptorTest {
     public void testRequiresWithRequires() {
         Requires r1 = requires("foo");
         ModuleDescriptor descriptor = ModuleDescriptor.newModule("m").requires(r1).build();
-        Requires r2 = descriptor.requires().iterator().next();
+        assertEquals(descriptor.requires().size(), 2);
+        var iterator = descriptor.requires().iterator();
+        Requires r2 = iterator.next();
+        if (r2.name().equals("java.base")) {
+            r2 = iterator.next();
+        }
         assertEquals(r1, r2);
     }