Repository: square/anvil (kotlin)

Issue:
[Generated Factories] Another MembersInjectors for super classes issue?
This looks an awful lot like https://github.com/square/anvil/issues/343

I confirmed this issue still exists when I update to latest Dagger and Anvil.  (2.40.5 and 2.3.10)
```
GradleProject1
--------------
class A : B <-- Well-formed MembersInjector
class B : C <-- broken

GradleProject2
--------------  
class C <-- Well-formed MembersInjector
```

When I inserted an intermediary class into the chain it made it clear that it breaks at the boundary between gradle projects.
```
GradleProject1
--------------
class A : B <-- Well-formed MembersInjector
class B : C <-- broken

GradleProject2
--------------  
class C : D <-- Well-formed MembersInjector
class D <-- Well-formed MembersInjector
```
```
GradleProject1
--------------
class A : B <-- Well-formed MembersInjector
class B : C <-- Well-formed MembersInjector
class C : D <-- broken

GradleProject2
--------------  
class D <-- Well-formed MembersInjector
```

Do I need to make a sample project, or is this good enough?

Pull request:
fixes #438
fixes #439

Patch:
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/DescriptorUtils.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/DescriptorUtils.kt
index 28f2bae34..46f93c111 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/DescriptorUtils.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/DescriptorUtils.kt
@@ -20,6 +20,7 @@ import org.jetbrains.kotlin.descriptors.ModuleDescriptor
 import org.jetbrains.kotlin.descriptors.PropertyDescriptor
 import org.jetbrains.kotlin.descriptors.containingPackage
 import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
 import org.jetbrains.kotlin.resolve.descriptorUtil.getAllSuperClassifiers
