Repository: square/anvil (kotlin)

Issue:
@Binds are not validated with `generateDaggerFactories`
With pure dagger this will lead to a compilation error:

```kotlin
@Module
abstract class OTestModule {

    @Binds
    abstract fun testBinding(impl: Context): StringBuilder
}
```

But when `generateDaggerFactories` is enabled, compilation is successful and we get an error during runtime. 

It would be great to get a compilation failure also with `generateDaggerFactories` enabled.

Pull request:
Fixes #649

Patch:
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 44e2ef5d2..c4ba29888 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
@@ -11,6 +11,7 @@ import com.squareup.anvil.annotations.compat.MergeModules
 import com.squareup.anvil.compiler.internal.fqName
 import com.squareup.anvil.compiler.internal.reference.ClassReference
 import com.squareup.anvil.compiler.internal.reference.toClassReferenceOrNull
+import dagger.Binds
 import dagger.Component
 import dagger.Lazy
 import dagger.MapKey
@@ -45,6 +46,7 @@ internal val daggerSubcomponentFqName = Subcomponent::class.fqName
 internal val daggerSubcomponentFactoryFqName = Subcomponent.Factory::class.fqName
 internal val daggerSubcomponentBuilderFqName = Subcomponent.Builder::class.fqName
 internal val daggerModuleFqName = Module::class.fqName
+internal val daggerBindsFqName = Binds::class.fqName
 internal val daggerProvidesFqName = Provides::class.fqName
 internal val daggerLazyFqName = Lazy::class.fqName
 internal val injectFqName = Inject::class.fqName
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/BindsMethodValidator.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/BindsMethodValidator.kt
new file mode 100644
index 000000000..40daa0865
--- /dev/null
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/BindsMethodValidator.kt
@@ -0,0 +1,85 @@
+package com.squareup.anvil.compiler.codegen.dagger
+
+import com.google.auto.service.AutoService
+import com.squareup.anvil.compiler.api.AnvilContext
+import com.squareup.anvil.compiler.api.CodeGenerator
+import com.squareup.anvil.compiler.codegen.PrivateCodeGenerator
+import com.squareup.anvil.compiler.daggerBindsFqName
+import com.squareup.anvil.compiler.daggerModuleFqName
+import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionFunctionReference
+import com.squareup.anvil.compiler.internal.reference.FunctionReference
+import com.squareup.anvil.compiler.internal.reference.allSuperTypeClassReferences
+import com.squareup.anvil.compiler.internal.reference.classAndInnerClassReferences
+import org.jetbrains.kotlin.descriptors.ModuleDescriptor
+import org.jetbrains.kotlin.psi.KtFile
+import java.io.File
+
+/**
+ * We do not need to generate any code for @Binds methods in the way that we do for @Provides
+ * methods, but we still need to validate them since we are taking Dagger's place in evaluating them
+ * for all compile-time needs. Without performing some validation here, it's possible a binding
+ * that Dagger would consider a compile time failure would instead manifest as a runtime failure
+ * when Anvil generates Dagger factories.
+ */
+@AutoService(CodeGenerator::class)
+internal class BindsMethodValidator : PrivateCodeGenerator() {
+
+  override fun isApplicable(context: AnvilContext) = context.generateFactories
+
+  override fun generateCodePrivate(
+    codeGenDir: File,
+    module: ModuleDescriptor,
+    projectFiles: Collection<KtFile>
+  ) {
+    projectFiles
+      .classAndInnerClassReferences(module)
+      .filter { it.isAnnotatedWith(daggerModuleFqName) }
+      .forEach { clazz ->
+        (clazz.companionObjects() + clazz)
+          .asSequence()
+          .flatMap { it.functions }
+          .filter { it.isAnnotatedWith(daggerBindsFqName) }
+          .also { functions ->
+            assertNoDuplicateFunctions(clazz, functions)
+          }
+          .forEach { function ->
+            validateBindsFunction(function)
+          }
+      }
+  }
+
+  private fun validateBindsFunction(function: FunctionReference.Psi) {
+    if (!function.isAbstract()) {
+      throw AnvilCompilationExceptionFunctionReference(
+        message = "@Binds methods must be abstract",
+        functionReference = function
+      )
+    }
+
+    if (function.parameters.size != 1) {
+      throw AnvilCompilationExceptionFunctionReference(
+        message = "@Binds methods must have exactly one parameter, " +
+          "whose type is assignable to the return type",
+        functionReference = function
+      )
+    }
+
+    val returnType =
+      function.returnTypeOrNull() ?: throw AnvilCompilationExceptionFunctionReference(
+        message = "@Binds methods must return a value (not void)",
+        functionReference = function
+      )
+    val parameterMatchesReturnType = function.parameters.single()
+      .type()
+      .asClassReference()
+      .allSuperTypeClassReferences(includeSelf = true)
+      .contains(returnType.asClassReference())
+
+    if (!parameterMatchesReturnType) {
+      throw AnvilCompilationExceptionFunctionReference(
+        message = "@Binds methods' parameter type must be assignable to the return type",
+        functionReference = function
+      )
+    }
+  }
+}
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/DaggerGenerationUtils.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/DaggerGenerationUtils.kt
index 428994747..36ff67dc6 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/DaggerGenerationUtils.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/DaggerGenerationUtils.kt
@@ -5,8 +5,10 @@ import com.squareup.anvil.compiler.daggerDoubleCheckFqNameString
 import com.squareup.anvil.compiler.daggerLazyFqName
 import com.squareup.anvil.compiler.injectFqName
 import com.squareup.anvil.compiler.internal.capitalize
