Repository: square/anvil (kotlin)

Issue:
Support replacing contributed classes without a direct reference in tests
Tests (unit tests and android tests) have this unfortunate situation that their classpath consists of the main classpath of this module plus test specific dependencies. This leads to duplicate bindings, if a type is implemented in two different modules and one of them should be used for tests.
```
:lib-a -> @ContriburesBinding class A : Interface
:lib-b -> @ContriburesBinding class B : Interface

:app
 -> implementation(':lib-a')
 -> testImplementation(':lib-b')
```
Anvil provides currently two solutions. Either the annotation for `class B` replaces `class A`, or the component in the test sources excludes `class A`. For the former `:lib-b` must depend on `:lib-a` in order to reference `class A`. That's often not desired. The 2nd option doesn't scale well. This exclusion has to be added for every test target and that's exactly what Anvil tries to solve.

There are multiple options how this can be solved. 
* The annotation could have a new attribute that say to prefer this binding / contributed class in test compilations. If two classes have this attribute set, then fail.
* Have a priority (integer) for contributions. Fail if conflicts use the same priority. 
* Allow replacing contributions with a String rather than a class reference. 
* ...

Pull request:
Add a priority field to @ContributesBinding to replace other contributed bindings. The priority should only be used, if you don't have access to the replaced class at compile time. Otherwise prefer the `replaces` field.

Fixes #161

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dc15daec1..a8ea0f4ad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 * `@ContributesBinding` supports qualifiers now, see the README and documentation for examples.  
 * Upgrade Dagger to `2.32`. Generating factories for assisted injection is no longer compatible with older Dagger versions due to the behavior change in Dagger itself. Make sure to use Dagger version `2.32` or newer in your project, too.
+* `@ContributesBinding` has a priority field now for cases where you don't have access to replaced bindings at compile time, see #161.  
 * Use the mangled function name to generate the factory for a provider method.
 
 ## 2.1.0 (2021-02-05)
diff --git a/annotations/src/main/java/com/squareup/anvil/annotations/ContributesBinding.kt b/annotations/src/main/java/com/squareup/anvil/annotations/ContributesBinding.kt
index 31f558725..e2059433d 100644
--- a/annotations/src/main/java/com/squareup/anvil/annotations/ContributesBinding.kt
+++ b/annotations/src/main/java/com/squareup/anvil/annotations/ContributesBinding.kt
@@ -1,5 +1,6 @@
 package com.squareup.anvil.annotations
 
+import com.squareup.anvil.annotations.ContributesBinding.Priority.NORMAL
 import kotlin.annotation.AnnotationRetention.RUNTIME
 import kotlin.annotation.AnnotationTarget.CLASS
 import kotlin.reflect.KClass
@@ -72,6 +73,10 @@ import kotlin.reflect.KClass
  * )
  * class FakeAuthenticator @Inject constructor() : Authenticator
  * ```
+ * If you don't have access to the class of another contributed binding that you want to replace,
+ * then you can change the [priority] of the bindings to avoid duplicate bindings. The contributed
+ * binding with the higher priority will be used.
+ *
  * [ContributesBinding] supports Kotlin objects, e.g.
  * ```
  * @ContributesBinding(AppScope::class)
@@ -98,6 +103,23 @@ public annotation class ContributesBinding(
    * use the same scope.
    */
   val replaces: Array<KClass<*>> = [],
+  /**
+   * The priority of this contributed binding. The priority should be changed only if you don't
+   * have access to the contributed binding class that you want to replace at compile time. If
+   * you have access and can reference the other class, then it's highly suggested to
+   * use [replaces] instead.
+   *
+   * In case of a duplicate binding for multiple contributed bindings the binding with the highest
+   * priority will be used and replace other contributed bindings for the same type with a lower
+   * priority. If duplicate contributed bindings use the same priority, then there will be an
+   * error for duplicate bindings.
+   *
+   * Note that [replaces] takes precedence. If you explicitly replace a binding it won't be
+   * considered no matter what its priority is.
+   *
+   * All contributed bindings have a [NORMAL] priority by default.
+   */
+  val priority: Priority = NORMAL,
   /**
    * Whether the qualifier for this class should be included in the generated binding method. This
    * parameter is only necessary to use when [ContributesBinding] and [ContributesMultibinding]
@@ -105,4 +127,12 @@ public annotation class ContributesBinding(
    * and don't use this parameter.
    */
   val ignoreQualifier: Boolean = false
-)
+) {
+  /**
+   * The priority of a contributed binding.
+   */
+  @Suppress("unused")
+  public enum class Priority {
+    NORMAL, HIGH, HIGHEST
+  }
+}
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 ca32aa654..809b05630 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/Utils.kt
@@ -1,6 +1,7 @@
 package com.squareup.anvil.compiler
 
 import com.squareup.anvil.annotations.ContributesBinding
