Eagerly ensure that user-supplied providers always return the expected type. Historically this was implicitly validated upon injection into whatever used the provider (via reflection magic). However, with the shift to MethodHandles, that particular signature check becomes harder to isolate/replicate without adding a bunch of unnecessary overhead. Thus, we instead change this to validate user-supplied providers when they provide the value (which is the soonest it can go wrong).

PiperOrigin-RevId: 752295067

Author: sameb

core/src/com/google/inject/internal/BindingProcessor.java

@@ -154,12 +154,13 @@ public Boolean visit(ProviderInstanceBinding<? extends T> binding) {
             ProvisionListenerStackCallback<T> listener =
                 injector.provisionListenerStore.get((Binding<T>) binding);
             int circularFactoryId = injector.circularFactoryIdFactory.next();
+            Class<? super T> rawType = key.getTypeLiteral().getRawType();
             InternalFactory<T> factory =
                 (initializable.isPresent()
                     ? new InternalFactoryToInitializableAdapter<T>(
-                        initializable.get(), source, listener, circularFactoryId)
+                        rawType, initializable.get(), source, listener, circularFactoryId)
                     : new ConstantProviderInternalFactory<T>(
-                        provider, source, listener, circularFactoryId));
+                        rawType, provider, source, listener, circularFactoryId));
 
             InternalFactory<? extends T> scopedFactory =
                 Scoping.scope(key, injector, factory, source, scoping);
