This is a rollforward with fixes.

Add a flag -Adagger.strictMultibindingValidation that will enforce that map binding contributions bound in a parent component cannot access bindings from child components. This fixes other bugs that may result from this unintentionally previously supported behavior. This flag is default off but will become the default in the future.

RELNOTES=Add -Adagger.strictMultibindingValidation to fix map binding contributions that depend on subcomponent bindings.
PiperOrigin-RevId: 330633408

Author: Chang-Eric

java/dagger/internal/codegen/binding/BindingGraphFactory.java

@@ -46,8 +46,10 @@
 import dagger.MembersInjector;
 import dagger.Reusable;
 import dagger.internal.codegen.base.ClearableCache;
+import dagger.internal.codegen.base.ContributionType;
 import dagger.internal.codegen.base.MapType;
 import dagger.internal.codegen.base.OptionalType;
+import dagger.internal.codegen.compileroption.CompilerOptions;
 import dagger.internal.codegen.langmodel.DaggerElements;
 import dagger.model.DependencyRequest;
 import dagger.model.Key;
@@ -84,6 +86,7 @@ public final class BindingGraphFactory implements ClearableCache {
   private final ModuleDescriptor.Factory moduleDescriptorFactory;
   private final BindingGraphConverter bindingGraphConverter;
   private final Map<Key, ImmutableSet<Key>> keysMatchingRequestCache = new HashMap<>();
+  private final CompilerOptions compilerOptions;
 
   @Inject
   BindingGraphFactory(
@@ -92,13 +95,15 @@ public final class BindingGraphFactory implements ClearableCache {
       KeyFactory keyFactory,
       BindingFactory bindingFactory,
       ModuleDescriptor.Factory moduleDescriptorFactory,
-      BindingGraphConverter bindingGraphConverter) {
+      BindingGraphConverter bindingGraphConverter,
+      CompilerOptions compilerOptions) {
     this.elements = elements;
     this.injectBindingRegistry = injectBindingRegistry;
     this.keyFactory = keyFactory;
     this.bindingFactory = bindingFactory;
     this.moduleDescriptorFactory = moduleDescriptorFactory;
     this.bindingGraphConverter = bindingGraphConverter;
+    this.compilerOptions = compilerOptions;
   }
 
   /**
@@ -702,12 +707,25 @@ private boolean containsExplicitBinding(ContributionBinding binding) {
 
     /** Returns true if {@code binding} was installed in a module in this resolver's component. */
     private boolean resolverContainsDelegateDeclarationForBinding(ContributionBinding binding) {
-      return binding.kind().equals(DELEGATE)
-          && delegateDeclarations.get(binding.key()).stream()
-              .anyMatch(
-                  declaration ->
-                      declaration.contributingModule().equals(binding.contributingModule())
-                          && declaration.bindingElement().equals(binding.bindingElement()));
+      if (!binding.kind().equals(DELEGATE)) {
+        return false;
+      }
+
+      // Map multibinding key values are wrapped with a framework type. This needs to be undone
+      // to look it up in the delegate declarations map.
+      // TODO(erichang): See if we can standardize the way map keys are used in these data
+      // structures, either always wrapped or unwrapped to be consistent and less errorprone.
+      Key bindingKey = binding.key();
+      if (compilerOptions.strictMultibindingValidation()
+          && binding.contributionType().equals(ContributionType.MAP)) {
+        bindingKey = keyFactory.unwrapMapValueType(bindingKey);
+      }
+
+      return delegateDeclarations.get(bindingKey).stream()
+          .anyMatch(
+              declaration ->
+                  declaration.contributingModule().equals(binding.contributingModule())
+                  && declaration.bindingElement().equals(binding.bindingElement()));
     }
 
     /** Returns the resolver lineage from parent to child. */

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

@@ -95,4 +95,11 @@ public final boolean doCheckForNulls() {
   public int keysPerComponentShard(TypeElement component) {
     return 3500;
   }
+
+  /**
+   * This option enables a fix to an issue where Dagger previously would erroneously allow
+   * multibinding contributions in a component to have dependencies on child components. This will
+   * eventually become the default and enforced.
+   */
+  public abstract boolean strictMultibindingValidation();
 }

java/dagger/internal/codegen/compileroption/JavacPluginCompilerOptions.java

@@ -113,4 +113,9 @@ public ValidationType explicitBindingConflictsWithInjectValidationType() {
   public boolean experimentalDaggerErrorMessages() {
     return false;
   }
+
+  @Override
+  public boolean strictMultibindingValidation() {
+    return false;
+  }
 }

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

@@ -30,6 +30,7 @@
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.FORMAT_GENERATED_SOURCE;
 import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.IGNORE_PRIVATE_AND_STATIC_INJECTION_FOR_COMPONENT;
 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.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;
@@ -174,6 +175,11 @@ public boolean experimentalDaggerErrorMessages() {
     return isEnabled(EXPERIMENTAL_DAGGER_ERROR_MESSAGES);
   }
 
+  @Override
+  public boolean strictMultibindingValidation() {
+    return isEnabled(STRICT_MULTIBINDING_VALIDATION);
+  }
+
   @Override
   public int keysPerComponentShard(TypeElement component) {
     if (processingEnvironment.getOptions().containsKey(KEYS_PER_COMPONENT_SHARD)) {
@@ -299,6 +305,8 @@ enum Feature implements EnumOption<FeatureStatus> {
 
     EXPERIMENTAL_DAGGER_ERROR_MESSAGES,
 
+    STRICT_MULTIBINDING_VALIDATION,
+
     VALIDATE_TRANSITIVE_COMPONENT_DEPENDENCIES(ENABLED)
     ;