Repository: square/anvil
Pull request #50 (2020-07-31 02:03:14)

Make the bound type optional
This PR depends on #49 


Patch:
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/ModuleMerger.kt b/compiler/src/main/java/com/squareup/anvil/compiler/ModuleMerger.kt
index bc24ee8d..6cf4f024 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/ModuleMerger.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/ModuleMerger.kt
@@ -7,6 +7,7 @@ import com.squareup.anvil.annotations.compat.MergeModules
 import dagger.Component
 import dagger.Module
 import dagger.Subcomponent
+import org.jetbrains.kotlin.backend.common.serialization.findPackage
 import org.jetbrains.kotlin.codegen.ImplementationBodyCodegen
 import org.jetbrains.kotlin.codegen.extensions.ExpressionCodegenExtension
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
@@ -20,6 +21,7 @@ import org.jetbrains.kotlin.resolve.constants.ConstantValue
 import org.jetbrains.kotlin.resolve.constants.KClassValue
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
 import org.jetbrains.kotlin.resolve.descriptorUtil.module
+import org.jetbrains.kotlin.resolve.descriptorUtil.parentsWithSelf
 import org.jetbrains.org.objectweb.asm.AnnotationVisitor
 import org.jetbrains.org.objectweb.asm.Type
 import kotlin.reflect.KClass
