Disallow providing or injecting `dagger.internal.Provider`. This will help with IDEs accidentally suggesting importing `dagger.internal.Provider`.
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
index e607f76..04eaf2e 100644
--- a/java/dagger/internal/codegen/base/FrameworkTypes.java
+++ b/java/dagger/internal/codegen/base/FrameworkTypes.java
@@ -55,6 +55,12 @@
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 @@
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
index 63ecb64..93f4f07 100644
--- 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 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 @@
// 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,7 +369,8 @@
*/
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())));
}
}
@@ -368,16 +378,28 @@
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
index 5510048..87bcad0 100644
--- 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 @@
// 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 @@
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
index 18cc734..a3a7ffa 100644
--- a/javatests/dagger/internal/codegen/BindsInstanceValidationTest.java
+++ b/javatests/dagger/internal/codegen/BindsInstanceValidationTest.java
@@ -187,4 +187,28 @@
});
}
+ @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
index b547ec0..ee02f9f 100644
--- a/javatests/dagger/internal/codegen/MembersInjectionTest.java
+++ b/javatests/dagger/internal/codegen/MembersInjectionTest.java
@@ -779,33 +779,6 @@
}
@Test
- public void rawFrameworkTypeField() {
- Source file =
- CompilerTests.javaSource(
- "test.RawFrameworkTypes",
- "package test;",
- "",
- "import javax.inject.Inject;",
- "import javax.inject.Provider;",
- "",
- "class RawProviderField {",
- " @Inject",
- " Provider fieldWithRawProvider;",
- "}");
-
- CompilerTests.daggerCompiler(file)
- .withProcessingOptions(compilerMode.processorOptions())
- .compile(
- subject -> {
- subject.hasErrorCount(1);
- subject.hasErrorContaining(
- "Dagger does not support injecting raw type: javax.inject.Provider")
- .onSource(file)
- .onLineContaining("Provider fieldWithRawProvider");
- });
- }
-
- @Test
public void throwExceptionInjectedMethod() {
Source file =
CompilerTests.javaSource(
@@ -831,10 +804,37 @@
}
@Test
+ public void rawFrameworkTypeField() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.RawProviderField",
+ "package test;",
+ "",
+ "import javax.inject.Inject;",
+ "import javax.inject.Provider;",
+ "",
+ "class RawProviderField {",
+ " @Inject",
+ " Provider fieldWithRawProvider;",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger does not support injecting raw type: javax.inject.Provider")
+ .onSource(file)
+ .onLineContaining("Provider fieldWithRawProvider");
+ });
+ }
+
+ @Test
public void rawFrameworkMethodTypeParameter() {
Source file =
CompilerTests.javaSource(
- "test.RawFrameworkTypes",
+ "test.RawProviderParameter",
"package test;",
"",
"import javax.inject.Inject;",
@@ -862,7 +862,7 @@
public void rawFrameworkConstructorTypeParameter() {
Source file =
CompilerTests.javaSource(
- "test.RawFrameworkTypes",
+ "test.RawProviderParameter",
"package test;",
"",
"import dagger.Component;",
@@ -888,6 +888,274 @@
}
@Test
+ public void rawMapFrameworkConstructorTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.RawMapProviderParameter",
+ "package test;",
+ "",
+ "import dagger.Component;",
+ "import javax.inject.Inject;",
+ "import javax.inject.Provider;",
+ "import java.util.Map;",
+ "",
+ "class RawMapProviderParameter {",
+ " @Inject",
+ " RawMapProviderParameter(",
+ " Map<String, Provider> rawProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger does not support injecting maps of raw framework types: "
+ + "java.util.Map<java.lang.String,javax.inject.Provider>")
+ .onSource(file)
+ .onLineContaining("Map<String, Provider> rawProviderParameter");
+ });
+ }
+
+ @Test
+ public void daggerProviderField() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.DaggerProviderField",
+ "package test;",
+ "",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "",
+ "class DaggerProviderField {",
+ " @Inject",
+ " Provider<String> fieldWithDaggerProvider;",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger disallows injecting the type: "
+ + "dagger.internal.Provider<java.lang.String>")
+ .onSource(file)
+ .onLineContaining("Provider<String> fieldWithDaggerProvider");
+ });
+ }
+
+ @Test
+ public void daggerProviderMethodTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.DaggerProviderParameter",
+ "package test;",
+ "",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "",
+ "class DaggerProviderParameter {",
+ " @Inject",
+ " void methodInjection(",
+ " Provider<String> daggerProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger disallows injecting the type: "
+ + "dagger.internal.Provider<java.lang.String>")
+ .onSource(file)
+ .onLineContaining("Provider<String> daggerProviderParameter");
+ });
+ }
+
+ @Test
+ public void daggerProviderConstructorTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.DaggerProviderParameter",
+ "package test;",
+ "",
+ "import dagger.Component;",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "",
+ "class DaggerProviderParameter {",
+ " @Inject",
+ " DaggerProviderParameter(",
+ " Provider<String> daggerProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger disallows injecting the type: "
+ + "dagger.internal.Provider<java.lang.String>")
+ .onSource(file)
+ .onLineContaining("Provider<String> daggerProviderParameter");
+ });
+ }
+
+ @Test
+ public void rawDaggerProviderConstructorTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.RawDaggerProviderParameter",
+ "package test;",
+ "",
+ "import dagger.Component;",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "",
+ "class RawDaggerProviderParameter {",
+ " @Inject",
+ " RawDaggerProviderParameter(",
+ " Provider rawDaggerProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger disallows injecting the type: dagger.internal.Provider")
+ .onSource(file)
+ .onLineContaining("Provider rawDaggerProviderParameter");
+ });
+ }
+
+ @Test
+ public void daggerMapProviderField() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.DaggerMapProviderField",
+ "package test;",
+ "",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "import java.util.Map;",
+ "",
+ "class DaggerMapProviderField {",
+ " @Inject",
+ " Map<String, Provider<Long>> fieldWithDaggerMapProvider;",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger does not support injecting maps of disallowed types: "
+ + "java.util.Map<java.lang.String,dagger.internal.Provider<java.lang.Long>>")
+ .onSource(file)
+ .onLineContaining("Map<String, Provider<Long>> fieldWithDaggerMapProvider");
+ });
+ }
+
+ @Test
+ public void daggerMapProviderMethodTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.DaggerMapProviderParameter",
+ "package test;",
+ "",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "import java.util.Map;",
+ "",
+ "class DaggerMapProviderParameter {",
+ " @Inject",
+ " void methodInjection(",
+ " Map<String, Provider<Long>> daggerMapProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger does not support injecting maps of disallowed types: "
+ + "java.util.Map<java.lang.String,dagger.internal.Provider<java.lang.Long>>")
+ .onSource(file)
+ .onLineContaining("Map<String, Provider<Long>> daggerMapProviderParameter");
+ });
+ }
+
+ @Test
+ public void daggerMapProviderConstructorTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.DaggerMapProviderParameter",
+ "package test;",
+ "",
+ "import dagger.Component;",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "import java.util.Map;",
+ "",
+ "class DaggerMapProviderParameter {",
+ " @Inject",
+ " DaggerMapProviderParameter(",
+ " Map<String, Provider<Long>> daggerMapProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger does not support injecting maps of disallowed types: "
+ + "java.util.Map<java.lang.String,dagger.internal.Provider<java.lang.Long>>")
+ .onSource(file)
+ .onLineContaining("Map<String, Provider<Long>> daggerMapProviderParameter");
+ });
+ }
+
+ @Test
+ public void rawDaggerMapProviderConstructorTypeParameter() {
+ Source file =
+ CompilerTests.javaSource(
+ "test.RawDaggerMapProviderParameter",
+ "package test;",
+ "",
+ "import dagger.Component;",
+ "import dagger.internal.Provider;",
+ "import javax.inject.Inject;",
+ "import java.util.Map;",
+ "",
+ "class RawDaggerMapProviderParameter {",
+ " @Inject",
+ " RawDaggerMapProviderParameter(",
+ " Map<String, Provider> rawDaggerMapProviderParameter) {}",
+ "}");
+
+ CompilerTests.daggerCompiler(file)
+ .withProcessingOptions(compilerMode.processorOptions())
+ .compile(
+ subject -> {
+ subject.hasErrorCount(1);
+ subject.hasErrorContaining(
+ "Dagger does not support injecting maps of disallowed types: "
+ + "java.util.Map<java.lang.String,dagger.internal.Provider>")
+ .onSource(file)
+ .onLineContaining("Map<String, Provider> rawDaggerMapProviderParameter");
+ });
+ }
+
+ @Test
public void injectsPrimitive() throws Exception {
Source injectedType =
CompilerTests.javaSource(
diff --git a/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java b/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java
index d315b77..ccff741 100644
--- a/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java
+++ b/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java
@@ -78,6 +78,19 @@
}
@Test
+ public void providesMethodReturnsDaggerInternalProvider() {
+ assertThatModuleMethod("@Provides dagger.internal.Provider<String> provideProvider() {}")
+ .hasError("@Provides methods must not return disallowed types");
+ }
+
+ @Test
+ public void providesIntoSetMethodReturnsDaggerInternalProvider() {
+ assertThatModuleMethod(
+ "@Provides @IntoSet dagger.internal.Provider<String> provideProvider() {}")
+ .hasError("@Provides methods must not return disallowed types");
+ }
+
+ @Test
public void providesMethodReturnsLazy() {
assertThatModuleMethod("@Provides Lazy<String> provideLazy() {}")
.hasError("@Provides methods must not return framework types");
@@ -118,12 +131,31 @@
.hasError("@Provides methods annotated with @ElementsIntoSet cannot return a raw Set");
}
+ @Test public void providesElementsIntoSetMethodReturnsSetDaggerProvider() {
+ assertThatModuleMethod(
+ "@Provides @ElementsIntoSet Set<dagger.internal.Provider<String>> provideProvider() {}")
+ .hasError("@Provides methods must not return disallowed types");
+ }
+
@Test public void providesMethodSetValuesNotASet() {
assertThatModuleMethod(
"@Provides @ElementsIntoSet List<String> provideStrings() { return null; }")
.hasError("@Provides methods annotated with @ElementsIntoSet must return a Set");
}
+ @Test
+ public void bindsMethodReturnsProvider() {
+ assertThatModuleMethod("@Binds abstract Provider<Number> bindsProvider(Provider<Long> impl);")
+ .hasError("@Binds methods must not return framework types");
+ }
+
+ @Test
+ public void bindsMethodReturnsDaggerProvider() {
+ assertThatModuleMethod("@Binds abstract dagger.internal.Provider<Number> "
+ + "bindsProvider(dagger.internal.Provider<Long> impl);")
+ .hasError("@Binds methods must not return disallowed types");
+ }
+
@Test public void modulesWithTypeParamsMustBeAbstract() {
Source moduleFile =
CompilerTests.javaSource(
