Use linked bindings for MapBinder/Multibinder/OptionalBinder aliases, instead of linking to the same internal factory implementation. (Reusing the internal factory results in some quirks that we want to avoid.)

sameb · Guice Team · commit 2d64067e9940 · 2023-05-10T12:34:58.000-07:00
PiperOrigin-RevId: 530977598
diff --git a/core/src/com/google/inject/internal/RealMapBinder.java b/core/src/com/google/inject/internal/RealMapBinder.java
@@ -290,39 +290,37 @@ public LinkedBindingBuilder<V> addBinding(K key) {
     return binder.bind(getKeyForNewValue(key));
   }
 
+  @SuppressWarnings({"unchecked", "rawtypes"}) // we use raw Key to link bindings.
   @Override
   public void configure(Binder binder) {
     checkConfiguration(!bindingSelection.isInitialized(), "MapBinder was already initialized");
 
     // Binds a Map<K, Provider<V>>
-    RealProviderMapProvider<K, V> providerMapProvider =
-        new RealProviderMapProvider<>(bindingSelection);
-    binder.bind(bindingSelection.getProviderMapKey()).toProvider(providerMapProvider);
+    binder
+        .bind(bindingSelection.getProviderMapKey())
+        .toProvider(new RealProviderMapProvider<>(bindingSelection));
 
     // The map this exposes is internally an ImmutableMap, so it's OK to massage
     // the guice Provider to jakarta Provider in the value (since Guice provider
     // implements jakarta Provider).
-      @SuppressWarnings({"unchecked", "rawtypes"})
-      Provider<Map<K, jakarta.inject.Provider<V>>> jakartaProviderMapProvider =
-          (Provider) providerMapProvider;
       binder
           .bind(bindingSelection.getJakartaProviderMapKey())
-          .toProvider(jakartaProviderMapProvider);
+          .to((Key) bindingSelection.getProviderMapKey());
 
-    RealMapProvider<K, V> mapProvider = new RealMapProvider<>(bindingSelection);
     // Bind Map<K, V> to the provider w/ extension support.
     binder
         .bind(bindingSelection.getMapKey())
-        .toProvider(new ExtensionRealMapProvider<>(mapProvider));
+        .toProvider(new ExtensionRealMapProvider<>(bindingSelection));
     // Bind Map<K, ? extends V> to the provider w/o the extension support.
-    binder.bind(bindingSelection.getMapOfKeyExtendsValueKey()).toProvider(mapProvider);
+    binder
+        .bind(bindingSelection.getMapOfKeyExtendsValueKey())
+        .to((Key) bindingSelection.getMapKey());
 
     // The Map.Entries are all ProviderMapEntry instances which do not allow setValue, so it is
     // safe to massage the return type like this
-      @SuppressWarnings({"unchecked", "rawtypes"})
-      Key<Set<Map.Entry<K, jakarta.inject.Provider<V>>>> jakartaEntrySetProviderKey =
-          (Key) bindingSelection.getEntrySetBinder().getSetKey();
-      binder.bind(bindingSelection.getEntrySetJakartaProviderKey()).to(jakartaEntrySetProviderKey);
+      binder
+          .bind(bindingSelection.getEntrySetJakartaProviderKey())
+          .to((Key) bindingSelection.getEntrySetBinder().getSetKey());
   }
 
   @Override
@@ -737,8 +735,14 @@ protected Map<K, Provider<V>> doProvision(InternalContext context, Dependency<?>
     }
   }
 
