Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[bugfix] Binding supertype which is narrower than return type is wrongly allowed" #887

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
2 changes: 0 additions & 2 deletions compiler-utils/api/compiler-utils.api
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,6 @@ public fun AnvilCompilationExceptionClassReference(
message = message,
cause = cause,
)

is Descriptor -> AnvilCompilationException(
classDescriptor = classReference.clazz,
message = message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
)
}
Expand All @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
)
}
}
Expand Down Expand Up @@ -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.",
)
}
}
Expand Down Expand Up @@ -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,
Expand Down
Loading