Disallow providing or injecting dagger.internal.Provider. This will help with IDEs accidentally suggesting importing dagger.internal.Provider.

Chang-Eric · Dagger Team · commit 962bb333570f · 2025-01-09T12:20:13.000-08:00
Also disallow injections of raw Provider in Maps, for both javax and dagger Providers. This matches the policy of disallowing raw Provider injections in general.

RELNOTES=Disallow providing or injecting `dagger.internal.Provider`. Also disallow injections of raw Provider in Maps, for both javax and dagger Providers.
PiperOrigin-RevId: 713755026
diff --git a/java/dagger/internal/codegen/base/FrameworkTypes.java b/java/dagger/internal/codegen/base/FrameworkTypes.java
@@ -55,6 +55,12 @@ public final class FrameworkTypes {
           TypeNames.PROVIDER,
           TypeNames.JAKARTA_PROVIDER);
 
+  // This is a set of types that are disallowed from use, but also aren't framework types in the
+  // sense that they aren't supported. Like we shouldn't try to unwrap these if we see them, though
+  // we shouldn't see them at all if they are correctly caught in validation.
+  private static final ImmutableSet<ClassName> DISALLOWED_TYPES =
+      ImmutableSet.of(TypeNames.DAGGER_PROVIDER);
+
   /** Returns true if the type represents a producer-related framework type. */
   public static boolean isProducerType(XType type) {
     return typeIsOneOf(PRODUCTION_TYPES, type);
@@ -73,6 +79,10 @@ public static boolean isMapValueFrameworkType(XType type) {
     return typeIsOneOf(MAP_VALUE_FRAMEWORK_TYPES, type);
   }
 
+  public static boolean isDisallowedType(XType type) {
+    return typeIsOneOf(DISALLOWED_TYPES, type);
+  }
+
   private static boolean typeIsOneOf(Set<ClassName> classNames, XType type) {
     return classNames.stream().anyMatch(className -> isTypeOf(type, className));
   }
diff --git a/java/dagger/internal/codegen/validation/BindingElementValidator.java b/java/dagger/internal/codegen/validation/BindingElementValidator.java
@@ -42,6 +42,7 @@
 import dagger.internal.codegen.model.Key;
 import dagger.internal.codegen.model.Scope;
 import dagger.internal.codegen.xprocessing.XElements;
+import dagger.internal.codegen.xprocessing.XTypes;
 import java.util.Formatter;
 import java.util.HashMap;
 import java.util.Map;
@@ -176,6 +177,9 @@ protected void checkAdditionalProperties() {}
     protected void checkType() {
       switch (ContributionType.fromBindingElement(element)) {
         case UNIQUE:
+          // Basic checks on the types
+          bindingElementType().ifPresent(this::checkKeyType);
+
           // Validate that a unique binding is not attempting to bind a framework type. This
           // validation is only appropriate for unique bindings because multibindings may collect
           // framework types.  E.g. Set<Provider<Foo>> is perfectly reasonable.
@@ -185,17 +189,22 @@ protected void checkType() {
           // This validation is only appropriate for unique bindings because multibindings may
           // collect assisted types.
           checkAssistedType();
-          // fall through
+
+          // Check for any specifically disallowed types
+          bindingElementType().ifPresent(this::checkDisallowedType);
+          break;
 
         case SET:
           bindingElementType().ifPresent(this::checkSetValueFrameworkType);
           break;
+
         case MAP:
           bindingElementType().ifPresent(this::checkMapValueFrameworkType);
           break;
 
         case SET_VALUES:
           checkSetValuesType();
+          break;
       }
     }
 
@@ -360,24 +369,37 @@ private void checkScopes() {
      */
     private void checkFrameworkType() {
       if (bindingElementType().filter(FrameworkTypes::isFrameworkType).isPresent()) {
-        report.addError(bindingElements("must not %s framework types", bindingElementTypeVerb()));
+        report.addError(bindingElements("must not %s framework types: %s",
+            bindingElementTypeVerb(), XTypes.toStableString(bindingElementType().get())));
       }
     }
 
     private void checkSetValueFrameworkType(XType bindingType) {
       checkKeyType(bindingType);
       if (FrameworkTypes.isSetValueFrameworkType(bindingType)) {
         report.addError(bindingElements(
-            "with @IntoSet/@ElementsIntoSet must not %s framework types",
-            bindingElementTypeVerb()));
+            "with @IntoSet/@ElementsIntoSet must not %s framework types: %s",
+            bindingElementTypeVerb(), XTypes.toStableString(bindingType)));
       }
+      checkDisallowedType(bindingType);
     }
 
     private void checkMapValueFrameworkType(XType bindingType) {
       checkKeyType(bindingType);
       if (FrameworkTypes.isMapValueFrameworkType(bindingType)) {
         report.addError(
-            bindingElements("with @IntoMap must not %s framework types", bindingElementTypeVerb()));
+            bindingElements("with @IntoMap must not %s framework types: %s",
+                bindingElementTypeVerb(), XTypes.toStableString(bindingType)));
+      }
+      checkDisallowedType(bindingType);
+    }
+
+    private void checkDisallowedType(XType bindingType) {
+      // TODO(erichang): Consider if we want to go inside complex types to ban
+      // dagger.internal.Provider as well? E.g. List<dagger.internal.Provider<Foo>>
+      if (FrameworkTypes.isDisallowedType(bindingType)) {
+        report.addError(bindingElements("must not %s disallowed types: %s",
+            bindingElementTypeVerb(), XTypes.toStableString(bindingElementType().get())));
       }
     }
   }
