Merge pull request #887 from square/revert-833-bugfix/binding-superty…

…pe-narrower-than-return-type-wrongly-allowed Revert "[bugfix] Binding supertype which is narrower than return type is wrongly allowed"
square · Feb 26, 2024 · cd79a81 · cd79a81
2 parents 676d426 + 5a04d60
commit cd79a81
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 137 deletions.
diff --git a/...ogic/conventions/src/main/kotlin/com/squareup/anvil/conventions/KtlintConventionPlugin.kt b/...ogic/conventions/src/main/kotlin/com/squareup/anvil/conventions/KtlintConventionPlugin.kt
@@ -13,18 +13,6 @@ open class KtlintConventionPlugin : Plugin<Project> {
 
     target.extensions.configure(KtlintExtension::class.java) { ktlint ->
       ktlint.version.set(target.libs.versions.ktlint)
-      val pathsToExclude = listOf(
-        "build/generated",
-        "root-build/generated",
-        "included-build/generated",
-      )
-      ktlint.filter {
-        it.exclude {
-          pathsToExclude.any { excludePath ->
-            it.file.path.contains(excludePath)
-          }
-        }
-      }
       ktlint.verbose.set(true)
     }
   }

diff --git a/compiler-utils/api/compiler-utils.api b/compiler-utils/api/compiler-utils.api
@@ -627,8 +627,6 @@ public final class com/squareup/anvil/compiler/internal/reference/TypeReference$
 public final class com/squareup/anvil/compiler/internal/reference/TypeReferenceKt {
 	public static final fun AnvilCompilationExceptionTypReference (Lcom/squareup/anvil/compiler/internal/reference/TypeReference;Ljava/lang/String;Ljava/lang/Throwable;)Lcom/squareup/anvil/compiler/api/AnvilCompilationException;
 	public static synthetic fun AnvilCompilationExceptionTypReference$default (Lcom/squareup/anvil/compiler/internal/reference/TypeReference;Ljava/lang/String;Ljava/lang/Throwable;ILjava/lang/Object;)Lcom/squareup/anvil/compiler/api/AnvilCompilationException;
-	public static final fun allSuperTypeReferences (Lcom/squareup/anvil/compiler/internal/reference/TypeReference;Z)Lkotlin/sequences/Sequence;
-	public static synthetic fun allSuperTypeReferences$default (Lcom/squareup/anvil/compiler/internal/reference/TypeReference;ZILjava/lang/Object;)Lkotlin/sequences/Sequence;
 	public static final fun toTypeReference (Lorg/jetbrains/kotlin/psi/KtTypeReference;Lcom/squareup/anvil/compiler/internal/reference/ClassReference$Psi;Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;)Lcom/squareup/anvil/compiler/internal/reference/TypeReference$Psi;
 	public static final fun toTypeReference (Lorg/jetbrains/kotlin/types/KotlinType;Lcom/squareup/anvil/compiler/internal/reference/ClassReference;Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;)Lcom/squareup/anvil/compiler/internal/reference/TypeReference$Descriptor;
 }

diff --git a/...iler-utils/src/main/java/com/squareup/anvil/compiler/internal/reference/ClassReference.kt b/...iler-utils/src/main/java/com/squareup/anvil/compiler/internal/reference/ClassReference.kt
@@ -451,7 +451,6 @@ public fun AnvilCompilationExceptionClassReference(
     message = message,
     cause = cause,
   )
