8290041: ModuleDescriptor.hashCode is inconsistent

Reviewed-by: alanb

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

@@ -2627,7 +2627,7 @@ private static <M> String toString(Set<M> mods, String what) {
     private static int modsHashCode(Iterable<? extends Enum<?>> enums) {
         int h = 0;
         for (Enum<?> e : enums) {
-            h = h * 43 + Objects.hashCode(e.name());
+            h += e.name().hashCode();
         }
         return h;
     }

test/jdk/java/lang/module/ModuleDescriptorHashCodeTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -21,22 +21,24 @@
  * questions.
  */
 
-import org.testng.annotations.Test;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.module.ModuleDescriptor;
+import java.lang.module.ModuleDescriptor.Exports;
+import java.lang.module.ModuleDescriptor.Opens;
+import java.lang.module.ModuleDescriptor.Requires;
 import java.util.Set;
 
+import org.testng.annotations.Test;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotSame;
 
 /**
  * @test
- * @bug 8275509
+ * @bug 8275509 8290041
+ * @summary Tests the ModuleDescriptor.hashCode()
  * @run testng ModuleDescriptorHashCodeTest
  * @run testng/othervm -Xshare:off ModuleDescriptorHashCodeTest
- * @summary Tests the ModuleDescriptor.hashCode() for boot layer modules
  */
 public class ModuleDescriptorHashCodeTest {
 
@@ -63,6 +65,99 @@ public void testBootModuleDescriptor() throws Exception {
         }
     }
 
+    /**
+     * Verifies that two "equal" module descriptors which only differ in the order of
+     * {@link ModuleDescriptor.Opens.Modifier opens modifiers}, that were used to construct the
+     * descriptors, have the same hashcode.
+     */
+    @Test
+    public void testOpensModifiersOrdering() throws Exception {
+        // important to use Set.of() (i.e. backed by immutable set) to reproduce the issue
+        final Set<Opens.Modifier> mods1 = Set.of(Opens.Modifier.SYNTHETIC, Opens.Modifier.MANDATED);
+        final ModuleDescriptor desc1 = createModuleDescriptor(mods1, null, null);
+
+        // create the same module descriptor again and this time just change the order of the
+        // "opens" modifiers' Set.
+
+        // important to use Set.of() (i.e. backed by immutable set) to reproduce the issue
+        final Set<Opens.Modifier> mods2 = Set.of(Opens.Modifier.MANDATED, Opens.Modifier.SYNTHETIC);
+        final ModuleDescriptor desc2 = createModuleDescriptor(mods2, null, null);
+
+        // basic verification of the modifiers themselves before we check the module descriptors
+        assertEquals(mods1, mods2, "Modifiers were expected to be equal");
+
+        // now verify the module descriptors
+        assertEquals(desc1, desc2, "Module descriptors were expected to be equal");
+        assertEquals(desc1.compareTo(desc2), 0, "compareTo was expected to return" +
+                " 0 for module descriptors that are equal");
+        System.out.println(desc1 + " hashcode = " + desc1.hashCode());
+        System.out.println(desc2 + " hashcode = " + desc2.hashCode());
+        assertEquals(desc1.hashCode(), desc2.hashCode(), "Module descriptor hashcodes" +
+                " were expected to be equal");
+    }
+
+    /**
+     * Verifies that two "equal" module descriptors which only differ in the order of
+     * {@link ModuleDescriptor.Exports.Modifier exports modifiers}, that were used to construct the
+     * descriptors, have the same hashcode.
+     */
+    @Test
+    public void testExportsModifiersOrdering() throws Exception {
+        // important to use Set.of() (i.e. backed by immutable set) to reproduce the issue
+        final Set<Exports.Modifier> mods1 = Set.of(Exports.Modifier.SYNTHETIC, Exports.Modifier.MANDATED);
+        final ModuleDescriptor desc1 = createModuleDescriptor(null, null, mods1);
+
+        // create the same module descriptor again and this time just change the order of the
+        // "exports" modifiers' Set.
+
+        // important to use Set.of() (i.e. backed by immutable set) to reproduce the issue
+        final Set<Exports.Modifier> mods2 = Set.of(Exports.Modifier.MANDATED, Exports.Modifier.SYNTHETIC);
+        final ModuleDescriptor desc2 = createModuleDescriptor(null, null, mods2);
+
+        // basic verification of the modifiers themselves before we check the module descriptors
+        assertEquals(mods1, mods2, "Modifiers were expected to be equal");
+
+        // now verify the module descriptors
+        assertEquals(desc1, desc2, "Module descriptors were expected to be equal");
+        assertEquals(desc1.compareTo(desc2), 0, "compareTo was expected to return" +
+                " 0 for module descriptors that are equal");
+        System.out.println(desc1 + " hashcode = " + desc1.hashCode());
+        System.out.println(desc2 + " hashcode = " + desc2.hashCode());
+        assertEquals(desc1.hashCode(), desc2.hashCode(), "Module descriptor hashcodes" +
+                " were expected to be equal");
+    }
+
+    /**
+     * Verifies that two "equal" module descriptors which only differ in the order of
+     * {@link ModuleDescriptor.Requires.Modifier requires modifiers}, that were used to construct the
+     * descriptors, have the same hashcode.
+     */
+    @Test
+    public void testRequiresModifiersOrdering() throws Exception {
+        // important to use Set.of() (i.e. backed by immutable set) to reproduce the issue
+        final Set<Requires.Modifier> mods1 = Set.of(Requires.Modifier.SYNTHETIC, Requires.Modifier.MANDATED);
+        final ModuleDescriptor desc1 = createModuleDescriptor(null, mods1, null);
+
+        // create the same module descriptor again and this time just change the order of the
+        // "exports" modifiers' Set.
+
+        // important to use Set.of() (i.e. backed by immutable set) to reproduce the issue
+        final Set<Requires.Modifier> mods2 = Set.of(Requires.Modifier.MANDATED, Requires.Modifier.SYNTHETIC);
+        final ModuleDescriptor desc2 = createModuleDescriptor(null, mods2, null);
+
+        // basic verification of the modifiers themselves before we check the module descriptors
+        assertEquals(mods1, mods2, "Modifiers were expected to be equal");
+
+        // now verify the module descriptors
+        assertEquals(desc1, desc2, "Module descriptors were expected to be equal");
+        assertEquals(desc1.compareTo(desc2), 0, "compareTo was expected to return" +
+                " 0 for module descriptors that are equal");
+        System.out.println(desc1 + " hashcode = " + desc1.hashCode());
+        System.out.println(desc2 + " hashcode = " + desc2.hashCode());
+        assertEquals(desc1.hashCode(), desc2.hashCode(), "Module descriptor hashcodes" +
+                " were expected to be equal");
+    }
+
     // Returns a ModuleDescriptor parsed out of the module-info.class of the passed Module
     private static ModuleDescriptor fromModuleInfoClass(Module module) throws IOException {
         try (InputStream moduleInfo = module.getResourceAsStream("module-info.class")) {
@@ -73,4 +168,23 @@ private static ModuleDescriptor fromModuleInfoClass(Module module) throws IOExce
             return ModuleDescriptor.read(moduleInfo);
         }
     }
+
+    // creates a module descriptor with passed (optional) opens/exports/requires modifiers
+    private static ModuleDescriptor createModuleDescriptor(
+            Set<Opens.Modifier> opensModifiers,
+            Set<Requires.Modifier> reqsModifiers,
+            Set<Exports.Modifier> expsModifiers) {
+
+        final ModuleDescriptor.Builder builder = ModuleDescriptor.newModule("foobar");
+        if (opensModifiers != null) {
+            builder.opens(opensModifiers, "a.p1", Set.of("a.m1"));
+        }
+        if (reqsModifiers != null) {
+            builder.requires(reqsModifiers, "a.m2");
+        }
+        if (expsModifiers != null) {
+            builder.exports(expsModifiers, "a.b.c", Set.of("a.m3"));
+        }
+        return builder.build();
+    }
 }