Repository: square/anvil (kotlin)

Issue:
@ContributesBinding with multi-component scope not working
Anvil 2.4.0:

Given the following:

    @MergeComponent(ScopeA::class)
    @MergeComponent(ComboScope::class)
    interface ComponentA

    @MergeComponent(ScopeB::class)
    @MergeComponent(ComboScope::class)
    interface ComponentB

    @ContributesBinding(ComboScope::class, boundType = D::class)
    class C : D(), E, F

When I try to inject `D` somewhere within `ComponentA` or `ComponentB` Dagger compilation complains about the binding missing. This problem does not occur when I use individual `@ContributesBinding` annotations for `ScopeA` and `ScopeB` instead of the single `ComboScope` annotation. `ComboScope` also works fine in other contexts, just not in this particular `@ContributesBinding` case.

Pull request:
Fixes #596

Patch:
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/AnnotationReferenceExtensions.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/AnnotationReferenceExtensions.kt
index e2bb8a609..269999054 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/AnnotationReferenceExtensions.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/AnnotationReferenceExtensions.kt
@@ -13,6 +13,7 @@ import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionA
 import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionClassReference
 import com.squareup.anvil.compiler.internal.reference.ClassReference
 import com.squareup.anvil.compiler.internal.reference.argumentAt
+import com.squareup.anvil.compiler.isAnvilModule
 import com.squareup.anvil.compiler.mergeComponentFqName
 import com.squareup.anvil.compiler.mergeModulesFqName
 import com.squareup.anvil.compiler.mergeSubcomponentFqName
