Repository: square/anvil (kotlin)

Issue:
Unnecessary MembersInjectorGenerator parsing/generation when the target class doesn't have its own injected properties
Right now, `MembersInjectGenerator` inspects the full hierarchy for every`KtClassOrObject`.  If it finds any member-injected properties anywhere in that hierarchy, then it generates a `MembersInjector<T>` for the target class.  This is doing more work than it needs to.  

If a project has an abstract `BaseFragment` or something with 100 different implementations, and none of them have any member injection, the current implementation will parse `BaseFragment` 101 times looking for properties.  

If that `BaseFragment` *does* have a member-injected property but none of the subclasses do, the current implementation will generate 100 injectors (1 for each subclass).  Dagger won't know they exist and will never use them.

Instead, the generator can start by just inspecting the target class without supers.  If the target has any *directly* declared injected fields, *then* the full hierarchy needs to be parsed and a new `MembersInjector<T>` should be generated.

Pull request description:
fixes #453

Patch:
diff --git a/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/ClassReference.kt b/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/ClassReference.kt
index ebfbca7bb..a139a2cfe 100644
--- a/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/ClassReference.kt
+++ b/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/ClassReference.kt
@@ -7,6 +7,7 @@ import com.squareup.anvil.compiler.internal.ClassReference.Psi
 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.ClassName
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
+import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.ModuleDescriptor
 import org.jetbrains.kotlin.name.ClassId
 import org.jetbrains.kotlin.name.FqName
