Under J2CL+GWT, remove keyType() and valueType() from the Enum*BiMap classes.

cpovirk · Google Java Core Libraries · commit 3de12be516dd · 2023-05-26T14:15:08.000-07:00
Previously, the methods were present but sometimes returned `null` Under J2CL. And soon, J2CL may drop support for the reflective enum-related APIs that they depend on.

`guava-gwt` is used by both GWT and J2CL users, so we have to keep our code buildable against both.

RELNOTES=`collect`: Removed the `Enum*BiMap` classes's `keyType()` and `valueType()` methods from `guava-gwt` to prepare for the removal of reflective enum-related APIs from J2CL. If this causes you problems as a GWT user, let us know.
PiperOrigin-RevId: 535711221
diff --git a/android/guava-tests/test/com/google/common/collect/EnumBiMapTest.java b/android/guava-tests/test/com/google/common/collect/EnumBiMapTest.java
@@ -183,11 +183,13 @@ public void testEnumBiMapConstructor() {
     assertEquals(bimap3, emptyBimap);
   }
 
+  @GwtIncompatible // keyType
   public void testKeyType() {
     EnumBiMap<Currency, Country> bimap = EnumBiMap.create(Currency.class, Country.class);
     assertEquals(Currency.class, bimap.keyType());
   }
 