@@ -52,6 +54,8 @@ internal class ModuleMerger(
           ?.value
           ?.map { it.toType(codegen) }
 
+    val anvilModuleName = createAnvilModuleName(codegen.descriptor)
+
     val modules = classScanner
         .findContributedClasses(
             module = codegen.descriptor.module,
@@ -59,6 +63,17 @@ internal class ModuleMerger(
             annotation = contributesToFqName
         )
         .asSequence()
+        .filter {
+          // We generate a Dagger module for each merged component. We use Anvil itself to
+          // contribute this generated module. It's possible that there are multiple components
+          // merging the same scope or the same scope is merged in different Gradle modules which
+          // depend on each other. This would cause duplicate bindings, because the generated
+          // modules contain the same bindings and are contributed to the same scope. To avoid this
+          // issue we filter all generated Anvil modules except for the one that was generated for
+          // this specific class.
+          val fqName = it.fqNameSafe
+          !fqName.isAnvilModule() || fqName == anvilModuleName
+        }
         .mapNotNull {
           val contributesAnnotation =
             it.annotationOrNull(contributesToFqName, scope = scope) ?: return@mapNotNull null
@@ -173,6 +188,20 @@ internal class ModuleMerger(
     block(this)
     visitEnd()
   }
+
+  private fun createAnvilModuleName(classDescriptor: ClassDescriptor): FqName {
+    val name = "$MODULE_PACKAGE_PREFIX.${classDescriptor.findPackage().fqName}." +
+        classDescriptor.parentsWithSelf
+            .filterIsInstance<ClassDescriptor>()
+            .toList()
+            .reversed()
+            .joinToString(separator = "", postfix = ANVIL_MODULE_SUFFIX) {
+              it.fqNameSafe
+                  .shortName()
+                  .asString()
+            }
+    return FqName(name)
+  }
 }
 
 @Suppress("DataClassPrivateConstructor")
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt b/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
index be920ce8..dbae8442 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
@@ -23,6 +23,8 @@ import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.constants.ConstantValue
 import org.jetbrains.kotlin.resolve.constants.KClassValue
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
+import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassNotAny
+import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperInterfaces
 import org.jetbrains.kotlin.resolve.descriptorUtil.module
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.org.objectweb.asm.Type
@@ -42,6 +44,8 @@ internal const val HINT_CONTRIBUTES_PACKAGE_PREFIX = "hint.anvil"
 internal const val HINT_BINDING_PACKAGE_PREFIX = "anvil.hint.binding"
 internal const val MODULE_PACKAGE_PREFIX = "anvil.module"
 
+internal const val ANVIL_MODULE_SUFFIX = "AnvilModule"
+
 internal fun ClassDescriptor.annotationOrNull(
   annotationFqName: FqName,
   scope: ClassDescriptor? = null
@@ -87,6 +91,11 @@ internal fun ConstantValue<*>.toType(
 // When the Kotlin type is of the form: KClass<OurType>.
 internal fun KotlinType.argumentType(): KotlinType = arguments.first().type
 
+internal fun FqName.isAnvilModule(): Boolean {
+  val name = asString()
+  return name.startsWith(MODULE_PACKAGE_PREFIX) && name.endsWith(ANVIL_MODULE_SUFFIX)
+}
+
 internal fun AnnotationDescriptor.getAnnotationValue(key: String): ConstantValue<*>? =
   allValueArguments[Name.identifier(key)]
 
@@ -98,10 +107,26 @@ internal fun AnnotationDescriptor.scope(module: ModuleDescriptor): ClassDescript
   )
 }
 
-internal fun AnnotationDescriptor.boundType(module: ModuleDescriptor): ClassDescriptor {
-  val kClassValue = requireNotNull(getAnnotationValue("boundType")) as KClassValue
-  return DescriptorUtils.getClassDescriptorForType(
-      kClassValue.getType(module)
-          .argumentType()
-  )
+internal fun AnnotationDescriptor.boundType(
+  module: ModuleDescriptor,
+  annotatedClass: ClassDescriptor
+): ClassDescriptor {
+  (getAnnotationValue("boundType") as? KClassValue)
+      ?.getType(module)
+      ?.argumentType()
+      ?.let { return DescriptorUtils.getClassDescriptorForType(it) }
+
+  val directSuperTypes = annotatedClass.getSuperInterfaces() +
+      (annotatedClass.getSuperClassNotAny()
+          ?.let { listOf(it) }
+          ?: emptyList())
+
+  return directSuperTypes.singleOrNull()
+      ?: throw AnvilCompilationException(
+          classDescriptor = annotatedClass,
+          message = "${annotatedClass.fqNameSafe} contributes a binding, but does not " +
+              "specify the bound type. This is only allowed with one direct super type. If " +
+              "there are multiple, then the bound type must be explicitly defined in the " +
+              "@${contributesBindingFqName.shortName()} annotation."
+      )
 }
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 8f80ffaf..1549a4e2 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
@@ -1,5 +1,6 @@
 package com.squareup.anvil.compiler.codegen
 
+import com.squareup.anvil.compiler.ANVIL_MODULE_SUFFIX
 import com.squareup.anvil.compiler.AnvilCompilationException
 import com.squareup.anvil.compiler.ClassScanner
 import com.squareup.anvil.compiler.HINT_BINDING_PACKAGE_PREFIX
@@ -132,7 +133,7 @@ internal class BindingModuleGenerator(
           }
           .map { contributedClass ->
             val annotation = contributedClass.annotation(contributesBindingFqName)
-            val boundType = annotation.boundType(module)
+            val boundType = annotation.boundType(module, contributedClass)
 
             checkExtendsBoundType(type = contributedClass, boundType = boundType)
 
@@ -206,7 +207,7 @@ internal class BindingModuleGenerator(
         .filterIsInstance<KtClassOrObject>()
         .toList()
         .reversed()
-        .joinToString(separator = "", postfix = "AnvilModule") {
+        .joinToString(separator = "", postfix = ANVIL_MODULE_SUFFIX) {
           it.requireFqName()
               .shortName()
               .asString()
diff --git a/sample/library/src/main/java/com/squareup/anvil/sample/mother/MotherProviderModule.kt b/sample/library/src/main/java/com/squareup/anvil/sample/mother/MotherProviderModule.kt
deleted file mode 100644
index 2a661616..00000000
--- a/sample/library/src/main/java/com/squareup/anvil/sample/mother/MotherProviderModule.kt
+++ /dev/null
@@ -1,12 +0,0 @@
-package com.squareup.anvil.sample.mother
-
-import com.squareup.anvil.annotations.ContributesTo
-import com.squareup.scopes.AppScope
-import dagger.Binds
-import dagger.Module
-
-@Module
-@ContributesTo(AppScope::class)
-abstract class MotherProviderModule {
-  @Binds abstract fun bindMotherProvider(provider: RealMotherProvider): MotherProvider
-}
diff --git a/sample/library/src/main/java/com/squareup/anvil/sample/mother/RealMotherProvider.kt b/sample/library/src/main/java/com/squareup/anvil/sample/mother/RealMotherProvider.kt
index 929e6a1a..b84b6ebf 100644
--- a/sample/library/src/main/java/com/squareup/anvil/sample/mother/RealMotherProvider.kt
+++ b/sample/library/src/main/java/com/squareup/anvil/sample/mother/RealMotherProvider.kt
@@ -1,11 +1,14 @@
 package com.squareup.anvil.sample.mother
 
+import com.squareup.anvil.annotations.ContributesBinding
 import com.squareup.anvil.sample.God
 import com.squareup.anvil.sample.God.HEPHAESTUS
 import com.squareup.anvil.sample.God.HERA
 import com.squareup.anvil.sample.God.ZEUS
+import com.squareup.scopes.AppScope
 import javax.inject.Inject
 
+@ContributesBinding(AppScope::class)
 class RealMotherProvider @Inject constructor() : MotherProvider {
   override fun mother(god: God): String =
     when (god) {


Test patch:
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/TestUtils.kt b/compiler/src/test/java/com/squareup/anvil/compiler/TestUtils.kt
index 5b4f0dd2..f066dcf1 100644
--- a/compiler/src/test/java/com/squareup/anvil/compiler/TestUtils.kt
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/TestUtils.kt
@@ -9,6 +9,7 @@ import com.tschuchort.compiletesting.SourceFile
 import dagger.Component
 import dagger.Module
 import dagger.Subcomponent
+import org.jetbrains.kotlin.name.FqName
 import org.junit.Assume.assumeTrue
 import java.io.File
 import java.util.Locale.US
@@ -154,4 +155,4 @@ internal fun assumeMergeComponent(annotationClass: KClass<*>) {
 
 internal fun Array<KClass<*>>.withoutAnvilModule(): List<KClass<*>> = toList().withoutAnvilModule()
 internal fun Collection<KClass<*>>.withoutAnvilModule(): List<KClass<*>> =
-  filterNot { it.qualifiedName!!.startsWith(MODULE_PACKAGE_PREFIX) }
+  filterNot { FqName(it.qualifiedName!!).isAnvilModule() }
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 0abb9a4b..adc7ebc8 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
@@ -173,6 +173,102 @@ class BindingModuleGeneratorTest(
     }
   }
 
+  @Test fun `the bound type is implied when there is only one super type`() {
+    compile(
+        """
+        package com.squareup.test
+        
+        import com.squareup.anvil.annotations.ContributesBinding
+        $import
+
+        interface ParentInterface
+
+        @ContributesBinding(Any::class)
+        interface ContributingInterface : ParentInterface
+        
+        $annotation(Any::class)
+        interface ComponentInterface
+    """
+    ) {
+      val modules = if (annotationClass == MergeModules::class) {
+        componentInterface.daggerModule.includes.toList()
+      } else {
+        componentInterface.anyDaggerComponent.modules
+      }
+      assertThat(modules).containsExactly(componentInterfaceAnvilModule.kotlin)
+
+      val methods = modules.first().java.declaredMethods
+      assertThat(methods).hasLength(1)
+
+      with(methods[0]) {
+        assertThat(returnType).isEqualTo(parentInterface)
+        assertThat(parameterTypes.toList()).containsExactly(contributingInterface)
+        assertThat(isAbstract(modifiers)).isTrue()
+        assertThat(isAnnotationPresent(Binds::class.java)).isTrue()
+      }
+    }
+  }
+
+  @Test fun `the bound type can only be implied with on super type (2 interfaces)`() {
+    compile(
+        """
+        package com.squareup.test
+        
+        import com.squareup.anvil.annotations.ContributesBinding
+        $import
+
+        interface ParentInterface
+        
+        @ContributesBinding(Any::class)
+        interface ContributingInterface : ParentInterface, CharSequence
+        
+        $annotation(Any::class)
+        interface ComponentInterface
+    """
+    ) {
+      assertThat(exitCode).isEqualTo(COMPILATION_ERROR)
+
+      assertThat(messages).contains("Source.kt: (9, 11)")
+      assertThat(messages).contains(
+          "com.squareup.test.ContributingInterface contributes a binding, but does not specify " +
+              "the bound type. This is only allowed with one direct super type. If there are " +
+              "multiple, then the bound type must be explicitly defined in the " +
+              "@ContributesBinding annotation."
+      )
+    }
+  }
+
+  @Test fun `the bound type can only be implied with on super type (class and interface)`() {
+    compile(
+        """
+        package com.squareup.test
+        
+        import com.squareup.anvil.annotations.ContributesBinding
+        $import
+
+        interface ParentInterface
+        
+        open class Abc
+        
+        @ContributesBinding(Any::class)
+        interface ContributingInterface : Abc(), ParentInterface
+        
+        $annotation(Any::class)
+        interface ComponentInterface
+    """
+    ) {
+      assertThat(exitCode).isEqualTo(COMPILATION_ERROR)
+
+      assertThat(messages).contains("Source.kt: (11, 11)")
+      assertThat(messages).contains(
+          "com.squareup.test.ContributingInterface contributes a binding, but does not specify " +
+              "the bound type. This is only allowed with one direct super type. If there are " +
+              "multiple, then the bound type must be explicitly defined in the " +
+              "@ContributesBinding annotation."
+      )
+    }
+  }
+
   @Test fun `the contributed binding class must extend the bound type`() {
     compile(
         """
diff --git a/integration-tests/library/src/main/java/com/squareup/anvil/test/Bindings.kt b/integration-tests/library/src/main/java/com/squareup/anvil/test/Bindings.kt
new file mode 100644
index 00000000..423626a5
--- /dev/null
+++ b/integration-tests/library/src/main/java/com/squareup/anvil/test/Bindings.kt
@@ -0,0 +1,17 @@
+package com.squareup.anvil.test
+
+import com.squareup.anvil.annotations.ContributesBinding
+import javax.inject.Inject
+
+interface ParentType
+
+interface MiddleType : ParentType
+
+@ContributesBinding(
+    scope = AppScope::class,
+    boundType = ParentType::class
+)
+class AppBinding @Inject constructor() : MiddleType
+
+@ContributesBinding(SubScope::class)
+class SubcomponentBinding @Inject constructor() : MiddleType
diff --git a/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeComponentTest.kt b/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeComponentTest.kt
index 970d5505..71882ba3 100644
--- a/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeComponentTest.kt
+++ b/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeComponentTest.kt
@@ -29,11 +29,24 @@ class MergeComponentTest {
     assertThat(AppComponent::class extends SubModule2::class).isFalse()
   }
 
+  @Test fun `contributed bindings bind types for each scope`() {
+    val appComponent = DaggerMergeComponentTest_AppComponent.create()
+    val subComponent = appComponent.subComponent()
+
+    assertThat(appComponent.parentType()).isInstanceOf(AppBinding::class.java)
+    assertThat(subComponent.parentType()).isInstanceOf(AppBinding::class.java)
+    assertThat(subComponent.middleType()).isInstanceOf(SubcomponentBinding::class.java)
+  }
+
   @MergeComponent(AppScope::class)
   interface AppComponent {
     fun subComponent(): SubComponent
+    fun parentType(): ParentType
   }
 
   @MergeSubcomponent(SubScope::class)
-  interface SubComponent
+  interface SubComponent {
+    fun middleType(): MiddleType
+    fun parentType(): ParentType
+  }
 }
diff --git a/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeModulesTest.kt b/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeModulesTest.kt
index 2dbc8f33..9cbb4fc9 100644
--- a/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeModulesTest.kt
+++ b/integration-tests/tests/src/test/java/com/squareup/anvil/test/MergeModulesTest.kt
@@ -2,7 +2,9 @@ package com.squareup.anvil.test
 
 import com.google.common.truth.Truth.assertThat
 import com.squareup.anvil.annotations.compat.MergeModules
+import dagger.Component
 import dagger.Module
+import dagger.Subcomponent
 import org.junit.Test
 
 class MergeModulesTest {
@@ -23,9 +25,30 @@ class MergeModulesTest {
     assertThat(annotation.includes.toList()).doesNotContain(AppModule2::class)
   }
 
+  @Test fun `contributed bindings bind types for each scope`() {
+    val appComponent = DaggerMergeModulesTest_AppComponent.create()
+    val subComponent = appComponent.subComponent()
+
+    assertThat(appComponent.parentType()).isInstanceOf(AppBinding::class.java)
+    assertThat(subComponent.parentType()).isInstanceOf(AppBinding::class.java)
+    assertThat(subComponent.middleType()).isInstanceOf(SubcomponentBinding::class.java)
+  }
+
   @MergeModules(AppScope::class)
   abstract class CompositeAppModule
 
   @MergeModules(SubScope::class)
   abstract class CompositeSubModule
+
+  @Component(modules = [CompositeAppModule::class])
+  interface AppComponent {
+    fun subComponent(): SubComponent
+    fun parentType(): ParentType
+  }
+
+  @Subcomponent(modules = [CompositeSubModule::class])
+  interface SubComponent {
+    fun middleType(): MiddleType
+    fun parentType(): ParentType
+  }
 }