@@ -184,3 +185,9 @@ public fun ClassReference.daggerScopes(
 
 @ExperimentalAnvilApi
 public fun ClassReference.asClassName(): ClassName = classId.asClassName()
+
+@ExperimentalAnvilApi
+public fun ClassReference.isInterface(): Boolean = when (this) {
+  is Descriptor -> clazz.kind == ClassKind.INTERFACE
+  is Psi -> clazz.isInterface()
+}
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 11f6bbfec..94aa19c32 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
@@ -14,6 +14,7 @@ import com.squareup.anvil.compiler.internal.asClassName
 import com.squareup.anvil.compiler.internal.capitalize
 import com.squareup.anvil.compiler.internal.findAnnotation
 import com.squareup.anvil.compiler.internal.hasAnnotation
+import com.squareup.anvil.compiler.internal.isInterface
 import com.squareup.anvil.compiler.internal.toClassReference
 import com.squareup.kotlinpoet.FunSpec
 import dagger.internal.ProviderOfLazy
@@ -79,13 +80,17 @@ internal fun FunSpec.Builder.addMemberInjection(
  * Impl -> Middle -> Base
  * The order of dependencies in `Impl_Factory`'s constructor should be:
  * Base -> Middle -> Impl
+ *
+ * @param inheritedOnly If true, only returns the injected properties declared in superclasses.
+ *   If false, the injected properties of the receiver class will be included.
  */
 internal fun KtClassOrObject.memberInjectParameters(
-  module: ModuleDescriptor
+  module: ModuleDescriptor,
+  inheritedOnly: Boolean = false
 ): List<MemberInjectParameter> {
-
   return toClassReference()
-    .allSuperTypeClassReferences(module, includeSelf = true)
+    .allSuperTypeClassReferences(module, includeSelf = !inheritedOnly)
+    .filterNot { it.isInterface() }
     .toList()
     .foldRight(listOf()) { classReference, acc ->
 
diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectorGenerator.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectorGenerator.kt
index eddbf1ca5..b62eb6f62 100644
--- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectorGenerator.kt
+++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/dagger/MembersInjectorGenerator.kt
@@ -7,12 +7,14 @@ import com.squareup.anvil.compiler.api.GeneratedFile
 import com.squareup.anvil.compiler.api.createGeneratedFile
 import com.squareup.anvil.compiler.codegen.MemberInjectParameter
 import com.squareup.anvil.compiler.codegen.PrivateCodeGenerator
+import com.squareup.anvil.compiler.codegen.mapToMemberInjectParameters
 import com.squareup.anvil.compiler.internal.asClassName
 import com.squareup.anvil.compiler.internal.buildFile
 import com.squareup.anvil.compiler.internal.capitalize
 import com.squareup.anvil.compiler.internal.classesAndInnerClass
 import com.squareup.anvil.compiler.internal.generateClassName
 import com.squareup.anvil.compiler.internal.isGenericClass
+import com.squareup.anvil.compiler.internal.isInterface
 import com.squareup.anvil.compiler.internal.safePackageString
 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.ClassName
@@ -45,10 +47,19 @@ internal class MembersInjectorGenerator : PrivateCodeGenerator() {
   ) {
     projectFiles
       .classesAndInnerClass(module)
+      .filterNot { it.isInterface() }
       .forEach { clazz ->
-        val parameters = clazz.memberInjectParameters(module)
+
+        // Only generate a MembersInjector if the target class declares its own member-injected
+        // properties. If it does, then any properties from superclasses must be added as well.
+        val declaredInjectedProperties = clazz.injectedMembers(module)
           .ifEmpty { return@forEach }
 
+        val parameters = clazz.memberInjectParameters(module, inheritedOnly = true)
+          .let { inherited ->
+            inherited + declaredInjectedProperties.mapToMemberInjectParameters(module, inherited)
+          }
+
         generateMembersInjectorClass(
           codeGenDir = codeGenDir,
           clazz = clazz,


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 3b3aa7611..3b39ca9a4 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
@@ -1047,6 +1047,59 @@ public class InjectClass_Factory(
     }
   }
 
+  @Test
+  fun `a factory class performs member injection when the only fields are in a super class`() {
+
+    compile(
+      """
+      package com.squareup.test
+
+      import javax.inject.Inject
+
+      class InjectClass @Inject constructor() : Base() 
+
+      abstract class Base {
+
+        @Inject
+        lateinit var base1: List<Int>
+
+        @Inject
+        lateinit var base2: List<String>
+      }
+      """
+    ) {
+
+      val factoryClass = injectClass.factoryClass()
+
+      val constructor = factoryClass.declaredConstructors.single()
+      assertThat(constructor.parameterTypes.toList())
+        .containsExactly(
+          Provider::class.java,
+          Provider::class.java
+        )
+
+      val base1 = listOf(3)
+      val base2 = listOf("base2")
+
+      val staticMethods = factoryClass.declaredMethods.filter { it.isStatic }
+
+      val factoryInstance = factoryClass.createInstance(
+        Provider { base1 },
+        Provider { base2 }
+      )
+      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("base1")).isEqualTo(base1)
+      assertThat(getInstance.getPropertyValue("base2")).isEqualTo(base2)
+    }
+  }
+
   @Test fun `a factory class performs member injection on a super class from another module`() {
 
     val otherModuleResult = compile(
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 4d1554077..c51751981 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
@@ -26,6 +26,7 @@ import java.lang.reflect.Method
 import javax.inject.Named
 import javax.inject.Provider
 import kotlin.reflect.KClass
+import kotlin.test.assertFailsWith
 
 @Suppress("UNCHECKED_CAST")
 @RunWith(Parameterized::class)
@@ -2175,6 +2176,44 @@ public final class InjectClass_MembersInjector<T, U, V> implements MembersInject
     }
   }
 
+  @Test
+  fun `a member injector is not generated when field injected properties are only in a super class`() {
+
+    compile(
+      """
+      package com.squareup.test
+ 
+      import javax.inject.Inject
+
+      abstract class Base {
+        @Inject lateinit var string: String
+      }
+
+      class InjectClass : Base() {
+     
+        override fun equals(other: Any?): Boolean {
+          if (this === other) return true
+          if (javaClass != other?.javaClass) return false
+     
+          other as InjectClass
+     
+          if (string != other.string) return false
+     
+          return true
+        }
+
+        override fun hashCode(): Int = string.hashCode()
+      }
+      """
+    ) {
+      assertThat(exitCode).isEqualTo(OK)
+
+      assertFailsWith<ClassNotFoundException> {
+        injectClass.membersInjector()
+      }
+    }
+  }
+
   private fun Class<*>.staticInjectMethod(memberName: String): Method {
     // We can't check the @InjectedFieldSignature annotation unfortunately, because it has class
     // retention.