+import com.squareup.anvil.annotations.ContributesBinding.Priority.NORMAL
 import com.squareup.anvil.annotations.ContributesMultibinding
 import com.squareup.anvil.annotations.ContributesTo
 import com.squareup.anvil.annotations.MergeComponent
@@ -35,6 +36,7 @@ import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.constants.ArrayValue
 import org.jetbrains.kotlin.resolve.constants.BooleanValue
 import org.jetbrains.kotlin.resolve.constants.ConstantValue
+import org.jetbrains.kotlin.resolve.constants.EnumValue
 import org.jetbrains.kotlin.resolve.constants.KClassValue
 import org.jetbrains.kotlin.resolve.constants.KClassValue.Value.NormalClass
 import org.jetbrains.kotlin.resolve.descriptorUtil.annotationClass
@@ -214,6 +216,11 @@ internal fun AnnotationDescriptor.boundType(
   )
 }
 
+internal fun AnnotationDescriptor.priority(): ContributesBinding.Priority {
+  val enumValue = getAnnotationValue("priority") as? EnumValue ?: return NORMAL
+  return ContributesBinding.Priority.valueOf(enumValue.enumEntryName.asString())
+}
+
 internal fun AnnotationDescriptor.ignoreQualifier(): Boolean {
   return (getAnnotationValue("ignoreQualifier") as? BooleanValue)?.value ?: false
 }
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 f3960dcfc..4f2f0c685 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
@@ -29,6 +29,7 @@ import com.squareup.anvil.compiler.isQualifier
 import com.squareup.anvil.compiler.mergeComponentFqName
 import com.squareup.anvil.compiler.mergeModulesFqName
 import com.squareup.anvil.compiler.mergeSubcomponentFqName
+import com.squareup.anvil.compiler.priority
 import com.squareup.anvil.compiler.replaces
 import com.squareup.anvil.compiler.scope
 import com.squareup.kotlinpoet.AnnotationSpec
@@ -43,6 +44,7 @@ import dagger.multibindings.IntoMap
 import dagger.multibindings.IntoSet
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.descriptors.ModuleDescriptor
+import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.KtClassLiteralExpression
 import org.jetbrains.kotlin.psi.KtClassOrObject