+import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionClassReference
 import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionPropertyReference
 import com.squareup.anvil.compiler.internal.reference.ClassReference
+import com.squareup.anvil.compiler.internal.reference.FunctionReference
 import com.squareup.anvil.compiler.internal.reference.ParameterReference
 import com.squareup.anvil.compiler.internal.reference.PropertyReference
 import com.squareup.anvil.compiler.internal.reference.TypeParameterReference
@@ -316,3 +318,21 @@ internal fun ClassName.optionallyParameterizedBy(
     parameterizedBy(typeParameters.map { it.typeVariableName })
   }
 }
+
+internal fun assertNoDuplicateFunctions(
+  declaringClass: ClassReference,
+  functions: Sequence<FunctionReference.Psi>
+) {
+  // Check for duplicate function names.
+  val duplicateFunctions = functions
+    .groupBy { it.fqName }
+    .filterValues { it.size > 1 }
+
+  if (duplicateFunctions.isNotEmpty()) {
+    throw AnvilCompilationExceptionClassReference(
+      classReference = declaringClass,
+      message = "Cannot have more than one binding method with the same name in " +
+        "a single module: ${duplicateFunctions.keys.joinToString()}"
+    )
+  }
+}
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/ProvidesMethodFactoryGenerator.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/ProvidesMethodFactoryGenerator.kt
index c1983b855..4caae9122 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/ProvidesMethodFactoryGenerator.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/ProvidesMethodFactoryGenerator.kt
@@ -12,7 +12,6 @@ import com.squareup.anvil.compiler.daggerProvidesFqName
 import com.squareup.anvil.compiler.internal.buildFile
 import com.squareup.anvil.compiler.internal.capitalize
 import com.squareup.anvil.compiler.internal.reference.AnnotatedReference
-import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionClassReference
 import com.squareup.anvil.compiler.internal.reference.AnvilCompilationExceptionFunctionReference
 import com.squareup.anvil.compiler.internal.reference.ClassReference
 import com.squareup.anvil.compiler.internal.reference.FunctionReference