+  @GwtIncompatible // valueType
   public void testValueType() {
     EnumBiMap<Currency, Country> bimap = EnumBiMap.create(Currency.class, Country.class);
     assertEquals(Country.class, bimap.valueType());
diff --git a/android/guava-tests/test/com/google/common/collect/EnumHashBiMapTest.java b/android/guava-tests/test/com/google/common/collect/EnumHashBiMapTest.java
@@ -197,6 +197,7 @@ public void testEnumBiMapConstructor() {
     assertEquals(bimap3, emptyBimap);
   }
 
+  @GwtIncompatible // keyType
   public void testKeyType() {
     EnumHashBiMap<Currency, String> bimap = EnumHashBiMap.create(Currency.class);
     assertEquals(Currency.class, bimap.keyType());
diff --git a/android/guava/src/com/google/common/collect/EnumBiMap.java b/android/guava/src/com/google/common/collect/EnumBiMap.java
@@ -18,6 +18,7 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Platform.getDeclaringClassOrObjectForJ2cl;
 
 import com.google.common.annotations.GwtCompatible;
 import com.google.common.annotations.GwtIncompatible;
@@ -42,8 +43,22 @@
 @J2ktIncompatible
 @ElementTypesAreNonnullByDefault
 public final class EnumBiMap<K extends Enum<K>, V extends Enum<V>> extends AbstractBiMap<K, V> {
-  private transient Class<K> keyType;
-  private transient Class<V> valueType;
+  /*
+   * J2CL's EnumMap does not need the Class instance, so we can use Object.class instead. (Or we
+   * could use null, but that messes with our nullness checking, including under J2KT. We could
+   * probably work around it by changing how we annotate the J2CL EnumMap, but that's probably more
+   * trouble than just using Object.class.)
+   *
+   * Then we declare the getters for these fields as @GwtIncompatible so that no one can try to use
+   * them under J2CL—or, as an unfortunate side effect, under GWT. We do still give the fields
+   * themselves their proper values under GWT, since GWT's EnumMap does need the Class instance.
+   *
+   * Note that sometimes these fields *do* have correct values under J2CL: They will if the caller
+   * calls `create(Foo.class)`, rather than `create(map)`. That's fine; we just shouldn't rely on
+   * it.
+   */
+  transient Class<K> keyTypeOrObjectUnderJ2cl;
+  transient Class<V> valueTypeOrObjectUnderJ2cl;
 
   /**
    * Returns a new, empty {@code EnumBiMap} using the specified key and value types.
@@ -66,44 +81,48 @@ public static <K extends Enum<K>, V extends Enum<V>> EnumBiMap<K, V> create(
    *     mappings
    */
   public static <K extends Enum<K>, V extends Enum<V>> EnumBiMap<K, V> create(Map<K, V> map) {
-    EnumBiMap<K, V> bimap = create(inferKeyType(map), inferValueType(map));
+    EnumBiMap<K, V> bimap =
+        create(inferKeyTypeOrObjectUnderJ2cl(map), inferValueTypeOrObjectUnderJ2cl(map));
     bimap.putAll(map);
     return bimap;
   }
 
-  private EnumBiMap(Class<K> keyType, Class<V> valueType) {
-    super(new EnumMap<K, V>(keyType), new EnumMap<V, K>(valueType));
-    this.keyType = keyType;
-    this.valueType = valueType;
+  private EnumBiMap(Class<K> keyTypeOrObjectUnderJ2cl, Class<V> valueTypeOrObjectUnderJ2cl) {
+    super(
+        new EnumMap<K, V>(keyTypeOrObjectUnderJ2cl), new EnumMap<V, K>(valueTypeOrObjectUnderJ2cl));
+    this.keyTypeOrObjectUnderJ2cl = keyTypeOrObjectUnderJ2cl;
+    this.valueTypeOrObjectUnderJ2cl = valueTypeOrObjectUnderJ2cl;
   }
 
-  static <K extends Enum<K>> Class<K> inferKeyType(Map<K, ?> map) {
+  static <K extends Enum<K>> Class<K> inferKeyTypeOrObjectUnderJ2cl(Map<K, ?> map) {
     if (map instanceof EnumBiMap) {
-      return ((EnumBiMap<K, ?>) map).keyType();
+      return ((EnumBiMap<K, ?>) map).keyTypeOrObjectUnderJ2cl;
     }
     if (map instanceof EnumHashBiMap) {
-      return ((EnumHashBiMap<K, ?>) map).keyType();
+      return ((EnumHashBiMap<K, ?>) map).keyTypeOrObjectUnderJ2cl;
     }
     checkArgument(!map.isEmpty());
-    return map.keySet().iterator().next().getDeclaringClass();
+    return getDeclaringClassOrObjectForJ2cl(map.keySet().iterator().next());
   }
 
-  private static <V extends Enum<V>> Class<V> inferValueType(Map<?, V> map) {
+  private static <V extends Enum<V>> Class<V> inferValueTypeOrObjectUnderJ2cl(Map<?, V> map) {
     if (map instanceof EnumBiMap) {
-      return ((EnumBiMap<?, V>) map).valueType;
+      return ((EnumBiMap<?, V>) map).valueTypeOrObjectUnderJ2cl;
     }
     checkArgument(!map.isEmpty());
-    return map.values().iterator().next().getDeclaringClass();
+    return getDeclaringClassOrObjectForJ2cl(map.values().iterator().next());
   }
 
   /** Returns the associated key type. */
+  @GwtIncompatible
   public Class<K> keyType() {
-    return keyType;
+    return keyTypeOrObjectUnderJ2cl;
   }
 
   /** Returns the associated value type. */
+  @GwtIncompatible
   public Class<V> valueType() {
-    return valueType;
+    return valueTypeOrObjectUnderJ2cl;
   }
 
   @Override
@@ -123,18 +142,19 @@ V checkValue(V value) {
   @GwtIncompatible // java.io.ObjectOutputStream
   private void writeObject(ObjectOutputStream stream) throws IOException {
     stream.defaultWriteObject();
-    stream.writeObject(keyType);
-    stream.writeObject(valueType);
+    stream.writeObject(keyTypeOrObjectUnderJ2cl);
+    stream.writeObject(valueTypeOrObjectUnderJ2cl);
     Serialization.writeMap(this, stream);
   }
 
   @SuppressWarnings("unchecked") // reading fields populated by writeObject
   @GwtIncompatible // java.io.ObjectInputStream
   private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException {
     stream.defaultReadObject();
-    keyType = (Class<K>) stream.readObject();
-    valueType = (Class<V>) stream.readObject();
-    setDelegates(new EnumMap<K, V>(keyType), new EnumMap<V, K>(valueType));
+    keyTypeOrObjectUnderJ2cl = (Class<K>) stream.readObject();
+    valueTypeOrObjectUnderJ2cl = (Class<V>) stream.readObject();
+    setDelegates(
+        new EnumMap<K, V>(keyTypeOrObjectUnderJ2cl), new EnumMap<V, K>(valueTypeOrObjectUnderJ2cl));
     Serialization.populateMap(this, stream);
   }
 
diff --git a/android/guava/src/com/google/common/collect/EnumHashBiMap.java b/android/guava/src/com/google/common/collect/EnumHashBiMap.java
@@ -47,7 +47,7 @@
 @ElementTypesAreNonnullByDefault
 public final class EnumHashBiMap<K extends Enum<K>, V extends @Nullable Object>
     extends AbstractBiMap<K, V> {
-  private transient Class<K> keyType;
+  transient Class<K> keyTypeOrObjectUnderJ2cl;
 
   /**
    * Returns a new, empty {@code EnumHashBiMap} using the specified key type.
@@ -71,16 +71,15 @@ public final class EnumHashBiMap<K extends Enum<K>, V extends @Nullable Object>
    */
   public static <K extends Enum<K>, V extends @Nullable Object> EnumHashBiMap<K, V> create(
       Map<K, ? extends V> map) {
-    EnumHashBiMap<K, V> bimap = create(EnumBiMap.inferKeyType(map));
+    EnumHashBiMap<K, V> bimap = create(EnumBiMap.inferKeyTypeOrObjectUnderJ2cl(map));
     bimap.putAll(map);
     return bimap;
   }
 
   private EnumHashBiMap(Class<K> keyType) {
-    super(
-        new EnumMap<K, V>(keyType),
-        Maps.<V, K>newHashMapWithExpectedSize(keyType.getEnumConstants().length));
-    this.keyType = keyType;
+    super(new EnumMap<K, V>(keyType), new HashMap<V, K>());
+    // TODO: cpovirk - Pre-size the HashMap based on the number of enum values?
+    this.keyTypeOrObjectUnderJ2cl = keyType;
   }
 
   // Overriding these 3 methods to show that values may be null (but not keys)
@@ -109,8 +108,9 @@ public V forcePut(K key, @ParametricNullness V value) {
   }
 
   /** Returns the associated key type. */
+  @GwtIncompatible
   public Class<K> keyType() {
-    return keyType;
+    return keyTypeOrObjectUnderJ2cl;
   }
 
   /**
@@ -120,17 +120,21 @@ public Class<K> keyType() {
   @GwtIncompatible // java.io.ObjectOutputStream
   private void writeObject(ObjectOutputStream stream) throws IOException {
     stream.defaultWriteObject();
-    stream.writeObject(keyType);
+    stream.writeObject(keyTypeOrObjectUnderJ2cl);
     Serialization.writeMap(this, stream);
   }
 
   @SuppressWarnings("unchecked") // reading field populated by writeObject
   @GwtIncompatible // java.io.ObjectInputStream
   private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException {
     stream.defaultReadObject();
-    keyType = (Class<K>) stream.readObject();
-    setDelegates(
-        new EnumMap<K, V>(keyType), new HashMap<V, K>(keyType.getEnumConstants().length * 3 / 2));
+    keyTypeOrObjectUnderJ2cl = (Class<K>) stream.readObject();
+    /*
+     * TODO: cpovirk - Pre-size the HashMap based on the number of enum values? (But *not* based on
+     * the number of entries in the map, as that makes it easy for hostile inputs to trigger lots of
+     * allocation—not that any program should be deserializing hostile inputs to begin with!)
+     */
+    setDelegates(new EnumMap<K, V>(keyTypeOrObjectUnderJ2cl), new HashMap<V, K>());
     Serialization.populateMap(this, stream);
   }
 
diff --git a/android/guava/src/com/google/common/collect/Platform.java b/android/guava/src/com/google/common/collect/Platform.java
@@ -117,6 +117,10 @@ static MapMaker tryWeakKeys(MapMaker mapMaker) {
     return mapMaker.weakKeys();
   }
 
+  static <E extends Enum<E>> Class<E> getDeclaringClassOrObjectForJ2cl(E e) {
+    return e.getDeclaringClass();
+  }
+
   static int reduceIterationsIfGwt(int iterations) {
     return iterations;
   }
diff --git a/guava-gwt/src-super/com/google/common/collect/super/com/google/common/collect/Platform.java b/guava-gwt/src-super/com/google/common/collect/super/com/google/common/collect/Platform.java
@@ -21,15 +21,15 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import jsinterop.annotations.JsMethod;
 import jsinterop.annotations.JsPackage;
 import jsinterop.annotations.JsProperty;
 import jsinterop.annotations.JsType;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Minimal GWT emulation of {@code com.google.common.collect.Platform}.
  *
- * <p><strong>This .java file should never be consumed by javac.</strong>
- *
  * @author Hayward Chan
  */
 final class Platform {
@@ -96,6 +96,19 @@ static MapMaker tryWeakKeys(MapMaker mapMaker) {
     return mapMaker;
   }
 
+  static <E extends Enum<E>> Class<E> getDeclaringClassOrObjectForJ2cl(E e) {
+    Class<E> classOrNull = getDeclaringClassOrNullForJ2cl(e);
+    @SuppressWarnings("unchecked")
+    Class<E> result = classOrNull == null ? (Class<E>) (Class<?>) Object.class : classOrNull;
+    return result;
+  }
+
+  @JsMethod
+  private static native <E extends Enum<E>> @Nullable Class<E> getDeclaringClassOrNullForJ2cl(
+      E e) /*-{
+    return e.@java.lang.Enum::getDeclaringClass()();
+  }-*/;
+
   static int reduceIterationsIfGwt(int iterations) {
     return iterations / 10;
   }
diff --git a/guava-gwt/src-super/com/google/common/collect/super/com/google/common/collect/Platform.native.js b/guava-gwt/src-super/com/google/common/collect/super/com/google/common/collect/Platform.native.js
@@ -0,0 +1,3 @@
+Platform.getDeclaringClassOrNullForJ2cl = function(e) {
+  return null;
+}
diff --git a/guava-tests/test/com/google/common/collect/EnumBiMapTest.java b/guava-tests/test/com/google/common/collect/EnumBiMapTest.java
@@ -183,11 +183,13 @@ public void testEnumBiMapConstructor() {
     assertEquals(bimap3, emptyBimap);
   }
 
+  @GwtIncompatible // keyType
   public void testKeyType() {
     EnumBiMap<Currency, Country> bimap = EnumBiMap.create(Currency.class, Country.class);
     assertEquals(Currency.class, bimap.keyType());
   }
 
+  @GwtIncompatible // valueType
   public void testValueType() {
     EnumBiMap<Currency, Country> bimap = EnumBiMap.create(Currency.class, Country.class);
     assertEquals(Country.class, bimap.valueType());
diff --git a/guava-tests/test/com/google/common/collect/EnumHashBiMapTest.java b/guava-tests/test/com/google/common/collect/EnumHashBiMapTest.java
@@ -197,6 +197,7 @@ public void testEnumBiMapConstructor() {
     assertEquals(bimap3, emptyBimap);
   }
 
+  @GwtIncompatible // keyType
   public void testKeyType() {
     EnumHashBiMap<Currency, String> bimap = EnumHashBiMap.create(Currency.class);
     assertEquals(Currency.class, bimap.keyType());
diff --git a/guava/src/com/google/common/collect/EnumBiMap.java b/guava/src/com/google/common/collect/EnumBiMap.java
diff --git a/guava/src/com/google/common/collect/EnumHashBiMap.java b/guava/src/com/google/common/collect/EnumHashBiMap.java
diff --git a/guava/src/com/google/common/collect/Platform.java b/guava/src/com/google/common/collect/Platform.java

Original file line number	Diff line number	Diff line change
`@@ -183,11 +183,13 @@ public void testEnumBiMapConstructor() {`
`183`	`183`	`assertEquals(bimap3, emptyBimap);`
`184`	`184`	`}`
`185`	`185`
	`186`	`+ @GwtIncompatible // keyType`
`186`	`187`	`public void testKeyType() {`
`187`	`188`	`EnumBiMap<Currency, Country> bimap = EnumBiMap.create(Currency.class, Country.class);`
`188`	`189`	`assertEquals(Currency.class, bimap.keyType());`
`189`	`190`	`}`
`190`	`191`
	`192`	`+ @GwtIncompatible // valueType`
`191`	`193`	`public void testValueType() {`
`192`	`194`	`EnumBiMap<Currency, Country> bimap = EnumBiMap.create(Currency.class, Country.class);`
`193`	`195`	`assertEquals(Country.class, bimap.valueType());`
Original file line number	Diff line number	Diff line change
`@@ -197,6 +197,7 @@ public void testEnumBiMapConstructor() {`
`197`	`197`	`assertEquals(bimap3, emptyBimap);`
`198`	`198`	`}`
`199`	`199`
	`200`	`+ @GwtIncompatible // keyType`
`200`	`201`	`public void testKeyType() {`
`201`	`202`	`EnumHashBiMap<Currency, String> bimap = EnumHashBiMap.create(Currency.class);`
`202`	`203`	`assertEquals(Currency.class, bimap.keyType());`
Original file line number	Diff line number	Diff line change
`@@ -117,6 +117,10 @@ static MapMaker tryWeakKeys(MapMaker mapMaker) {`
`117`	`117`	`return mapMaker.weakKeys();`
`118`	`118`	`}`
`119`	`119`
	`120`	`+ static <E extends Enum<E>> Class<E> getDeclaringClassOrObjectForJ2cl(E e) {`
	`121`	`+ return e.getDeclaringClass();`
	`122`	`+ }`
	`123`	`+`
`120`	`124`	`static int reduceIterationsIfGwt(int iterations) {`
`121`	`125`	`return iterations;`
`122`	`126`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Platform.getDeclaringClassOrNullForJ2cl = function(e) {`
	`2`	`+ return null;`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -126,6 +126,10 @@ static MapMaker tryWeakKeys(MapMaker mapMaker) {`
`126`	`126`	`return mapMaker.weakKeys();`
`127`	`127`	`}`
`128`	`128`
	`129`	`+ static <E extends Enum<E>> Class<E> getDeclaringClassOrObjectForJ2cl(E e) {`
	`130`	`+ return e.getDeclaringClass();`
	`131`	`+ }`
	`132`	`+`
`129`	`133`	`static int reduceIterationsIfGwt(int iterations) {`
`130`	`134`	`return iterations;`
`131`	`135`	`}`