@@ -178,6 +179,7 @@ public Boolean visit(ProviderKeyBinding<? extends T> binding) {
             @SuppressWarnings("unchecked")
             BoundProviderFactory<T> boundProviderFactory =
                 new BoundProviderFactory<T>(
+                    key.getTypeLiteral().getRawType(),
                     injector,
                     providerKey,
                     source,

core/src/com/google/inject/internal/BoundProviderFactory.java

@@ -29,11 +29,12 @@ final class BoundProviderFactory<T> extends ProviderInternalFactory<T> implement
   private InternalFactory<? extends jakarta.inject.Provider<? extends T>> providerFactory;
 
   BoundProviderFactory(
+      Class<? super T> rawType,
       InjectorImpl injector,
       Key<? extends jakarta.inject.Provider<? extends T>> providerKey,
       Object source,
       ProvisionListenerStackCallback<T> provisionCallback) {
-    super(source, injector.circularFactoryIdFactory.next());
+    super(rawType, source, injector.circularFactoryIdFactory.next());
     this.provisionCallback = provisionCallback;
     this.injector = injector;
     this.providerKey = providerKey;

core/src/com/google/inject/internal/ConstantProviderInternalFactory.java

@@ -19,20 +19,21 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.inject.spi.Dependency;
-import jakarta.inject.Provider;
 import javax.annotation.Nullable;
+import jakarta.inject.Provider;
 
 /** An InternalFactory that delegates to a constant provider. */
 final class ConstantProviderInternalFactory<T> extends ProviderInternalFactory<T> {
   private final Provider<T> provider;
   @Nullable private final ProvisionListenerStackCallback<T> provisionCallback;
 
   ConstantProviderInternalFactory(
+      Class<? super T> rawType,
       Provider<T> provider,
       Object source,
       @Nullable ProvisionListenerStackCallback<T> provisionCallback,
       int circularFactoryId) {
-    super(source, circularFactoryId);
+    super(rawType, source, circularFactoryId);
     this.provider = checkNotNull(provider);
     this.provisionCallback = provisionCallback;
   }

core/src/com/google/inject/internal/ConstructorInjector.java

@@ -121,7 +121,7 @@ MethodHandle getConstructHandle(
               MethodHandles.dropArguments(
                   MethodHandles.identity(Object.class), 1, InternalContext.class),
               membersHandle);
-      // Then execute thje membersHandle after constructing the object (and calling
+      // Then execute the membersHandle after constructing the object (and calling
       // finishConstructionAndSetReference)
       handle = MethodHandles.foldArguments(membersHandle, handle);
     } else {

core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java

@@ -33,11 +33,12 @@ final class InternalFactoryToInitializableAdapter<T> extends ProviderInternalFac
   private final Initializable<? extends jakarta.inject.Provider<? extends T>> initializable;
 
   public InternalFactoryToInitializableAdapter(
+      Class<? super T> rawType,
       Initializable<? extends jakarta.inject.Provider<? extends T>> initializable,
       Object source,
       ProvisionListenerStackCallback<T> provisionCallback,
       int circularFactoryId) {
-    super(source, circularFactoryId);
+    super(rawType, source, circularFactoryId);
     this.provisionCallback = provisionCallback;
     this.initializable = checkNotNull(initializable, "provider");
   }

core/src/com/google/inject/internal/MembersInjectorImpl.java

@@ -307,7 +307,6 @@ MethodHandle getInjectMembersAndNotifyListenersHandle(@Nullable LinkageContext l
       // (Object, InternalContext)->void
       var notifyListeners =
           MethodHandles.dropArguments(getNotifyListenersHandle(), 1, InternalContext.class);
-      ;
 
       local = MethodHandles.foldArguments(notifyListeners, injectMembers);
 

core/src/com/google/inject/internal/ProvidedByInternalFactory.java

@@ -22,9 +22,9 @@
 import com.google.inject.Key;
 import com.google.inject.internal.InjectorImpl.JitLimitation;
 import com.google.inject.spi.Dependency;
-import jakarta.inject.Provider;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+import jakarta.inject.Provider;
 
 /**
  * An {@link InternalFactory} for {@literal @}{@link ProvidedBy} bindings.
@@ -33,7 +33,6 @@
  */
 class ProvidedByInternalFactory<T> extends ProviderInternalFactory<T> implements DelayedInitialize {
 
-  private final Class<?> rawType;
   private final Class<? extends Provider<?>> providerType;
   private final Key<? extends Provider<T>> providerKey;
   private InternalFactory<? extends Provider<T>> providerFactory;
@@ -44,8 +43,7 @@ class ProvidedByInternalFactory<T> extends ProviderInternalFactory<T> implements
       Class<? extends Provider<?>> providerType,
       Key<? extends Provider<T>> providerKey,
       int circularFactoryId) {
-    super(providerKey, circularFactoryId);
-    this.rawType = rawType;
+    super(rawType, providerKey, circularFactoryId);
     this.providerType = providerType;
     this.providerKey = providerKey;
   }
@@ -86,13 +84,11 @@ MethodHandleResult makeHandle(LinkageContext context, boolean linked) {
   }
 
   @Override
-  protected MethodHandle provisionHandle(MethodHandle providerHandle) {
-    // Do normal provisioning and then check that the result is the correct subtype.
-    MethodHandle invokeProvider = super.provisionHandle(providerHandle);
+  protected MethodHandle validateReturnTypeHandle(MethodHandle providerHandle) {
     return MethodHandles.filterReturnValue(
-        invokeProvider,
+        providerHandle,
         MethodHandles.insertArguments(
-            CHECK_SUBTYPE_NOT_PROVIDED_MH, 1, source, providerType, rawType));
+            CHECK_SUBTYPE_NOT_PROVIDED_MH, 1, source, providerType, providedRawType));
   }
 
   private static final MethodHandle CHECK_SUBTYPE_NOT_PROVIDED_MH =
@@ -101,6 +97,8 @@ protected MethodHandle provisionHandle(MethodHandle providerHandle) {
           "doCheckSubtypeNotProvided",
           methodType(Object.class, Object.class, Object.class, Class.class, Class.class));
 
+  // Historically this had a different error check than other providers,
+  // so we preserve that behavior.
   @Keep
   static Object doCheckSubtypeNotProvided(
       Object result,
@@ -115,18 +113,13 @@ static Object doCheckSubtypeNotProvided(
     return result;
   }
 
+  // Historically this had a different error check than other providers,
+  // so we preserve that behavior.
   @Override
-  protected T provision(
-      jakarta.inject.Provider<? extends T> provider,
-      InternalContext context,
-      Dependency<?> dependency)
-      throws InternalProvisionException {
-    Object o = super.provision(provider, context, dependency);
-    if (o != null && !rawType.isInstance(o)) {
-      throw InternalProvisionException.subtypeNotProvided(providerType, rawType).addSource(source);
+  protected void validateReturnType(T t) throws InternalProvisionException {
+    if (t != null && !providedRawType.isInstance(t)) {
+      throw InternalProvisionException.subtypeNotProvided(providerType, providedRawType)
+          .addSource(source);
     }
-    @SuppressWarnings("unchecked") // protected by isInstance() check above
-    T t = (T) o;
-    return t;
   }
 }

core/src/com/google/inject/internal/ProviderInternalFactory.java

@@ -20,22 +20,25 @@
 import static com.google.inject.internal.InternalMethodHandles.castReturnTo;
 import static java.lang.invoke.MethodType.methodType;
 
+import com.google.errorprone.annotations.Keep;
 import com.google.inject.spi.Dependency;
-import jakarta.inject.Provider;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import javax.annotation.Nullable;
+import jakarta.inject.Provider;
 
 /**
  * Base class for InternalFactories that are used by Providers, to handle circular dependencies.
  *
  * @author [email protected] (Sam Berlin)
  */
 abstract class ProviderInternalFactory<T> extends InternalFactory<T> {
+  protected final Class<?> providedRawType; // Technically this is "? super T", but this is easier.
   protected final Object source;
   private final int circularFactoryId;
 
-  ProviderInternalFactory(Object source, int circularFactoryId) {
+  ProviderInternalFactory(Class<?> providedRawType, Object source, int circularFactoryId) {
+    this.providedRawType = providedRawType;
     this.source = checkNotNull(source, "source");
     this.circularFactoryId = circularFactoryId;
   }
@@ -150,9 +153,36 @@ protected MethodHandle provisionHandle(MethodHandle providerHandle) {
     }
     // null check the result using the dependency.
     invokeProvider = InternalMethodHandles.nullCheckResult(invokeProvider, source);
+    invokeProvider = validateReturnTypeHandle(invokeProvider);
     return invokeProvider;
   }
 
+  protected MethodHandle validateReturnTypeHandle(MethodHandle resultHandle) {
+    return MethodHandles.filterReturnValue(
+        resultHandle,
+        MethodHandles.insertArguments(CHECK_SUBTYPE_NOT_PROVIDED_MH, 1, source, providedRawType));
+  }
+
+  private static final MethodHandle CHECK_SUBTYPE_NOT_PROVIDED_MH =
+      InternalMethodHandles.findStaticOrDie(
+          ProviderInternalFactory.class,
+          "doCheckSubtypeNotProvided",
+          methodType(Object.class, Object.class, Object.class, Class.class));
+
+  @Keep
+  static Object doCheckSubtypeNotProvided(Object result, Object source, Class<?> providedType)
+      throws InternalProvisionException {
+    if (result != null && !providedType.isInstance(result)) {
+      // Historically this was surfaced as a ProvisionException embedding a
+      // ClassCastException, so we keep that behavior. (Maybe one day we can
+      // shift it to an explicit error without the inner ClassCastException.)
+      throw InternalProvisionException.errorInProvider(
+              new ClassCastException("Cannot cast " + result.getClass() + " to " + providedType))
+          .addSource(source);
+    }
+    return result;
+  }
+
   /**
    * Provisions a new instance. Subclasses should override this to catch exceptions and rethrow as
    * ErrorsExceptions.
@@ -171,6 +201,18 @@ protected T provision(
     if (t == null && !dependency.isNullable()) {
       InternalProvisionException.onNullInjectedIntoNonNullableDependency(source, dependency);
     }
+    validateReturnType(t);
     return t;
   }
+
+  protected void validateReturnType(T t) throws InternalProvisionException {
+    if (t != null && !providedRawType.isInstance(t)) {
+      // Historically this was surfaced as a ProvisionException embedding a
+      // ClassCastException, so we keep that behavior. (Maybe one day we can
+      // shift it to an explicit error without the inner ClassCastException.
+      throw InternalProvisionException.errorInProvider(
+              new ClassCastException("Cannot cast " + t.getClass() + " to " + providedRawType))
+          .addSource(source);
+    }
+  }
 }

core/test/com/google/inject/ProvisionExceptionsTest.java

@@ -326,6 +326,33 @@ Object providesDependsOnFailingProvider(String failing) {
             provideFailingValue); // the thing that failed.
   }
 
+  @Test
+  @SuppressWarnings({"rawtypes", "unchecked"}) // Test requires incorrect types
+  public void testConstructorBindingDependencyHasWrongType() {
+    var module =
+        new AbstractModule() {
+          @Override
+          protected void configure() {
+            bind(String.class)
+                .toProvider(
+                    new Provider() {
+                      @Override
+                      public Integer get() {
+                        return 1;
+                      }
+                    });
+          }
+        };
+    Injector injector = Guice.createInjector(module);
+    var pe = assertThrows(ProvisionException.class, () -> injector.getInstance(WantsString.class));
+    assertThat(pe).hasCauseThat().isInstanceOf(ClassCastException.class);
+  }
+
+  private static class WantsString {
+    @Inject
+    WantsString(String s) {}
+  }
+
   private static interface Exploder {}
 
   public static class Explosion implements Exploder {

extensions/testlib/test/com/google/inject/testing/fieldbinder/BoundFieldModuleTest.java

@@ -669,7 +669,7 @@ public Object get() {
   }
 
   @SuppressWarnings({"rawtypes", "JUnitIncompatibleType"}) // Testing rawtypes
-  public void testRawProviderCanBindToIncorrectType() {
+  public void testRawProviderCannotBindToIncorrectType() {
     final Integer testValue = 1024;
     Object instance =
         new Object() {
@@ -686,7 +686,12 @@ public Object get() {
     BoundFieldModule module = BoundFieldModule.of(instance);
     Injector injector = Guice.createInjector(module);
 
-    assertEquals(testValue, injector.getInstance(String.class));
+    try {
+      injector.getInstance(String.class);
+      fail();
+    } catch (ProvisionException e) {
+      assertEquals(e.getCause().getClass(), ClassCastException.class);
+    }
   }
 
   public void testMultipleBindErrorsAreAggregated() {
@@ -1006,7 +1011,7 @@ public void testGetBoundFields_getField() throws Exception {
 
     assertEquals(value, injector.getInstance(info.getBoundKey()));
   }
-  
+
   public void testGetBoundFields_getKey() throws Exception {
     Object instance =
         new Object() {