diff --git a/java/dagger/internal/codegen/validation/DependencyRequestValidator.java b/java/dagger/internal/codegen/validation/DependencyRequestValidator.java
@@ -18,7 +18,9 @@
 
 import static androidx.room.compiler.processing.XElementKt.isField;
 import static androidx.room.compiler.processing.XElementKt.isTypeElement;
+import static dagger.internal.codegen.base.FrameworkTypes.isDisallowedType;
 import static dagger.internal.codegen.base.FrameworkTypes.isFrameworkType;
+import static dagger.internal.codegen.base.FrameworkTypes.isMapValueFrameworkType;
 import static dagger.internal.codegen.base.RequestKinds.extractKeyType;
 import static dagger.internal.codegen.binding.AssistedInjectionAnnotations.isAssistedFactoryType;
 import static dagger.internal.codegen.binding.AssistedInjectionAnnotations.isAssistedInjectionType;
@@ -40,6 +42,7 @@
 import androidx.room.compiler.processing.XVariableElement;
 import com.google.common.collect.ImmutableSet;
 import dagger.internal.codegen.base.FrameworkTypes;
+import dagger.internal.codegen.base.MapType;
 import dagger.internal.codegen.base.RequestKinds;
 import dagger.internal.codegen.binding.InjectionAnnotations;
 import dagger.internal.codegen.javapoet.TypeNames;
@@ -154,6 +157,14 @@ private void checkType() {
         // will just be noise.
         return;
       }
+      if (isDisallowedType(requestType)) {
+        report.addError(
+            "Dagger disallows injecting the type: " + XTypes.toStableString(requestType),
+            requestElement);
+        // If the requested type is a disallowed type then skip the remaining checks as they
+        // will just be noise.
+        return;
+      }
       XType keyType = extractKeyType(requestType);
       if (qualifiers.isEmpty() && isDeclared(keyType)) {
         XTypeElement typeElement = keyType.getTypeElement();
@@ -191,6 +202,24 @@ && isAssistedFactoryType(typeElement)) {
                   requestElement, keyType.getTypeArguments().get(0)));
         }
       }
+      if (MapType.isMap(keyType)) {
+        MapType mapType = MapType.from(keyType);
+        if (!mapType.isRawType()) {
+          XType valueType = mapType.valueType();
+          if (isMapValueFrameworkType(valueType) && isRawParameterizedType(valueType)) {
+            report.addError(
+                "Dagger does not support injecting maps of raw framework types: "
+                + XTypes.toStableString(requestType),
+                requestElement);
+          }
+          if (isDisallowedType(valueType)) {
+            report.addError(
+                "Dagger does not support injecting maps of disallowed types: "
+                + XTypes.toStableString(requestType),
+                requestElement);
+          }
+        }
+      }
     }
   }
 
diff --git a/javatests/dagger/internal/codegen/BindsInstanceValidationTest.java b/javatests/dagger/internal/codegen/BindsInstanceValidationTest.java
@@ -187,4 +187,28 @@ public void bindsInstanceFrameworkType() {
             });
   }
 
+  @Test
+  public void bindsInstanceDaggerProvider() {
+    Source bindsDaggerProvider =
+        CompilerTests.javaSource(
+            "test.BindsInstanceFrameworkType",
+            "package test;",
+            "",
+            "import dagger.BindsInstance;",
+            "import dagger.internal.Provider;",
+            "import dagger.producers.Producer;",
+            "",
+            "interface BindsInstanceFrameworkType {",
+            "  @BindsInstance void bindsProvider(Provider<Object> objectProvider);",
+            "}");
+    CompilerTests.daggerCompiler(bindsDaggerProvider)
+        .withProcessingOptions(compilerMode.processorOptions())
+        .compile(
+            subject -> {
+              subject.hasErrorCount(1);
+              subject.hasErrorContaining("@BindsInstance parameters must not be disallowed types")
+                  .onSource(bindsDaggerProvider)
+                  .onLine(8);
+            });
+  }
 }
diff --git a/javatests/dagger/internal/codegen/MembersInjectionTest.java b/javatests/dagger/internal/codegen/MembersInjectionTest.java
diff --git a/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java b/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java
