Remove framework types from multibinding contribution keys.

This CL removes the framework types from multibinding contribution keys.

Before:

  * `@Provides @IntoMap`: `Map<K,Provider<V>>`
  * `@Produces @IntoMap`: `Map<K,Producer<V>>`
  * `@Binds @IntoMap`: (depends on delegating binding)

After:

  * `@Provides @IntoMap`: `Map<K,V>`
  * `@Produces @IntoMap`: `Map<K,V>`
  * `@Binds @IntoMap`: `Map<K,V>`

I've also added a flag (default enabled) that can be manually disabled to make migrations easier.

RELNOTES=N/A
PiperOrigin-RevId: 678046562

java/dagger/internal/codegen/binding/ComponentDeclarations.java

@@ -79,6 +79,8 @@ ImmutableSet<ContributionBinding> bindings(Key key) {
   ImmutableSet<DelegateDeclaration> delegates(Key key) {
     // @Binds @IntoMap declarations have key Map<K, V> but may be requested as
     // Map<K, Provider/Producer<V>> keys, so unwrap the multibinding map contribution key first.
+    // TODO(b/366277730): This can be simplified to "delegates.get(key)" once the flag for
+    // "useFrameworkTypeInMapMultibindingContributionKey" is removed.
     return delegates.get(
         key.multibindingContributionIdentifier().isPresent()
             // TODO(bcorso): Consider using TypeNameKey here instead of Key, to avoid losing

java/dagger/internal/codegen/binding/KeyFactory.java

@@ -42,6 +42,7 @@
 import dagger.internal.codegen.base.OptionalType;
 import dagger.internal.codegen.base.RequestKinds;
 import dagger.internal.codegen.base.SetType;
+import dagger.internal.codegen.compileroption.CompilerOptions;
 import dagger.internal.codegen.javapoet.TypeNames;
 import dagger.internal.codegen.model.DaggerAnnotation;
 import dagger.internal.codegen.model.DaggerExecutableElement;
@@ -57,11 +58,16 @@
 /** A factory for {@link Key}s. */
 public final class KeyFactory {
   private final XProcessingEnv processingEnv;
+  private final CompilerOptions compilerOptions;
   private final InjectionAnnotations injectionAnnotations;
 
   @Inject
-  KeyFactory(XProcessingEnv processingEnv, InjectionAnnotations injectionAnnotations) {
+  KeyFactory(
+      XProcessingEnv processingEnv,
+      CompilerOptions compilerOptions,
+      InjectionAnnotations injectionAnnotations) {
     this.processingEnv = processingEnv;
+    this.compilerOptions = compilerOptions;
     this.injectionAnnotations = injectionAnnotations;
   }
 
@@ -91,6 +97,10 @@ private XType optionalOf(XType type) {
 
   /** Returns {@code Map<KeyType, FrameworkType<ValueType>>}. */
   private XType mapOfFrameworkType(XType keyType, ClassName frameworkClassName, XType valueType) {
+    checkArgument(
+        MapType.VALID_FRAMEWORK_REQUEST_KINDS.stream()
+            .map(RequestKinds::frameworkClassName)
+            .anyMatch(frameworkClassName::equals));
     return mapOf(
         keyType,
         processingEnv.getDeclaredType(
@@ -120,10 +130,12 @@ public Key forSubcomponentCreator(XType creatorType) {
   }
 
   public Key forProvidesMethod(XMethodElement method, XTypeElement contributingModule) {
+    checkArgument(method.hasAnnotation(TypeNames.PROVIDES));
     return forBindingMethod(method, contributingModule, Optional.of(TypeNames.PROVIDER));
   }
 
   public Key forProducesMethod(XMethodElement method, XTypeElement contributingModule) {
+    checkArgument(method.hasAnnotation(TypeNames.PRODUCES));
     return forBindingMethod(method, contributingModule, Optional.of(TypeNames.PRODUCER));
   }
 
@@ -212,7 +224,8 @@ private XType bindingMethodKeyType(
             method.getAllAnnotations().stream()
                 .map(XAnnotations::toString)
                 .collect(toImmutableList()));
-        return frameworkClassName.isPresent()
+        return (frameworkClassName.isPresent()
+                && compilerOptions.useFrameworkTypeInMapMultibindingContributionKey())
             ? mapOfFrameworkType(mapKeyType.get(), frameworkClassName.get(), returnType)
             : mapOf(mapKeyType.get(), returnType);
       case SET_VALUES:
@@ -226,12 +239,14 @@ private XType bindingMethodKeyType(
   /**
    * Returns the key for a binding associated with a {@link DelegateDeclaration}.
    *
-   * <p>If {@code delegateDeclaration} is {@code @IntoMap}, transforms the {@code Map<K, V>} key
-   * from {@link DelegateDeclaration#key()} to {@code Map<K, FrameworkType<V>>}. If {@code
-   * delegateDeclaration} is not a map contribution, its key is returned.
+   * <p>If {@code delegateDeclaration} is a multibinding map contribution and
+   * {@link CompilerOptions#useFrameworkTypeInMapMultibindingContributionKey()} is enabled, then
+   * transforms the {@code Map<K, V>} key into {@code Map<K, FrameworkType<V>>}, otherwise returns
+   * the unaltered key.
    */
   Key forDelegateBinding(DelegateDeclaration delegateDeclaration, ClassName frameworkType) {
     return delegateDeclaration.contributionType().equals(ContributionType.MAP)
+            && compilerOptions.useFrameworkTypeInMapMultibindingContributionKey()
         ? wrapMapValue(delegateDeclaration.key(), frameworkType)
         : delegateDeclaration.key();
   }

java/dagger/internal/codegen/compileroption/CompilerOptions.java

@@ -129,6 +129,15 @@ public final boolean doCheckForNulls() {
    */
   public abstract boolean generatedClassExtendsComponent();
 
+  /**
+   * Returns {@code true} if the key for map multibinding contributions contain a framework type.
+   *
+   * <p>This option is for migration purposes only, and will be removed in a future release.
+   *
+   * <p>The default value is {@code false}.
+   */
+  public abstract boolean useFrameworkTypeInMapMultibindingContributionKey();
+
   /** Returns the number of bindings allowed per shard. */
   public int keysPerComponentShard(XTypeElement component) {
     return 3500;

java/dagger/internal/codegen/compileroption/ProcessingEnvironmentCompilerOptions.java

@@ -35,6 +35,7 @@
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.PLUGINS_VISIT_FULL_BINDING_GRAPHS;
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.STRICT_MULTIBINDING_VALIDATION;
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.STRICT_SUPERFICIAL_VALIDATION;
+import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.USE_FRAMEWORK_TYPE_IN_MAP_MULTIBINDING_CONTRIBUTION_KEY;
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.VALIDATE_TRANSITIVE_COMPONENT_DEPENDENCIES;
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.WARN_IF_INJECTION_FACTORY_NOT_GENERATED_UPSTREAM;
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.WRITE_PRODUCER_NAME_IN_TOKEN;
@@ -189,6 +190,11 @@ public boolean experimentalDaggerErrorMessages() {
     return isEnabled(EXPERIMENTAL_DAGGER_ERROR_MESSAGES);
   }
 
+  @Override
+  public boolean useFrameworkTypeInMapMultibindingContributionKey() {
+    return isEnabled(USE_FRAMEWORK_TYPE_IN_MAP_MULTIBINDING_CONTRIBUTION_KEY);
+  }
+
   @Override
   public boolean ignoreProvisionKeyWildcards() {
     return isEnabled(IGNORE_PROVISION_KEY_WILDCARDS);
@@ -345,6 +351,8 @@ enum Feature implements EnumOption<FeatureStatus> {
 
     GENERATED_CLASS_EXTENDS_COMPONENT,
 
+    USE_FRAMEWORK_TYPE_IN_MAP_MULTIBINDING_CONTRIBUTION_KEY,
+
     IGNORE_PROVISION_KEY_WILDCARDS(ENABLED),
 
     VALIDATE_TRANSITIVE_COMPONENT_DEPENDENCIES(ENABLED)

java/dagger/internal/codegen/javac/JavacPluginCompilerOptions.java

@@ -121,6 +121,11 @@ public boolean experimentalDaggerErrorMessages() {
     return false;
   }
 
+  @Override
+  public boolean useFrameworkTypeInMapMultibindingContributionKey() {
+    return false;
+  }
+
   @Override
   public boolean strictMultibindingValidation() {
     return false;

javatests/dagger/internal/codegen/MapMultibindingValidationTest.java

@@ -88,7 +88,7 @@ public void duplicateMapKeys_UnwrappedMapKey() {
             subject -> {
               subject.hasErrorCount(1);
               subject.hasErrorContaining(
-                      "The same map key is bound more than once for Map<String,Provider<Object>>")
+                      "The same map key is bound more than once for Map<String,Object>")
                   .onSource(module)
                   .onLineContaining("class MapModule");
               subject.hasErrorContaining("provideObjectForAKey()");
@@ -285,7 +285,7 @@ public void inconsistentMapKeyAnnotations() {
             subject -> {
               subject.hasErrorCount(1);
               subject.hasErrorContaining(
-                      "Map<String,Provider<Object>> uses more than one @MapKey annotation type")
+                      "Map<String,Object> uses more than one @MapKey annotation type")
                   .onSource(module)
                   .onLineContaining("class MapModule");
               subject.hasErrorContaining("provideObjectForAKey()");

javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest_mapBindingsWithEnumKey_FAST_INIT_MODE_test.DaggerTestComponent

@@ -111,10 +111,10 @@ final class DaggerTestComponent {
           case 0: // java.util.Map<test.PathEnum,javax.inject.Provider<test.Handler>> 
           return (T) ImmutableMap.<PathEnum, javax.inject.Provider<Handler>>of(PathEnum.ADMIN, testComponentImpl.provideAdminHandlerProvider, PathEnum.LOGIN, testComponentImpl.provideLoginHandlerProvider);
 
-          case 1: // java.util.Map<test.PathEnum,javax.inject.Provider<test.Handler>> test.MapModuleOne#provideAdminHandler 
+          case 1: // java.util.Map<test.PathEnum,test.Handler> test.MapModuleOne#provideAdminHandler 
           return (T) MapModuleOne_ProvideAdminHandlerFactory.provideAdminHandler(testComponentImpl.mapModuleOne);
 
-          case 2: // java.util.Map<test.PathEnum,javax.inject.Provider<test.Handler>> test.MapModuleTwo#provideLoginHandler 
+          case 2: // java.util.Map<test.PathEnum,test.Handler> test.MapModuleTwo#provideLoginHandler 
           return (T) MapModuleTwo_ProvideLoginHandlerFactory.provideLoginHandler(testComponentImpl.mapModuleTwo);
 
           default: throw new AssertionError(id);

javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest_mapBindingsWithStringKey_FAST_INIT_MODE_test.DaggerTestComponent

@@ -111,10 +111,10 @@ final class DaggerTestComponent {
           case 0: // java.util.Map<java.lang.String,javax.inject.Provider<test.Handler>> 
           return (T) ImmutableMap.<String, javax.inject.Provider<Handler>>of("Admin", testComponentImpl.provideAdminHandlerProvider, "Login", testComponentImpl.provideLoginHandlerProvider);
 
-          case 1: // java.util.Map<java.lang.String,javax.inject.Provider<test.Handler>> test.MapModuleOne#provideAdminHandler 
+          case 1: // java.util.Map<java.lang.String,test.Handler> test.MapModuleOne#provideAdminHandler 
           return (T) MapModuleOne_ProvideAdminHandlerFactory.provideAdminHandler(testComponentImpl.mapModuleOne);
 
-          case 2: // java.util.Map<java.lang.String,javax.inject.Provider<test.Handler>> test.MapModuleTwo#provideLoginHandler 
+          case 2: // java.util.Map<java.lang.String,test.Handler> test.MapModuleTwo#provideLoginHandler 
           return (T) MapModuleTwo_ProvideLoginHandlerFactory.provideLoginHandler(testComponentImpl.mapModuleTwo);
 
           default: throw new AssertionError(id);

javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest_mapBindings_FAST_INIT_MODE_test.DaggerTestComponent

@@ -110,16 +110,16 @@ final class DaggerTestComponent {
       @Override
       public T get() {
         switch (id) {
-          case 0: // java.util.Map<java.lang.Integer,javax.inject.Provider<java.lang.Integer>> test.MapModule#provideInt 
+          case 0: // java.util.Map<java.lang.Integer,java.lang.Integer> test.MapModule#provideInt 
           return (T) (Integer) MapModule.provideInt();
 
-          case 1: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.MapModule#provideLong0 
+          case 1: // java.util.Map<java.lang.Long,java.lang.Long> test.MapModule#provideLong0 
           return (T) (Long) MapModule.provideLong0();
 
-          case 2: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.MapModule#provideLong1 
+          case 2: // java.util.Map<java.lang.Long,java.lang.Long> test.MapModule#provideLong1 
           return (T) (Long) MapModule.provideLong1();
 
-          case 3: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.MapModule#provideLong2 
+          case 3: // java.util.Map<java.lang.Long,java.lang.Long> test.MapModule#provideLong2 
           return (T) (Long) MapModule.provideLong2();
 
           default: throw new AssertionError(id);

javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest_mapBindings_FAST_INIT_MODE_test.DaggerTestComponent

@@ -92,13 +92,13 @@ final class DaggerTestComponent {
       @Override
       public T get() {
         switch (id) {
-          case 0: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.SubcomponentMapModule#provideLong3 
+          case 0: // java.util.Map<java.lang.Long,java.lang.Long> test.SubcomponentMapModule#provideLong3 
           return (T) (Long) SubcomponentMapModule.provideLong3();
 
-          case 1: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.SubcomponentMapModule#provideLong4 
+          case 1: // java.util.Map<java.lang.Long,java.lang.Long> test.SubcomponentMapModule#provideLong4 
           return (T) (Long) SubcomponentMapModule.provideLong4();
 
-          case 2: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.SubcomponentMapModule#provideLong5 
+          case 2: // java.util.Map<java.lang.Long,java.lang.Long> test.SubcomponentMapModule#provideLong5 
           return (T) (Long) SubcomponentMapModule.provideLong5();
 
           default: throw new AssertionError(id);
@@ -181,16 +181,16 @@ final class DaggerTestComponent {
       @Override
       public T get() {
         switch (id) {
-          case 0: // java.util.Map<java.lang.Integer,javax.inject.Provider<java.lang.Integer>> test.MapModule#provideInt 
+          case 0: // java.util.Map<java.lang.Integer,java.lang.Integer> test.MapModule#provideInt 
           return (T) (Integer) MapModule.provideInt();
 
-          case 1: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.MapModule#provideLong0 
+          case 1: // java.util.Map<java.lang.Long,java.lang.Long> test.MapModule#provideLong0 
           return (T) (Long) MapModule.provideLong0();
 
-          case 2: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.MapModule#provideLong1 
+          case 2: // java.util.Map<java.lang.Long,java.lang.Long> test.MapModule#provideLong1 
           return (T) (Long) MapModule.provideLong1();
 
-          case 3: // java.util.Map<java.lang.Long,javax.inject.Provider<java.lang.Long>> test.MapModule#provideLong2 
+          case 3: // java.util.Map<java.lang.Long,java.lang.Long> test.MapModule#provideLong2 
           return (T) (Long) MapModule.provideLong2();
 
           default: throw new AssertionError(id);

test_defs.bzl

@@ -16,18 +16,12 @@
 
 load("@rules_java//java:defs.bzl", "java_library", "java_test")
 load("//:build_defs.bzl", "JAVA_RELEASE_MIN")
-load(
-    "@io_bazel_rules_kotlin//kotlin:kotlin.bzl",
-    "kt_jvm_library",
-    "kt_jvm_test",
-)
 
 # Defines a set of build variants and the list of extra javacopts to build with.
 # The key will be appended to the generated test names to ensure uniqueness.
 _NON_FUNCTIONAL_BUILD_VARIANTS = {None: []}
 _FUNCTIONAL_BUILD_VARIANTS = {
     None: [],  # The default build variant (no javacopts).
-    "ExtendsComponent": ["-Adagger.generatedClassExtendsComponent=enabled"],
     "Shards": ["-Adagger.keysPerComponentShard=2"],
     "FastInit": ["-Adagger.fastInit=enabled"],
     "FastInit_Shards": ["-Adagger.fastInit=enabled", "-Adagger.keysPerComponentShard=2"],