-  private static final class RealMapProvider<K, V>
-      extends RealMapBinderProviderWithDependencies<K, V, Map<K, V>> {
+  /**
+   * Implementation of a provider instance for the map that also exposes details about the MapBinder
+   * using the extension SPI, delegating to another provider instance for non-extension (e.g, the
+   * actual provider instance info) data.
+   */
+  private static final class ExtensionRealMapProvider<K, V>
+      extends RealMapBinderProviderWithDependencies<K, V, Map<K, V>>
+      implements ProviderWithExtensionVisitor<Map<K, V>>, MapBinderBinding<Map<K, V>> {
     Set<Dependency<?>> dependencies = RealMapBinder.MODULE_DEPENDENCIES;
 
     /**
@@ -752,14 +756,10 @@ private static final class RealMapProvider<K, V>
 
     private boolean initialized = false;
 
-    RealMapProvider(BindingSelection<K, V> bindingSelection) {
+    ExtensionRealMapProvider(BindingSelection<K, V> bindingSelection) {
       super(bindingSelection);
     }
 
-    BindingSelection<K, V> getBindingSelection() {
-      return bindingSelection;
-    }
-
     @Override
     protected void doInitialize(InjectorImpl injector, Errors errors) throws ErrorsException {
       if (initialized) {
@@ -821,43 +821,11 @@ protected Map<K, V> doProvision(InternalContext context, Dependency<?> dependenc
     public Set<Dependency<?>> getDependencies() {
       return dependencies;
     }
-  }
-
-  /**
-   * Implementation of a provider instance for the map that also exposes details about the MapBinder
-   * using the extension SPI, delegating to another provider instance for non-extension (e.g, the
-   * actual provider instance info) data.
-   */
-  private static final class ExtensionRealMapProvider<K, V>
-      extends RealMapBinderProviderWithDependencies<K, V, Map<K, V>>
-      implements ProviderWithExtensionVisitor<Map<K, V>>, MapBinderBinding<Map<K, V>> {
-    final RealMapProvider<K, V> delegate;
-
-    ExtensionRealMapProvider(RealMapProvider<K, V> delegate) {
-      super(delegate.bindingSelection);
-      this.delegate = delegate;
-    }
 
     BindingSelection<K, V> getBindingSelection() {
       return bindingSelection;
     }
 
-    @Override
-    protected void doInitialize(InjectorImpl injector, Errors errors) throws ErrorsException {
-      delegate.doInitialize(injector, errors);
-    }
-
-    @Override
-    protected Map<K, V> doProvision(InternalContext context, Dependency<?> dependency)
-        throws InternalProvisionException {
-      return delegate.doProvision(context, dependency);
-    }
-
-    @Override
-    public Set<Dependency<?>> getDependencies() {
-      return delegate.getDependencies();
-    }
-
     @Override
     @SuppressWarnings("unchecked")
     public <B, W> W acceptExtensionVisitor(
@@ -1041,42 +1009,34 @@ private MultimapBinder(BindingSelection<K, V> bindingSelection) {
       this.bindingSelection = bindingSelection;
     }
 
+    @SuppressWarnings({"unchecked", "rawtypes"}) // we use raw Key to link bindings together.
     @Override
     public void configure(Binder binder) {
       // Binds a Map<K, Set<Provider<V>>>
-      Provider<Map<K, Set<Provider<V>>>> multimapProvider =
-          new RealProviderMultimapProvider<K, V>(bindingSelection.getMapKey());
-      binder.bind(bindingSelection.getProviderSetMultimapKey()).toProvider(multimapProvider);
+      binder
+          .bind(bindingSelection.getProviderSetMultimapKey())
+          .toProvider(new RealProviderMultimapProvider<K, V>(bindingSelection.getMapKey()));
 
       // Provide links from a few different public keys to the providerMultimapKey.
       // The collection this exposes is internally an ImmutableMap, so it's OK to massage
       // the guice Provider to jakarta Provider in the value (since the guice Provider implements
       // jakarta Provider).
-        @SuppressWarnings({"unchecked", "rawtypes"})
-        Provider<Map<K, Set<jakarta.inject.Provider<V>>>> jakartaProvider =
-            (Provider) multimapProvider;
         binder
             .bind(bindingSelection.getJakartaProviderSetMultimapKey())
-            .toProvider(jakartaProvider);
+            .to((Key) bindingSelection.getProviderSetMultimapKey());
 
-      @SuppressWarnings({"unchecked", "rawtypes"})
-      Provider<Map<K, Collection<Provider<V>>>> collectionProvider = (Provider) multimapProvider;
       binder
           .bind(bindingSelection.getProviderCollectionMultimapKey())
-          .toProvider(collectionProvider);
+          .to((Key) bindingSelection.getProviderSetMultimapKey());
 
-        @SuppressWarnings({"unchecked", "rawtypes"})
-        Provider<Map<K, Collection<jakarta.inject.Provider<V>>>> collectionJakartaProvider =
-            (Provider) multimapProvider;
         binder
             .bind(bindingSelection.getJakartaProviderCollectionMultimapKey())
-            .toProvider(collectionJakartaProvider);
+            .to((Key) bindingSelection.getProviderSetMultimapKey());
 
       // Binds a Map<K, Set<V>>
-      @SuppressWarnings({"unchecked", "rawtypes"})
-      Provider<Map<K, Set<V>>> realMultimapProvider =
-          new RealMultimapProvider(bindingSelection.getMapKey());
-      binder.bind(bindingSelection.getMultimapKey()).toProvider(realMultimapProvider);
+      binder
+          .bind(bindingSelection.getMultimapKey())
+          .toProvider(new RealMultimapProvider(bindingSelection.getMapKey()));
     }
 
     @Override
diff --git a/core/src/com/google/inject/internal/RealMultibinder.java b/core/src/com/google/inject/internal/RealMultibinder.java
@@ -96,35 +96,27 @@ static <T> TypeLiteral<Set<? extends T>> setOfExtendsOf(TypeLiteral<T> elementTy
     this.bindingSelection = new BindingSelection<>(key);
   }
 
+  @SuppressWarnings({"unchecked", "rawtypes"}) // we use raw Key to link bindings together.
   @Override
   public void configure(Binder binder) {
     checkConfiguration(!bindingSelection.isInitialized(), "Multibinder was already initialized");
 
-    RealMultibinderProvider<T> setProvider = new RealMultibinderProvider<T>(bindingSelection);
     // Bind the setKey to the provider wrapped w/ extension support.
     binder
         .bind(bindingSelection.getSetKey())
-        .toProvider(new ExtensionRealMultibinderProvider<>(setProvider));
-    // Bind the <? extends T> to the provider w/o extension support.
-    // It's important the exactly one binding implement the extension support and show
-    // the other keys as aliases, to adhere to the extension contract.
-    binder.bind(bindingSelection.getSetOfExtendsKey()).toProvider(setProvider);
-
-    Provider<Collection<Provider<T>>> collectionOfProvidersProvider =
-        new RealMultibinderCollectionOfProvidersProvider<T>(bindingSelection);
+        .toProvider(new RealMultibinderProvider<>(bindingSelection));
+    binder.bind(bindingSelection.getSetOfExtendsKey()).to(bindingSelection.getSetKey());
+
     binder
         .bind(bindingSelection.getCollectionOfProvidersKey())
-        .toProvider(collectionOfProvidersProvider);
+        .toProvider(new RealMultibinderCollectionOfProvidersProvider<T>(bindingSelection));
 
     // The collection this exposes is internally an ImmutableList, so it's OK to massage
     // the guice Provider to jakarta Provider in the value (since the guice Provider implements
     // jakarta Provider).
-      @SuppressWarnings("unchecked")
-      Provider<Collection<jakarta.inject.Provider<T>>> jakartaProvider =
-          (Provider) collectionOfProvidersProvider;
       binder
           .bind(bindingSelection.getCollectionOfJakartaProvidersKey())
-          .toProvider(jakartaProvider);
+          .to((Key) bindingSelection.getCollectionOfProvidersKey());
   }
 
   public void permitDuplicates() {
@@ -218,7 +210,8 @@ public int hashCode() {
    * so it can be used to supply a {@code Set<T>} and {@code Set<? extends T>}, the latter being
    * useful for Kotlin support.
    */
-  private static final class RealMultibinderProvider<T> extends BaseFactory<T, Set<T>> {
+  private static final class RealMultibinderProvider<T> extends BaseFactory<T, Set<T>>
+      implements ProviderWithExtensionVisitor<Set<T>>, MultibinderBinding<Set<T>> {
     List<Binding<T>> bindings;
     SingleParameterInjector<T>[] injectors;
     boolean permitDuplicates;
@@ -281,33 +274,6 @@ private InternalProvisionException newDuplicateValuesException(T[] values) {
                   bindingSelection.getSetKey(), bindings, values, ImmutableList.of(getSource())));
       return new InternalProvisionException(message);
     }
-  }
-
-  /**
-   * Implementation of BaseFactory that exposes details about the multibinder through the extension
-   * SPI.
-   */
-  private static final class ExtensionRealMultibinderProvider<T> extends BaseFactory<T, Set<T>>
-      implements ProviderWithExtensionVisitor<Set<T>>, MultibinderBinding<Set<T>> {
-    final RealMultibinderProvider<T> delegate;
-
-    ExtensionRealMultibinderProvider(RealMultibinderProvider<T> delegate) {
-      // Note: method reference doesn't work for the 2nd arg for some reason when compiling on java8
-      super(delegate.bindingSelection, bs -> bs.getDependencies());
-      this.delegate = delegate;
-    }
-
-    @Override
-    protected void doInitialize() {
-      delegate.doInitialize();
-    }
-
-    @Override
-    protected ImmutableSet<T> doProvision(InternalContext context, Dependency<?> dependency)
-        throws InternalProvisionException {
-      return delegate.doProvision(context, dependency);
-    }
-
     @SuppressWarnings("unchecked")
     @Override
     public <B, V> V acceptExtensionVisitor(
diff --git a/core/src/com/google/inject/internal/RealOptionalBinder.java b/core/src/com/google/inject/internal/RealOptionalBinder.java
@@ -183,6 +183,7 @@ public LinkedBindingBuilder<T> setBinding() {
     return binder.bind(getKeyForActualBinding());
   }
 
+  @SuppressWarnings({"unchecked", "rawtypes"}) // we use raw Key to link bindings together.
   @Override
   public void configure(Binder binder) {
     bindingSelection.checkNotInitialized();
@@ -196,45 +197,34 @@ public void configure(Binder binder) {
     // * T is bound
 
     // cgcb.Optional<c.g.i.Provider<T>>
-    InternalProviderInstanceBindingImpl.Factory<Optional<Provider<T>>> optionalProviderFactory =
-        new RealOptionalProviderProvider<>(bindingSelection);
-    binder.bind(key.ofType(optionalOfProvider(typeLiteral))).toProvider(optionalProviderFactory);
+    Key<Optional<Provider<T>>> guavaOptProviderKey = key.ofType(optionalOfProvider(typeLiteral));
+    binder
+        .bind(guavaOptProviderKey)
+        .toProvider(new RealOptionalProviderProvider<>(bindingSelection));
     // ju.Optional<c.g.i.Provider<T>>
-    InternalProviderInstanceBindingImpl.Factory<java.util.Optional<Provider<T>>>
-        javaOptionalProviderFactory = new JavaOptionalProviderProvider<>(bindingSelection);
+    Key<java.util.Optional<Provider<T>>> javaOptProviderKey =
+        key.ofType(javaOptionalOfProvider(typeLiteral));
     binder
-        .bind(key.ofType(javaOptionalOfProvider(typeLiteral)))
-        .toProvider(javaOptionalProviderFactory);
+        .bind(javaOptProviderKey)
+        .toProvider(new JavaOptionalProviderProvider<>(bindingSelection));
 
     // Provider is assignable to jakarta.inject.Provider and the provider that the factory contains
     // cannot be modified so we can use some rawtypes hackery to share the same implementation.
     // cgcb.Optional<jakarta.inject.Provider<T>>
-      @SuppressWarnings("unchecked")
-      InternalProviderInstanceBindingImpl.Factory<Optional<jakarta.inject.Provider<T>>>
-          optionalJakartaProviderFactory =
-              (InternalProviderInstanceBindingImpl.Factory) optionalProviderFactory;
-      binder
-          .bind(key.ofType(optionalOfJakartaProvider(typeLiteral)))
-          .toProvider(optionalJakartaProviderFactory);
+      binder.bind(key.ofType(optionalOfJakartaProvider(typeLiteral))).to((Key) guavaOptProviderKey);
       // ju.Optional<jakarta.inject.Provider<T>>
-      @SuppressWarnings("unchecked")
-      InternalProviderInstanceBindingImpl.Factory<java.util.Optional<jakarta.inject.Provider<T>>>
-          javaOptionalJakartaProviderFactory =
-              (InternalProviderInstanceBindingImpl.Factory) javaOptionalProviderFactory;
       binder
           .bind(key.ofType(javaOptionalOfJakartaProvider(typeLiteral)))
-          .toProvider(javaOptionalJakartaProviderFactory);
+          .to((Key) javaOptProviderKey);
 
     // cgcb.Optional<T>
-    Key<Optional<T>> optionalKey = key.ofType(optionalOf(typeLiteral));
+    Key<Optional<T>> guavaOptKey = key.ofType(optionalOf(typeLiteral));
     binder
-        .bind(optionalKey)
-        .toProvider(new RealOptionalKeyProvider<>(bindingSelection, optionalKey));
+        .bind(guavaOptKey)
+        .toProvider(new RealOptionalKeyProvider<>(bindingSelection, guavaOptKey));
     // ju.Optional<T>
-    Key<java.util.Optional<T>> javaOptionalKey = key.ofType(javaOptionalOf(typeLiteral));
-    binder
-        .bind(javaOptionalKey)
-        .toProvider(new JavaOptionalProvider<>(bindingSelection, javaOptionalKey));
+    Key<java.util.Optional<T>> javaOptKey = key.ofType(javaOptionalOf(typeLiteral));
+    binder.bind(javaOptKey).toProvider(new JavaOptionalProvider<>(bindingSelection, javaOptKey));
   }
 
   /** Provides the binding for {@code java.util.Optional<T>}. */
@@ -661,8 +651,7 @@ Dependency<?> getDependency() {
 
     /** Implementation of {@link OptionalBinderBinding#containsElement}. */
     boolean containsElement(Element element) {
-      // All of our bindings are ProviderInstanceBindings whose providers extend
-      // RealOptionalBinderProviderWithDependencies and have 'this' as its binding selection.
+      // We contain it if the provider is us.
       if (element instanceof ProviderInstanceBinding) {
         jakarta.inject.Provider<?> providerInstance =
             ((ProviderInstanceBinding<?>) element).getUserSuppliedProvider();
@@ -671,11 +660,16 @@ boolean containsElement(Element element) {
               .bindingSelection.equals(this);
         }
       }
+
       if (element instanceof Binding) {
+        TypeLiteral<?> typeLiteral = key.getTypeLiteral();
         Key<?> elementKey = ((Binding) element).getKey();
-        // if it isn't one of the things we bound directly it might be an actual or default key
+        // if it isn't one of the things we bound directly it might be an actual or default key,
+        // or the javax or jakarta aliases of the optional provider.
         return elementKey.equals(getKeyForActualBinding())
-            || elementKey.equals(getKeyForDefaultBinding());
+            || elementKey.equals(getKeyForDefaultBinding())
+            || elementKey.equals(key.ofType(javaOptionalOfJakartaProvider(typeLiteral)))
+            || elementKey.equals(key.ofType(optionalOfJakartaProvider(typeLiteral)));
       }
       return false; // cannot match;
     }
