Skip to content

Commit

Permalink
Only use shallow checks for implicit bound types in bindings (#27)
Browse files Browse the repository at this point in the history
Previously we were checking all supertypes, which was wrong
  • Loading branch information
ZacSweers authored Jul 20, 2024
1 parent 1f3cbbd commit 088e90e
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ internal class KspContributionMerger(

if (excludedClasses.isNotEmpty()) {
val intersect: Set<ClassName> = mergeAnnotatedClass
.superTypesExcludingAny(resolver)
.superTypesExcludingAny(resolver, shallow = false)
.mapNotNull { it.resolveKSClassDeclaration()?.toClassName() }
.toSet()
// Need to intersect with both merged and origin types to be sure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ internal object BindsMethodValidator : AnvilApplicabilityChecker {

private fun KSTypeReference.resolveSuperTypesExcludingAny(): Sequence<KSType> {
val clazz = resolve().resolveKSClassDeclaration() ?: return emptySequence()
return clazz.superTypesExcludingAny(resolver)
return clazz.superTypesExcludingAny(resolver, shallow = false)
}

private fun KSFunctionDeclaration.singleParameterOrNull(): KSTypeReference? =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ internal fun KSAnnotation.resolveBoundType(
val declaredBoundType = boundTypeOrNull()?.resolveKSClassDeclaration()
if (declaredBoundType != null) return declaredBoundType
// Resolve from the first and only supertype
return declaringClass.superTypesExcludingAny(resolver)
return declaringClass.superTypesExcludingAny(resolver, shallow = true)
.single()
.resolveKSClassDeclaration() ?: throw KspAnvilException(
message = "Couldn't resolve bound type for ${declaringClass.qualifiedName}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ internal fun KSClassDeclaration.checkSingleSuperType(
?.boundTypeOrNull() != null
if (hasExplicitBoundType) return

if (superTypesExcludingAny(resolver).count() != 1) {
if (superTypesExcludingAny(resolver, shallow = true).count() != 1) {
throw KspAnvilException(
message = "${qualifiedName?.asString()} contributes a binding, but does not " +
"specify the bound type. This is only allowed with exactly one direct super type. " +
Expand All @@ -83,7 +83,7 @@ internal fun KSClassDeclaration.checkClassExtendsBoundType(
val boundType = getKSAnnotationsByQualifiedName(annotationFqName.asString())
.firstOrNull()
?.boundTypeOrNull()
?: superTypesExcludingAny(resolver).singleOrNull()
?: superTypesExcludingAny(resolver, shallow = true).singleOrNull()
?: throw KspAnvilException(
message = "Couldn't find the bound type.",
node = this,
Expand All @@ -105,8 +105,16 @@ internal fun KSClassDeclaration.checkClassExtendsBoundType(

internal fun KSClassDeclaration.superTypesExcludingAny(
resolver: Resolver,
): Sequence<KSType> = getAllSuperTypes()
.filterNot { it == resolver.builtIns.anyType }
shallow: Boolean,
): Sequence<KSType> {
val supertypeSequence = if (shallow) {
superTypes.map { it.resolve() }
} else {
getAllSuperTypes()
}
return supertypeSequence
.filterNot { it == resolver.builtIns.anyType }
}

internal fun KSClassDeclaration.isInterface(): Boolean {
return classKind == ClassKind.INTERFACE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,24 @@ class ContributesBindingGeneratorTest : AnvilCompilationModeTest(
}
}

@TestFactory fun `multiple interfaces in hierarchy for bound types is ok`() =
testFactory {
compile(
"""
package com.squareup.test
import com.squareup.anvil.annotations.ContributesBinding
interface ParentParentInterface
interface ParentInterface : ParentParentInterface
@ContributesBinding(Any::class)
interface ContributingInterface : ParentInterface
""",
expectExitCode = OK,
)
}

@TestFactory fun `the bound type can only be implied with one super type - 2 interfaces`() =
testFactory {
compile(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,25 @@ class ContributesMultibindingGeneratorTest : AnvilCompilationModeTest(
}
}

@TestFactory fun `multiple interfaces in hierarchy for bound types is ok`() =
testFactory {
compile(
"""
package com.squareup.test
import com.squareup.anvil.annotations.ContributesMultibinding
interface ParentParentInterface
interface ParentInterface : ParentParentInterface
@ContributesMultibinding(Any::class)
interface ContributingInterface : ParentInterface
""",
mode = mode,
expectExitCode = ExitCode.OK,
)
}

@TestFactory fun `the bound type can only be implied with one super type (2 interfaces)`() =
testFactory {
compile(
Expand Down

0 comments on commit 088e90e

Please sign in to comment.