@@ -237,6 +239,25 @@ internal class BindingModuleGenerator(
             checkExtendsBoundType(type = contributedClass, boundType = boundType)
             checkNotGeneric(type = contributedClass, boundTypeDescriptor = boundType)
 
+            Binding(contributedClass, annotation, boundType)
+          }
+          .let { bindings ->
+            if (isMultibinding) {
+              bindings
+            } else {
+              bindings
+                .groupBy {
+                  // It's safe to use the FqName here, because we don't allow generic super types.
+                  // If we wouldn't have this limitation, we'd need to use the Kotlin type.
+                  it.boundType.fqNameSafe
+                }
+                .map {
+                  it.value.findHighestPriorityBinding()
+                }
+                .asSequence()
+            }
+          }
+          .map { (contributedClass, annotation, boundType) ->
             val concreteType = contributedClass.fqNameSafe
 
             val qualifiers = if (annotation.ignoreQualifier()) {
@@ -473,4 +494,31 @@ private sealed class GeneratedMethod {
   class BindingMethod(override val spec: FunSpec) : GeneratedMethod()
 }
 
+private data class Binding(
+  val contributedClass: ClassDescriptor,
+  val annotation: AnnotationDescriptor,
+  val boundType: ClassDescriptor
+)
+
+private fun List<Binding>.findHighestPriorityBinding(): Binding {
+  if (size == 1) return this[0]
+
+  val bindings = groupBy { it.annotation.priority() }
+    .toSortedMap()
+    .let { it.getValue(it.lastKey()) }
+
+  if (bindings.size > 1) {
+    throw AnvilCompilationException(
+      "There are multiple contributed bindings with the same bound type. The bound type is " +
+        "${bindings[0].boundType.fqNameSafe}. The contributed binding classes are: " +
+        bindings.joinToString(
+          prefix = "[",
+          postfix = "]"
+        ) { it.contributedClass.fqNameSafe.asString() }
+    )
+  }
+
+  return bindings[0]
+}
+
 private val Collection<GeneratedMethod>.specs get() = map { it.spec }


Test patch:
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModulePriorityTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModulePriorityTest.kt
new file mode 100644
index 000000000..df66cced5
--- /dev/null
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/codegen/BindingModulePriorityTest.kt
@@ -0,0 +1,158 @@
+package com.squareup.anvil.compiler.codegen
+
+import com.google.common.truth.Truth.assertThat
+import com.squareup.anvil.annotations.MergeComponent
+import com.squareup.anvil.annotations.MergeSubcomponent
+import com.squareup.anvil.annotations.compat.MergeModules
+import com.squareup.anvil.compiler.compile
+import com.squareup.anvil.compiler.componentInterfaceAnvilModule
+import com.squareup.anvil.compiler.contributingInterface
+import com.squareup.anvil.compiler.parentInterface
+import com.squareup.anvil.compiler.secondContributingInterface
+import com.tschuchort.compiletesting.KotlinCompilation.ExitCode.COMPILATION_ERROR
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.junit.runners.Parameterized.Parameters
+import kotlin.reflect.KClass
+
+@RunWith(Parameterized::class)
+class BindingModulePriorityTest(
+  annotationClass: KClass<*>
+) {
+
+  private val annotation = "@${annotationClass.simpleName}"
+  private val import = "import ${annotationClass.java.canonicalName}"
+
+  companion object {
+    @Parameters(name = "{0}")
+    @JvmStatic fun annotationClasses(): Collection<Any> {
+      return listOf(MergeComponent::class, MergeSubcomponent::class, MergeModules::class)
+    }
+  }
+
+  @Test fun `the binding with the higher priority is used`() {
+    compile(
+      """
+      package com.squareup.test
+      
+      import com.squareup.anvil.annotations.ContributesBinding
+      import com.squareup.anvil.annotations.ContributesBinding.Priority.HIGH
+      import com.squareup.anvil.annotations.ContributesBinding.Priority.HIGHEST
+      $import
+      
+      interface ParentInterface
+      
+      @ContributesBinding(Any::class, priority = HIGHEST)
+      interface ContributingInterface : ParentInterface
+      
+      @ContributesBinding(Any::class, priority = HIGH)
+      interface ContributingInterface2 : ParentInterface
+      
+      @ContributesBinding(Any::class)
+      interface ContributingInterface3 : ParentInterface
+      
+      $annotation(Any::class)
+      interface ComponentInterface
+      """
+    ) {
+      val bindingMethod = componentInterfaceAnvilModule.declaredMethods.single()
+
+      with(bindingMethod) {
+        assertThat(returnType).isEqualTo(parentInterface)
+        assertThat(parameterTypes.toList()).containsExactly(contributingInterface)
+      }
+    }
+  }
+
+  @Test fun `the binding with the higher priority is used with one replaced binding`() {
+    compile(
+      """
+      package com.squareup.test
+      
+      import com.squareup.anvil.annotations.ContributesBinding
+      import com.squareup.anvil.annotations.ContributesBinding.Priority.HIGH
+      import com.squareup.anvil.annotations.ContributesBinding.Priority.HIGHEST
+      $import
+      
+      interface ParentInterface
+      
+      @ContributesBinding(Any::class, priority = HIGHEST)
+      interface ContributingInterface : ParentInterface
+      
+      @ContributesBinding(Any::class, priority = HIGH, replaces = [ContributingInterface::class])
+      interface SecondContributingInterface : ParentInterface
+      
+      @ContributesBinding(Any::class)
+      interface ContributingInterface3 : ParentInterface
+      
+      $annotation(Any::class)
+      interface ComponentInterface
+      """
+    ) {
+      val bindingMethod = componentInterfaceAnvilModule.declaredMethods.single()
+
+      with(bindingMethod) {
+        assertThat(returnType).isEqualTo(parentInterface)
+        assertThat(parameterTypes.toList()).containsExactly(secondContributingInterface)
+      }
+    }
+  }
+
+  @Test fun `bindings with the same priority throw an error`() {
+    compile(
+      """
+      package com.squareup.test
+      
+      import com.squareup.anvil.annotations.ContributesBinding
+      $import
+      
+      interface ParentInterface
+      
+      @ContributesBinding(Any::class)
+      interface ContributingInterface : ParentInterface
+      
+      @ContributesBinding(Any::class)
+      interface SecondContributingInterface : ParentInterface
+      
+      $annotation(Any::class)
+      interface ComponentInterface
+      """
+    ) {
+      assertThat(exitCode).isEqualTo(COMPILATION_ERROR)
+
+      assertThat(messages).contains(
+        "There are multiple contributed bindings with the same bound type. The bound type is " +
+          "com.squareup.test.ParentInterface. The contributed binding classes are: ["
+      )
+      // Check the contributed bindings separately, we cannot rely on the order in the string.
+      assertThat(messages).contains("com.squareup.test.ContributingInterface")
+      assertThat(messages).contains("com.squareup.test.SecondContributingInterface")
+    }
+  }
+
+  @Test fun `multiple multibindings with the same type are allowed`() {
+    compile(
+      """
+      package com.squareup.test
+      
+      import com.squareup.anvil.annotations.ContributesMultibinding
+      $import
+      
+      interface ParentInterface
+      
+      @ContributesMultibinding(Any::class)
+      interface ContributingInterface : ParentInterface
+      
+      @ContributesMultibinding(Any::class)
+      interface SecondContributingInterface : ParentInterface
+      
+      $annotation(Any::class)
+      interface ComponentInterface
+      """
+    ) {
+      val bindingMethods = componentInterfaceAnvilModule.declaredMethods
+      assertThat(bindingMethods).hasLength(2)
+    }
+  }
+}
diff --git a/integration-tests/library/src/main/java/com/squareup/anvil/test/PriorityBindings.kt b/integration-tests/library/src/main/java/com/squareup/anvil/test/PriorityBindings.kt
new file mode 100644
index 000000000..f620f05e1
--- /dev/null
+++ b/integration-tests/library/src/main/java/com/squareup/anvil/test/PriorityBindings.kt
@@ -0,0 +1,15 @@
+package com.squareup.anvil.test
+
+import com.squareup.anvil.annotations.ContributesBinding
+import com.squareup.anvil.annotations.ContributesBinding.Priority.HIGH
+
+public interface PriorityBinding
+
+@ContributesBinding(
+  scope = AppScope::class,
+  priority = HIGH
+)
+public object PriorityBindingHigh : PriorityBinding
+
+@ContributesBinding(AppScope::class)
+public object PriorityBindingNormal : PriorityBinding
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 065f5fbd6..e2da0d34c 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
@@ -57,6 +57,11 @@ internal class MergeComponentTest {
     assertThat(appComponent.mapBindingsNamed()).containsExactly("2", MapBinding2)
   }
 
+  @Test fun `the binding with the highest priority is bound`() {
+    val appComponent = DaggerMergeComponentTest_AppComponent.create()
+    assertThat(appComponent.priorityBinding()).isEqualTo(PriorityBindingHigh)
+  }
+
   @MergeComponent(AppScope::class)
   @Singleton
   @Suppress("unused")
@@ -68,6 +73,7 @@ internal class MergeComponentTest {
     fun parentTypes(): Set<ParentType>
     fun mapBindings(): Map<String, ParentType>
     @Named("abc") fun mapBindingsNamed(): Map<String, ParentType>
+    fun priorityBinding(): PriorityBinding
   }
 
   @MergeSubcomponent(SubScope::class)