@@ -31,12 +32,9 @@ import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 
 internal fun ClassDescriptor.superClassMemberInjectedParameters(
   module: ModuleDescriptor
-): List<MemberInjectParameter> = superClassMemberInjectedDescriptors(module)
-  .mapToMemberInjectParameters(module, asClassName())
+): List<MemberInjectParameter> {
 
-fun ClassDescriptor.superClassMemberInjectedDescriptors(
-  module: ModuleDescriptor
-): List<CallableMemberDescriptor> {
+  val allInjectedPropertyNames = mutableSetOf<Name>()
 
   return getAllSuperClassifiers()
     // no point in parsing android/androidx classes for injected params, so skip them
@@ -45,21 +43,19 @@ fun ClassDescriptor.superClassMemberInjectedDescriptors(
     // returns [ Impl, Base, kotlin.Any ]
     // drop the first element since this function is supposed to return properties from supers.
     .drop(1)
+    .map { it.fqNameSafe.requireClassDescriptor(module) }
     // look for any @Inject-annotated members.
     // Downstream properties are parsed before the properties they override.
     // If a property has already been added from a downstream sub-class, then ignore it.
-    .fold(mutableListOf<CallableMemberDescriptor>()) { acc, classifierDescriptor ->
-
-      classifierDescriptor.fqNameSafe
-        .requireClassDescriptor(module)
+    .flatMap { classDescriptor ->
+      classDescriptor
         .unsubstitutedMemberScope
         .memberInjectedPropertyDescriptors()
-        .forEach { property ->
-          if (acc.none { existing -> existing.name == property.name }) {
-            acc.add(property)
-          }
+        .mapNotNull { property ->
+          // Only parse the property if it hasn't been added already.
+          property.takeIf { allInjectedPropertyNames.add(property.name) }
         }
-      acc
+        .mapToMemberInjectParameters(module, classDescriptor.asClassName())
     }
     .toList()
 }
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectionUtils.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectionUtils.kt
index e0c1f84ce..c2cc5c178 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectionUtils.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectionUtils.kt
@@ -82,9 +82,14 @@ internal fun KtClassOrObject.memberInjectParameters(
 
   val psiSuperClasses = allPsiSuperClasses(module)
 
-  // We start at the bottom of the hierarchy and work our way up.
-  val descriptorParams = psiSuperClasses.lastOrNull()
-    ?.classDescriptorOrNull(module)
+  // If a super class is defined in the same module, select that.
+  // Otherwise, select the current KtClassOrObject.
+  val mostSuperPsi = psiSuperClasses.lastOrNull() ?: this@memberInjectParameters
+
+  // Use the descriptor of the most-super PSI class in order to look for more super types in
+  // other modules.
+  val descriptorParams = mostSuperPsi
+    .classDescriptorOrNull(module)
     ?.superClassMemberInjectedParameters(module)
     .orEmpty()
 


Test patch:
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt
index aef03e5e7..7182000da 100644
--- a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt
@@ -1049,6 +1049,172 @@ public class InjectClass_Factory(
     }
   }
 
+  @Test fun `a factory class performs member injection on a super class from another module`() {
+
+    val otherModuleResult = compile(
+      """
+      package com.squareup.test
+
+      import javax.inject.Inject
+
+      abstract class Base {
+
+        @Inject
+        lateinit var base1: List<Int>
+
+        @Inject
+        lateinit var base2: List<String>
+      }
+      """
+    ) {
+      assertThat(exitCode).isEqualTo(OK)
+    }
+
+    compile(
+      """
+      package com.squareup.test
+
+      import com.squareup.test.Base
+      import javax.inject.Inject
+      import javax.inject.Provider
+      import dagger.Lazy
+
+      class InjectClass @Inject constructor() : Base() {
+
+        @Inject
+        lateinit var name: String
+      }
+      """,
+      previousCompilationResult = otherModuleResult
+    ) {
+
+      val factoryClass = injectClass.factoryClass()
+
+      val constructor = factoryClass.declaredConstructors.single()
+      assertThat(constructor.parameterTypes.toList())
+        .containsExactly(
+          Provider::class.java,
+          Provider::class.java,
+          Provider::class.java
+        )
+
+      val name = "name"
+      val base1 = listOf(3)
+      val base2 = listOf("base2")
+
+      val staticMethods = factoryClass.declaredMethods.filter { it.isStatic }
+
+      val factoryInstance = factoryClass.createInstance(
+        Provider { base1 },
+        Provider { base2 },
+        Provider { name }
+      )
+      assertThat(factoryInstance::class.java).isEqualTo(factoryClass)
+
+      val newInstance = staticMethods.single { it.name == "newInstance" }
+        .invoke(null)
+      assertThat(newInstance).isNotNull()
+
+      val getInstance = (factoryInstance as Factory<*>).get()
+
+      assertThat(getInstance.getPropertyValue("name")).isEqualTo(name)
+      assertThat(getInstance.getPropertyValue("base1")).isEqualTo(base1)
+      assertThat(getInstance.getPropertyValue("base2")).isEqualTo(base2)
+    }
+  }
+
+  @Test
+  fun `a factory class performs member injection on a grand-super class from another module`() {
+
+    val otherModuleResult = compile(
+      """
+      package com.squareup.test
+
+      import javax.inject.Inject
+
+      abstract class Base {
+
+        @Inject
+        lateinit var base1: List<Int>
+
+        @Inject
+        lateinit var base2: List<String>
+      }
+      """
+    ) {
+      assertThat(exitCode).isEqualTo(OK)
+    }
+
+    compile(
+      """
+      package com.squareup.test
+
+      import com.squareup.test.Base
+      import javax.inject.Inject
+      import javax.inject.Provider
+      import dagger.Lazy
+
+      class InjectClass @Inject constructor() : Middle() {
+
+        @Inject
+        lateinit var name: String
+      }
+
+      abstract class Middle : Base() {
+
+        @Inject
+        lateinit var middle1: Set<Int>
+
+        @Inject
+        lateinit var middle2: Set<String>
+      }
+      """,
+      previousCompilationResult = otherModuleResult
+    ) {
+
+      val factoryClass = injectClass.factoryClass()
+
+      val constructor = factoryClass.declaredConstructors.single()
+      assertThat(constructor.parameterTypes.toList())
+        .containsExactly(
+          Provider::class.java,
+          Provider::class.java,
+          Provider::class.java,
+          Provider::class.java,
+          Provider::class.java
+        )
+
+      val name = "name"
+      val middle1 = setOf(1)
+      val middle2 = setOf("middle2")
+      val base1 = listOf(3)
+      val base2 = listOf("base2")
+
+      val staticMethods = factoryClass.declaredMethods.filter { it.isStatic }
+
+      val factoryInstance = factoryClass.createInstance(
+        Provider { base1 },
+        Provider { base2 },
+        Provider { middle1 },
+        Provider { middle2 },
+        Provider { name }
+      )
+      assertThat(factoryInstance::class.java).isEqualTo(factoryClass)
+
+      val newInstance = staticMethods.single { it.name == "newInstance" }
+        .invoke(null)
+      assertThat(newInstance).isNotNull()
+
+      val getInstance = (factoryInstance as Factory<*>).get()
+
+      assertThat(getInstance.getPropertyValue("name")).isEqualTo(name)
+      assertThat(getInstance.getPropertyValue("middle1")).isEqualTo(middle1)
+      assertThat(getInstance.getPropertyValue("middle2")).isEqualTo(middle2)
+      assertThat(getInstance.getPropertyValue("base1")).isEqualTo(base1)
+      assertThat(getInstance.getPropertyValue("base2")).isEqualTo(base2)
+    }
+  }
+
   @Test
   fun `a factory class is generated which injects members when calling get`() {
     /*
@@ -2418,6 +2584,7 @@ public final class InjectClass_Factory implements Factory<InjectClass> {
 
   private fun compile(
     @Language("kotlin") vararg sources: String,
+    previousCompilationResult: Result? = null,
     block: Result.() -> Unit = { }
   ): Result = compileAnvil(
     sources = sources,
@@ -2425,6 +2592,7 @@ public final class InjectClass_Factory implements Factory<InjectClass> {
     generateDaggerFactories = !useDagger,
     // Many constructor parameters are unused.
     allWarningsAsErrors = false,
+    previousCompilationResult = previousCompilationResult,
     useIR = USE_IR,
     block = block
   )
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/MembersInjectorGeneratorTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/MembersInjectorGeneratorTest.kt
index 83d656a4f..23af827f8 100644
--- a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/MembersInjectorGeneratorTest.kt
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/MembersInjectorGeneratorTest.kt
@@ -12,6 +12,7 @@ import com.squareup.anvil.compiler.internal.testing.getValue
 import com.squareup.anvil.compiler.internal.testing.isStatic
 import com.squareup.anvil.compiler.internal.testing.membersInjector
 import com.squareup.anvil.compiler.nestedInjectClass
+import com.tschuchort.compiletesting.KotlinCompilation.ExitCode.OK
 import com.tschuchort.compiletesting.KotlinCompilation.Result
 import dagger.Lazy
 import dagger.MembersInjector
@@ -1466,6 +1467,166 @@ public final class InjectClass_MembersInjector<T, U, V> implements MembersInject
     }
   }
 
+  @Test fun `a member injector is generated for a class with a super class in another module`() {
+
+    val otherModuleResult = compile(
+      """
+      package com.squareup.test
+
+      import javax.inject.Inject
+
+      abstract class Base {
+        @Inject lateinit var string: String
+      }
+      """
+    ) {
+      assertThat(exitCode).isEqualTo(OK)
+    }
+
+    compile(
+      """
+      package com.squareup.test
+
+      import com.squareup.test.Base
+      import javax.inject.Inject
+
+      class InjectClass : Base() {
+        @Inject lateinit var numbers: List<Int>
+     
+        override fun equals(other: Any?): Boolean {
+          if (this === other) return true
+          if (javaClass != other?.javaClass) return false
+     
+          other as InjectClass
+     
+          if (numbers != other.numbers) return false
+          if (string != other.string) return false
+     
+          return true
+        }
+      }
+      """,
+      previousCompilationResult = otherModuleResult
+    ) {
+      val baseMembersInjector = classLoader.loadClass("com.squareup.test.Base")
+        .membersInjector()
+
+      val injectClassMembersInjector = injectClass.membersInjector()
+
+      val constructor = injectClassMembersInjector.declaredConstructors.single()
+      assertThat(constructor.parameterTypes.toList())
+        .containsExactly(
+          Provider::class.java,
+          Provider::class.java
+        )
+
+      val membersInjectorInstance = constructor
+        .newInstance(
+          Provider { "a" },
+          Provider { listOf(1, 2) }
+        ) as MembersInjector<Any>
+
+      val injectInstanceConstructor = injectClass.createInstance()
+      membersInjectorInstance.injectMembers(injectInstanceConstructor)
+
+      val injectInstanceStatic = injectClass.createInstance()
+
+      injectClassMembersInjector.staticInjectMethod("numbers")
+        .invoke(null, injectInstanceStatic, listOf(1, 2))
+      baseMembersInjector.staticInjectMethod("string")
+        .invoke(null, injectInstanceStatic, "a")
+
+      assertThat(injectInstanceConstructor).isEqualTo(injectInstanceStatic)
+      assertThat(injectInstanceConstructor).isNotSameInstanceAs(injectInstanceStatic)
+    }
+  }
+
+  @Test
+  fun `a member injector is generated for a class with a grand-super class in another module`() {
+
+    val otherModuleResult = compile(
+      """
+      package com.squareup.test
+
+      import javax.inject.Inject
+
+      abstract class Base {
+        @Inject lateinit var string: String
+      }
+      """
+    ) {
+      assertThat(exitCode).isEqualTo(OK)
+    }
+
+    compile(
+      """
+      package com.squareup.test
+
+      import com.squareup.test.Base
+      import javax.inject.Inject
+
+      abstract class Mid : Base() {
+        @Inject lateinit var strings: List<String>
+      }
+
+      class InjectClass : Mid() {
+        @Inject lateinit var numbers: List<Int>
+     
+        override fun equals(other: Any?): Boolean {
+          if (this === other) return true
+          if (javaClass != other?.javaClass) return false
+     
+          other as InjectClass
+     
+          if (numbers != other.numbers) return false
+          if (strings != other.strings) return false
+          if (string != other.string) return false
+     
+          return true
+        }
+      }
+      """,
+      previousCompilationResult = otherModuleResult
+    ) {
+      val baseMembersInjector = classLoader.loadClass("com.squareup.test.Base")
+        .membersInjector()
+      val midMembersInjector = classLoader.loadClass("com.squareup.test.Mid")
+        .membersInjector()
+
+      val injectClassMembersInjector = injectClass.membersInjector()
+
+      val constructor = injectClassMembersInjector.declaredConstructors.single()
+      assertThat(constructor.parameterTypes.toList())
+        .containsExactly(
+          Provider::class.java,
+          Provider::class.java,
+          Provider::class.java
+        )
+
+      val membersInjectorInstance = constructor
+        .newInstance(
+          Provider { "a" },
+          Provider { listOf("b") },
+          Provider { listOf(1, 2) }
+        ) as MembersInjector<Any>
+
+      val injectInstanceConstructor = injectClass.createInstance()
+      membersInjectorInstance.injectMembers(injectInstanceConstructor)
+
+      val injectInstanceStatic = injectClass.createInstance()
+
+      injectClassMembersInjector.staticInjectMethod("numbers")
+        .invoke(null, injectInstanceStatic, listOf(1, 2))
+      midMembersInjector.staticInjectMethod("strings")
+        .invoke(null, injectInstanceStatic, listOf("b"))
+      baseMembersInjector.staticInjectMethod("string")
+        .invoke(null, injectInstanceStatic, "a")
+
+      assertThat(injectInstanceConstructor).isEqualTo(injectInstanceStatic)
+      assertThat(injectInstanceConstructor).isNotSameInstanceAs(injectInstanceStatic)
+    }
+  }
+
   private fun Class<*>.staticInjectMethod(memberName: String): Method {
     // We can't check the @InjectedFieldSignature annotation unfortunately, because it has class
     // retention.
@@ -1476,6 +1637,7 @@ public final class InjectClass_MembersInjector<T, U, V> implements MembersInject
 
   private fun compile(
     @Language("kotlin") vararg sources: String,
+    previousCompilationResult: Result? = null,
     block: Result.() -> Unit = { }
   ): Result = compileAnvil(
     sources = sources,
@@ -1483,6 +1645,7 @@ public final class InjectClass_MembersInjector<T, U, V> implements MembersInject
     generateDaggerFactories = !useDagger,
     useIR = USE_IR,
     allWarningsAsErrors = WARNINGS_AS_ERRORS,
+    previousCompilationResult = previousCompilationResult,
     block = block
   )
 }