-
   is Descriptor -> AnvilCompilationException(
     classDescriptor = classReference.clazz,
     message = message,

diff --git a/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/reference/TypeReference.kt b/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/reference/TypeReference.kt
@@ -666,30 +666,3 @@ private fun TypeName.lambdaFix(): TypeName {
     .parameterizedBy(allTypes)
     .copy(nullable = isNullable)
 }
-
-/**
- * This will return all super types as [TypeReference], whether they're parsed as [KtTypeReference]
- * or [KotlinType]. This will include generated code, assuming it has already been generated.
- * The returned sequence will be distinct by FqName, and Psi types are preferred over Descriptors.
- *
- * The first elements in the returned sequence represent the direct superclass to the receiver. The
- * last elements represent the types which are furthest up-stream.
- *
- * @param includeSelf If true, the receiver class is the first element of the sequence
- */
-@ExperimentalAnvilApi
-public fun TypeReference.allSuperTypeReferences(
-  includeSelf: Boolean = false,
-): Sequence<TypeReference> {
-  return generateSequence(listOf(this)) { superTypes ->
-    superTypes
-      .mapNotNull { it.asClassReferenceOrNull() }
-      .flatMap { classRef ->
-        classRef.directSuperTypeReferences()
-      }
-      .takeIf { it.isNotEmpty() }
-  }
-    .drop(if (includeSelf) 0 else 1)
-    .flatten()
-    .distinct()
-}
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
@@ -7,9 +7,9 @@ import com.squareup.anvil.compiler.codegen.CheckOnlyCodeGenerator
 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.ClassReference
 import com.squareup.anvil.compiler.internal.reference.MemberFunctionReference
-import com.squareup.anvil.compiler.internal.reference.TypeReference
-import com.squareup.anvil.compiler.internal.reference.allSuperTypeReferences
+import com.squareup.anvil.compiler.internal.reference.allSuperTypeClassReferences
 import com.squareup.anvil.compiler.internal.reference.classAndInnerClassReferences
 import com.squareup.anvil.compiler.internal.reference.toTypeReference
 import org.jetbrains.kotlin.descriptors.ModuleDescriptor
@@ -29,29 +29,6 @@ internal class BindsMethodValidator : CheckOnlyCodeGenerator() {
 
   override fun isApplicable(context: AnvilContext) = context.generateFactories
 
-  internal object Errors {
-    internal const val BINDS_MUST_BE_ABSTRACT = "@Binds methods must be abstract"
-    internal const val BINDS_MUST_HAVE_SINGLE_PARAMETER =
-      "@Binds methods must have exactly one parameter, " +
-        "whose type is assignable to the return type"
-    internal const val BINDS_MUST_RETURN_A_VALUE = "@Binds methods must return a value (not void)"
-    internal fun bindsParameterMustBeAssignable(
-      paramSuperTypeNames: List<String>,
-      returnTypeName: String,
-      parameterType: String?,
-    ): String {
-      val superTypesMessage = if (paramSuperTypeNames.isEmpty()) {
-        "has no supertypes."
-      } else {
-        "only has the following supertypes: $paramSuperTypeNames"
-      }
-
-      return "@Binds methods' parameter type must be assignable to the return type. " +
-        "Expected binding of type $returnTypeName but impl parameter of type " +
-        "$parameterType $superTypesMessage"
-    }
-  }
-
   override fun checkCode(
     codeGenDir: File,
     module: ModuleDescriptor,
@@ -77,7 +54,7 @@ internal class BindsMethodValidator : CheckOnlyCodeGenerator() {
   private fun validateBindsFunction(function: MemberFunctionReference.Psi) {
     if (!function.isAbstract()) {
       throw AnvilCompilationExceptionFunctionReference(
-        message = Errors.BINDS_MUST_BE_ABSTRACT,
+        message = "@Binds methods must be abstract",
         functionReference = function,
       )
     }
@@ -89,47 +66,64 @@ internal class BindsMethodValidator : CheckOnlyCodeGenerator() {
       )
     }
 
-    val bindingParameter = listOfNotNull(
-      function.singleParameterTypeOrNull(),
-      function.receiverTypeOrNull(),
-    ).singleOrNull()
-
-    bindingParameter ?: throw AnvilCompilationExceptionFunctionReference(
-      message = Errors.BINDS_MUST_HAVE_SINGLE_PARAMETER,
-      functionReference = function,
-    )
-
-    val returnTypeName = function.returnTypeOrNull()?.asTypeName()
+    val hasSingleBindingParameter =
+      function.parameters.size == 1 && !function.function.isExtensionDeclaration()
+    if (!hasSingleBindingParameter) {
+      throw AnvilCompilationExceptionFunctionReference(
+        message = "@Binds methods must have exactly one parameter, " +
+          "whose type is assignable to the return type",
+        functionReference = function,
+      )
+    }
 
-    returnTypeName ?: throw AnvilCompilationExceptionFunctionReference(
-      message = Errors.BINDS_MUST_RETURN_A_VALUE,
+    function.returnTypeOrNull() ?: throw AnvilCompilationExceptionFunctionReference(
+      message = "@Binds methods must return a value (not void)",
       functionReference = function,
     )
 
-    val parameterSuperTypes =
-      bindingParameter
-        .allSuperTypeReferences(includeSelf = true)
-        .map { it.asTypeName() }
+    if (!function.parameterMatchesReturnType() && !function.receiverMatchesReturnType()) {
+      val returnType = function.returnType().asClassReference().shortName
+      val paramSuperTypes = (function.parameterSuperTypes() ?: function.receiverSuperTypes())!!
+        .map { it.shortName }
         .toList()
 
-    if (returnTypeName !in parameterSuperTypes) {
-      val superTypeNames = parameterSuperTypes.map { it.toString() }
+      val superTypesMessage = if (paramSuperTypes.size == 1) {
+        "has no supertypes."
+      } else {
+        "only has the following supertypes: ${paramSuperTypes.drop(1)}"
+      }
       throw AnvilCompilationExceptionFunctionReference(
-        message = Errors.bindsParameterMustBeAssignable(
-          paramSuperTypeNames = superTypeNames.drop(1),
-          returnTypeName = returnTypeName.toString(),
-          parameterType = superTypeNames.first(),
-        ),
+        message = "@Binds methods' parameter type must be assignable to the return type. " +
+          "Expected binding of type $returnType but impl parameter of type " +
+          "${paramSuperTypes.first()} $superTypesMessage",
         functionReference = function,
       )
     }
   }
 
-  private fun MemberFunctionReference.Psi.singleParameterTypeOrNull(): TypeReference? {
-    return parameters.singleOrNull()?.type()
+  private fun MemberFunctionReference.Psi.parameterMatchesReturnType(): Boolean {
+    return parameterSuperTypes()
+      ?.contains(returnType().asClassReference())
+      ?: false
+  }
+
+  private fun MemberFunctionReference.Psi.parameterSuperTypes(): Sequence<ClassReference>? {
+    return parameters.singleOrNull()
+      ?.type()
+      ?.asClassReference()
+      ?.allSuperTypeClassReferences(includeSelf = true)
+  }
+
+  private fun MemberFunctionReference.Psi.receiverMatchesReturnType(): Boolean {
+    return receiverSuperTypes()
+      ?.contains(returnType().asClassReference())
+      ?: false
   }
 
-  private fun MemberFunctionReference.Psi.receiverTypeOrNull(): TypeReference? {
-    return function.receiverTypeReference?.toTypeReference(declaringClass, module)
+  private fun MemberFunctionReference.Psi.receiverSuperTypes(): Sequence<ClassReference>? {
+    return function.receiverTypeReference
+      ?.toTypeReference(declaringClass, module)
+      ?.asClassReference()
+      ?.allSuperTypeClassReferences(includeSelf = true)
   }
 }
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
@@ -55,8 +55,8 @@ class BindsMethodValidatorTest(
       )
       if (!useDagger) {
         assertThat(messages).contains(
-          "Expected binding of type com.squareup.test.Bar but impl parameter of type com.squareup.test.Foo only has the following " +
-            "supertypes: [com.squareup.test.Ipsum, com.squareup.test.Lorem]",
+          "Expected binding of type Bar but impl parameter of type Foo only has the following " +
+            "supertypes: [Ipsum, Lorem]",
         )
       }
     }
@@ -88,7 +88,7 @@ class BindsMethodValidatorTest(
       )
       if (!useDagger) {
         assertThat(messages).contains(
-          "Expected binding of type com.squareup.test.Bar but impl parameter of type com.squareup.test.Foo has no supertypes.",
+          "Expected binding of type Bar but impl parameter of type Foo has no supertypes.",
         )
       }
     }
@@ -314,44 +314,6 @@ class BindsMethodValidatorTest(
     }
   }
 
-  @Test
-  fun `binding which supertype is narrower than return type fails to compile`() {
-    compile(
-      """
-        package com.squareup.test
-
-        import dagger.Module
-        import dagger.Binds
-    
-        sealed interface ItemDetail {
-          object DetailTypeA : ItemDetail
-        }
-
-        interface ItemMapper<T : ItemDetail>
-
-        class DetailTypeAItemMapper : ItemMapper<ItemDetail.DetailTypeA>
-        
-        @Module
-        interface SomeModule {
-          @Binds fun shouldBeInvalidComplexBinding(real: DetailTypeAItemMapper): ItemMapper<ItemDetail>
-        }
-      """.trimIndent(),
-    ) {
-      assertThat(exitCode).isEqualTo(COMPILATION_ERROR)
-      assertThat(messages).contains(
-        "@Binds methods' parameter type must be assignable to the return type",
-      )
-      if (!useDagger) {
-        assertThat(messages).contains(
-          "@Binds methods' parameter type must be assignable to the return type. Expected " +
-            "binding of type com.squareup.test.ItemMapper<com.squareup.test.ItemDetail> but impl " +
-            "parameter of type com.squareup.test.DetailTypeAItemMapper only has the following " +
-            "supertypes: [com.squareup.test.ItemMapper<com.squareup.test.ItemDetail.DetailTypeA>]",
-        )
-      }
-    }
-  }
-
   private fun compile(
     @Language("kotlin") vararg sources: String,
     previousCompilationResult: JvmCompilationResult? = null,