@@ -65,18 +64,7 @@ internal class ProvidesMethodFactoryGenerator : PrivateCodeGenerator() {
             checkFunctionIsNotAbstract(clazz, function)
           }
           .also { functions ->
-            // Check for duplicate function names.
-            val duplicateFunctions = functions
-              .groupBy { it.fqName }
-              .filterValues { it.size > 1 }
-
-            if (duplicateFunctions.isNotEmpty()) {
-              throw AnvilCompilationExceptionClassReference(
-                classReference = clazz,
-                message = "Cannot have more than one binding method with the same name in " +
-                  "a single module: ${duplicateFunctions.keys.joinToString()}"
-              )
-            }
+            assertNoDuplicateFunctions(clazz, functions)
           }
           .forEach { function ->
             generateFactoryClass(


Test patch:
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/BindsMethodValidatorTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/BindsMethodValidatorTest.kt
new file mode 100644
index 000000000..f15f5d023
--- /dev/null
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/BindsMethodValidatorTest.kt
@@ -0,0 +1,180 @@
+package com.squareup.anvil.compiler.dagger
+
+import com.google.common.truth.Truth.assertThat
+import com.squareup.anvil.compiler.USE_IR
+import com.squareup.anvil.compiler.WARNINGS_AS_ERRORS
+import com.squareup.anvil.compiler.internal.testing.compileAnvil
+import com.squareup.anvil.compiler.isError
+import com.squareup.anvil.compiler.isFullTestRun
+import com.tschuchort.compiletesting.KotlinCompilation.Result
+import org.intellij.lang.annotations.Language
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.junit.runners.Parameterized.Parameters
+
+@Suppress("UNCHECKED_CAST")
+@RunWith(Parameterized::class)
+class BindsMethodValidatorTest(
+  private val useDagger: Boolean
+) {
+
+  companion object {
+    @Parameters(name = "Use Dagger: {0}")
+    @JvmStatic fun useDagger(): Collection<Any> {
+      return listOf(isFullTestRun(), false).distinct()
+    }
+  }
+
+  @Test
+  fun `a binding with an incompatible parameter type fails to compile`() {
+    compile(
+      """
+      package com.squareup.test
+ 
+      import dagger.Binds
+      import dagger.Module
+      import javax.inject.Inject
+
+      class Foo @Inject constructor()
+      interface Bar
+
+      @Module
+      abstract class BarModule {
+        @Binds
+        abstract fun bindsBar(impl: Foo): Bar
+      }
+      """
+    ) {
+      assertThat(exitCode).isError()
+      assertThat(messages).contains(
+        "@Binds methods' parameter type must be assignable to the return type"
+      )
+    }
+  }
+
+  @Test
+  fun `a non-abstract binding fails to compile`() {
+    compile(
+      """
+      package com.squareup.test
+ 
+      import dagger.Binds
+      import dagger.Module
+      import javax.inject.Inject
+
+      class Foo @Inject constructor() : Bar
+      interface Bar
+
+      @Module
+      class BarModule {
+        @Binds
+        fun bindsBar(impl: Foo): Bar {
+          return impl
+        }
+      }
+      """
+    ) {
+      assertThat(exitCode).isError()
+      assertThat(messages).contains("@Binds methods must be abstract")
+    }
+  }
+
+  @Test
+  fun `a binding with no parameters fails to compile`() {
+    compile(
+      """
+      package com.squareup.test
+ 
+      import dagger.Binds
+      import dagger.Module
+      import javax.inject.Inject
+
+      class Foo @Inject constructor() : Bar
+      interface Bar
+
+      @Module
+      abstract class BarModule {
+        @Binds
+        abstract fun bindsBar(): Bar
+      }
+      """
+    ) {
+      assertThat(exitCode).isError()
+      assertThat(messages).contains(
+        "@Binds methods must have exactly one parameter, " +
+          "whose type is assignable to the return type"
+      )
+    }
+  }
+
+  @Test
+  fun `a binding with multiple parameters fails to compile`() {
+    compile(
+      """
+      package com.squareup.test
+ 
+      import dagger.Binds
+      import dagger.Module
+      import javax.inject.Inject
+
+      class Foo @Inject constructor() : Bar
+      class Hammer @Inject constructor() : Bar
+      interface Bar
+
+      @Module
+      abstract class BarModule {
+        @Binds
+        abstract fun bindsBar(impl1: Foo, impl2: Hammer): Bar
+      }
+      """
+    ) {
+      assertThat(exitCode).isError()
+      assertThat(messages).contains(
+        "@Binds methods must have exactly one parameter, " +
+          "whose type is assignable to the return type"
+      )
+    }
+  }
+
+  @Test
+  fun `a binding with no return type fails to compile`() {
+    compile(
+      """
+      package com.squareup.test
+ 
+      import dagger.Binds
+      import dagger.Module
+      import javax.inject.Inject
+
+      class Foo @Inject constructor() : Bar
+      interface Bar
+
+      @Module
+      abstract class BarModule {
+        @Binds
+        abstract fun bindsBar(impl1: Foo)
+      }
+      """
+    ) {
+      assertThat(exitCode).isError()
+      assertThat(messages).contains(
+        "@Binds methods must return a value (not void)"
+      )
+    }
+  }
+
+  private fun compile(
+    @Language("kotlin") vararg sources: String,
+    previousCompilationResult: Result? = null,
+    block: Result.() -> Unit = { }
+  ): Result = compileAnvil(
+    sources = sources,
+    enableDaggerAnnotationProcessor = useDagger,
+    generateDaggerFactories = !useDagger,
+    useIR = USE_IR,
+    allWarningsAsErrors = WARNINGS_AS_ERRORS,
+    previousCompilationResult = previousCompilationResult,
+    block = block
+  )
+}