@@ -100,6 +101,9 @@ internal fun <T : AnnotationReference> List<T>.checkNoDuplicateScope(
   if (size < 2) return
   if (size == 2 && this[0].scope() != this[1].scope()) return
 
+  // Ignore generated Anvil modules. We'll throw a better error later for @Merge* annotations.
+  if (this[0].declaringClass().fqName.isAnvilModule()) return
+
   // Check for duplicate scopes. Multiple contributions to the same scope are forbidden.
   val duplicates = groupBy { it.scope() }.filterValues { it.size > 1 }
 
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/BindingModuleGenerator.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/BindingModuleGenerator.kt
index a07d3ed81..4eb48941d 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/BindingModuleGenerator.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/BindingModuleGenerator.kt
@@ -53,9 +53,7 @@ internal class BindingModuleGenerator(
 
   // Keeps track of for which scopes which files were generated. Usually there is only one file,
   // but technically there can be multiple.
-  private val mergedClasses = mutableMapOf<MergedClassKey, File>()
-
-  private val excludedTypesForClass = mutableMapOf<MergedClassKey, List<ClassReference>>()
+  private val mergedClasses = mutableMapOf<ClassReference, File>()
 
   private val contributedBindingClasses = mutableListOf<ClassReference>()
   private val contributedMultibindingClasses = mutableListOf<ClassReference>()
@@ -87,24 +85,9 @@ internal class BindingModuleGenerator(
     // Generate a Dagger module for each @MergeComponent and friends.
     return classes
       .mapNotNull { clazz ->
-        supportedFqNames
-          .firstNotNullOfOrNull { supportedFqName ->
-            clazz.annotations.firstOrNull { it.fqName == supportedFqName }
-          }
-          ?.let { clazz to it }
-      }
-      .map { (clazz, mergeAnnotation) ->
-        val scope = mergeAnnotation.scope()
-        val mergedClassKey = MergedClassKey(scope, clazz)
-
-        // Remember for which scopes which types were excluded so that we later don't generate
-        // a binding method for these types.
-        mergeAnnotation
-          .exclude()
-          .takeIf { it.isNotEmpty() }
-          ?.let { excludedReferences ->
-            excludedTypesForClass[mergedClassKey] = excludedReferences
-          }
+        val mergeAnnotations = clazz.annotations
+          .filter { it.fqName in supportedFqNames }
+          .ifEmpty { return@mapNotNull null }
 
         val packageName = generatePackageName(clazz)
         val className = clazz.generateClassName().relativeClassName.asString()
@@ -122,12 +105,12 @@ internal class BindingModuleGenerator(
         }
 
         val content = daggerModuleContent(
-          scope = scope.fqName.asString(),
+          scopes = mergeAnnotations.map { it.scope() },
           clazz = clazz,
           generatedMethods = emptyList()
         )
 
-        mergedClasses[mergedClassKey] = file
+        mergedClasses[clazz] = file
 
         GeneratedFile(file, content)
       }
@@ -138,11 +121,10 @@ internal class BindingModuleGenerator(
     codeGenDir: File,
     module: ModuleDescriptor
   ): Collection<GeneratedFile> {
-    return mergedClasses.map { (mergedClassKey, daggerModuleFile) ->
-      val scope = mergedClassKey.scope
-
-      // Precompute this list once per class since it's expensive.
-      val excludedNames = excludedTypesForClass[mergedClassKey].orEmpty()
+    return mergedClasses.map { (mergedClass, daggerModuleFile) ->
+      val annotations = mergedClass.annotations.filter { it.fqName in supportedFqNames }
+      val scopes = annotations.map { it.scope() }
+      val excludedClasses = annotations.flatMap { it.exclude() }
 
       // Contributed Dagger modules can replace other Dagger modules but also contributed bindings.
       // If a binding is replaced, then we must not generate the binding method.
@@ -152,20 +134,24 @@ internal class BindingModuleGenerator(
       val bindingsReplacedInDaggerModules = contributedModuleClasses
         .asSequence()
         .plus(
-          classScanner
-            .findContributedClasses(
-              module = module,
-              annotation = contributesToFqName,
-              scope = scope.fqName
-            )
-            .filter { it.isAnnotatedWith(daggerModuleFqName) }
+          scopes.flatMap { scope ->
+            classScanner
+              .findContributedClasses(
+                module = module,
+                annotation = contributesToFqName,
+                scope = scope.fqName
+              )
+              .filter { it.isAnnotatedWith(daggerModuleFqName) }
+          }
         )
         // Ignore replaced bindings specified by excluded modules for this scope.
-        .filter { clazz -> clazz !in excludedNames }
+        .filter { clazz -> clazz !in excludedClasses }
         .flatMap { clazz ->
-          clazz.annotations
-            .find(annotationName = contributesToFqName, scope = scope)
-            .flatMap { it.replaces() }
+          scopes.flatMap { scope ->
+            clazz.annotations
+              .find(annotationName = contributesToFqName, scope = scope)
+              .flatMap { it.replaces() }
+          }
         }
         .toList()
 
@@ -179,31 +165,37 @@ internal class BindingModuleGenerator(
         isMultibinding: Boolean
       ): List<GeneratedMethod> {
 
-        val contributedBindingsDependencies = classScanner.findContributedClasses(
-          module,
-          annotation = annotationFqName,
-          scope = scope.fqName
-        )
+        val contributedBindingsDependencies = scopes.flatMap { scope ->
+          classScanner.findContributedClasses(
+            module,
+            annotation = annotationFqName,
+            scope = scope.fqName
+          )
+        }
 
         val allContributedClasses = collectedClasses
           .plus(contributedBindingsDependencies)
 
         val replacedBindings = allContributedClasses
           .flatMap { classReference ->
-            classReference.annotations
-              .find(annotationName = annotationFqName, scope = scope)
-              .flatMap { it.replaces() }
+            scopes.flatMap { scope ->
+              classReference.annotations
+                .find(annotationName = annotationFqName, scope = scope)
+                .flatMap { it.replaces() }
+            }
           }
 
         return allContributedClasses
           .asSequence()
           .filterNot { it in replacedBindings }
           .filterNot { it in bindingsReplacedInDaggerModules }
-          .filterNot { it in excludedNames }
+          .filterNot { it in excludedClasses }
           .flatMap { clazz ->
-            clazz.annotations
-              .find(annotationName = annotationFqName, scope = scope)
-              .map { it.toContributedBinding(isMultibinding) }
+            scopes.flatMap { scope ->
+              clazz.annotations
+                .find(annotationName = annotationFqName, scope = scope)
+                .map { it.toContributedBinding(isMultibinding) }
+            }
           }
           .let { bindings ->
             if (isMultibinding) {
@@ -229,8 +221,8 @@ internal class BindingModuleGenerator(
       )
 
       val content = daggerModuleContent(
-        scope = scope.fqName.asString(),
-        clazz = mergedClassKey.clazz,
+        scopes = scopes,
+        clazz = mergedClass,
         generatedMethods = generatedMethods
       )
 
@@ -241,7 +233,7 @@ internal class BindingModuleGenerator(
   }
 
   private fun daggerModuleContent(
-    scope: String,
+    scopes: List<ClassReference>,
     clazz: ClassReference,
     generatedMethods: List<GeneratedMethod>
   ): String {
@@ -274,12 +266,16 @@ internal class BindingModuleGenerator(
       addType(
         builder
           .addAnnotation(Module::class)
-          .addAnnotation(
-            AnnotationSpec
-              .builder(ContributesTo::class)
-              .addMember("$scope::class")
-              .build()
-          )
+          .apply {
+            scopes.forEach { scope ->
+              addAnnotation(
+                AnnotationSpec
+                  .builder(ContributesTo::class)
+                  .addMember("%T::class", scope.asClassName())
+                  .build()
+              )
+            }
+          }
           .build()
       )
     }
@@ -388,8 +384,3 @@ private fun ContributedBinding.toGeneratedMethod(
 }
 
 private val Collection<GeneratedMethod>.specs get() = map { it.spec }
-
-private data class MergedClassKey(
-  val scope: ClassReference,
-  val clazz: ClassReference
-)


Test patch:
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModuleGeneratorTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModuleGeneratorTest.kt
index 082bca453..9fa72b807 100644
--- a/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModuleGeneratorTest.kt
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModuleGeneratorTest.kt
@@ -406,7 +406,8 @@ class BindingModuleGeneratorTest(
     }
   }
 
-  @Test fun `the contributed binding class must not have a generic type parameter with super type chain`() {
+  @Test
+  fun `the contributed binding class must not have a generic type parameter with super type chain`() {
     compile(
       """
       package com.squareup.test
@@ -597,7 +598,8 @@ class BindingModuleGeneratorTest(
     }
   }
 
-  @Test fun `methods are generated for bindings contributed to multiple scopes with multiple compilations`() {
+  @Test
+  fun `methods are generated for bindings contributed to multiple scopes with multiple compilations`() {
     assumeIrBackend()
 
     val previousResult = compile(
@@ -750,7 +752,8 @@ class BindingModuleGeneratorTest(
     }
   }
 
-  @Test fun `bindings contributed to multiple scopes can be replaced by other contributed bindings`() {
+  @Test
+  fun `bindings contributed to multiple scopes can be replaced by other contributed bindings`() {
     assumeIrBackend()
 
     compile(
@@ -883,6 +886,76 @@ class BindingModuleGeneratorTest(
     }
   }
 
+  @Test fun `a contributed binding is merged in multiple scopes`() {
+    assumeIrBackend()
+
+    compile(
+      """
+      package com.squareup.test
+      
+      import com.squareup.anvil.annotations.ContributesBinding
+      $import
+
+      interface ParentInterface
+
+      @ContributesBinding(Unit::class)
+      interface ContributingInterface : ParentInterface
+      
+      $annotation(Any::class)
+      $annotation(Unit::class)
+      interface ComponentInterface
+      
+      $annotation(Int::class)
+      $annotation(Unit::class)
+      interface SubcomponentInterface
+      """
+    ) {
+      listOf(componentInterface, subcomponentInterface).forEach { component ->
+        with(component.anvilModule.declaredMethods.single()) {
+          assertThat(returnType).isEqualTo(parentInterface)
+          assertThat(parameterTypes.toList()).containsExactly(contributingInterface)
+          assertThat(isAbstract).isTrue()
+          assertThat(isAnnotationPresent(Binds::class.java)).isTrue()
+        }
+      }
+    }
+  }
+
+  @Test fun `a contributed binding can be excluded in one component`() {
+    assumeIrBackend()
+
+    compile(
+      """
+      package com.squareup.test
+      
+      import com.squareup.anvil.annotations.ContributesBinding
+      $import
+
+      interface ParentInterface
+
+      @ContributesBinding(Unit::class)
+      @ContributesBinding(Any::class)
+      interface ContributingInterface : ParentInterface
+      
+      $annotation(Any::class)
+      $annotation(Unit::class)
+      interface ComponentInterface
+      
+      $annotation(Any::class)
+      $annotation(Unit::class, exclude = [ContributingInterface::class])
+      interface SubcomponentInterface
+      """
+    ) {
+      with(componentInterface.anvilModule.declaredMethods.single()) {
+        assertThat(returnType).isEqualTo(parentInterface)
+        assertThat(parameterTypes.toList()).containsExactly(contributingInterface)
+        assertThat(isAbstract).isTrue()
+        assertThat(isAnnotationPresent(Binds::class.java)).isTrue()
+      }
+      assertThat(subcomponentInterface.anvilModule.declaredMethods).isEmpty()
+    }
+  }
+
   private val Class<*>.anyDaggerComponent: AnyDaggerComponent
     get() = anyDaggerComponent(annotationClass)
 }