Skip to content

Commit

Permalink
Merge pull request #230 from square/ralf/boundtype-bug
Browse files Browse the repository at this point in the history
If the bound type is specified, then multiple super types are allowed.
  • Loading branch information
vRallev authored Mar 17, 2021
2 parents f499559 + c55bcab commit 251bc2d
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal class ContributesBindingGenerator : CodeGenerator {
.onEach { clazz ->
clazz.checkClassIsPublic()
clazz.checkNotMoreThanOneQualifier(module, contributesBindingFqName)
clazz.checkSingleSuperType(contributesBindingFqName)
clazz.checkSingleSuperType(module, contributesBindingFqName)
clazz.checkClassExtendsBoundType(module, contributesBindingFqName)
}
.map { clazz ->
Expand Down Expand Up @@ -120,8 +120,13 @@ internal fun KtClassOrObject.checkClassIsPublic() {
}

internal fun KtClassOrObject.checkSingleSuperType(
module: ModuleDescriptor,
annotationFqName: FqName
) {
// If the bound type exists, then you're allowed to have multiple super types. Without the bound
// type there must be exactly one super type.
if (boundType(annotationFqName, module) != null) return

val superTypes = superTypeListEntries
if (superTypes.size != 1) {
throw AnvilCompilationException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal class ContributesMultibindingGenerator : CodeGenerator {
clazz.checkClassIsPublic()
clazz.checkNotMoreThanOneQualifier(module, contributesMultibindingFqName)
clazz.checkNotMoreThanOneMapKey(module)
clazz.checkSingleSuperType(contributesMultibindingFqName)
clazz.checkSingleSuperType(module, contributesMultibindingFqName)
clazz.checkClassExtendsBoundType(module, contributesMultibindingFqName)
}
.map { clazz ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,51 @@ internal fun KtClassOrObject.scope(
annotationFqName: FqName,
module: ModuleDescriptor
): FqName {
return findScopeClassLiteralExpression(annotationFqName)
.let {
val children = it.children
return findClassLiteralExpression(annotationFqName, name = "scope", index = 0)
.let { classLiteralExpression ->
if (classLiteralExpression == null) {
throw AnvilCompilationException(
"The first argument for $annotationFqName must be a class literal: $text",
element = this
)
}

val children = classLiteralExpression.children
children.singleOrNull() ?: throw AnvilCompilationException(
"Expected a single child, but had ${children.size} instead: ${it.text}",
"Expected a single child, but there were ${children.size} instead: " +
classLiteralExpression.text,
element = this
)
}
.requireFqName(module)
}

private fun KtClassOrObject.findScopeClassLiteralExpression(
annotationFqName: FqName
): KtClassLiteralExpression {
internal fun KtClassOrObject.boundType(
annotationFqName: FqName,
module: ModuleDescriptor
): FqName? {
return findClassLiteralExpression(annotationFqName, name = "boundType", index = 1)
?.let {
val children = it.children
children.singleOrNull() ?: throw AnvilCompilationException(
"Expected a single child, but there were ${children.size} instead: ${it.text}",
element = this
)
}
?.requireFqName(module)
}

/**
* Finds the class literal expression in the given annotation. [name] refers to the parameter name
* in the annotation and [index] to the position of the argument, e.g. if you look for the scope in
* `@ContributesBinding(Int::class, boundType = Unit::class)`, then [name] would be "scope" and the
* index 0. If you look for the bound type, then [name] would be "boundType" and the index 1.
*/
private fun KtClassOrObject.findClassLiteralExpression(
annotationFqName: FqName,
name: String,
index: Int
): KtClassLiteralExpression? {
val annotationEntry = findAnnotation(annotationFqName)
?: throw AnvilCompilationException(
"Couldn't find $annotationFqName for Psi element: $text",
Expand All @@ -150,7 +181,7 @@ private fun KtClassOrObject.findScopeClassLiteralExpression(
.mapNotNull { valueArgument ->
val children = valueArgument.children
if (children.size == 2 && children[0] is KtValueArgumentName &&
(children[0] as KtValueArgumentName).asName.asString() == "scope" &&
(children[0] as KtValueArgumentName).asName.asString() == name &&
children[1] is KtClassLiteralExpression
) {
children[1] as KtClassLiteralExpression
Expand All @@ -164,14 +195,10 @@ private fun KtClassOrObject.findScopeClassLiteralExpression(
// If there is no named argument, then take the first argument, which must be a class literal
// expression, e.g. @ContributesTo(Unit::class)
return annotationValues
.firstOrNull()
.elementAtOrNull(index)
?.let { valueArgument ->
valueArgument.children.firstOrNull() as? KtClassLiteralExpression
}
?: throw AnvilCompilationException(
"The first argument for $annotationFqName must be a class literal: $text",
element = this
)
}

internal fun PsiElement.fqNameOrNull(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,24 @@ class ContributesBindingGeneratorTest {
}
}

@Test fun `the bound type is not implied when explicitly defined`() {
compile(
"""
package com.squareup.test
import com.squareup.anvil.annotations.ContributesBinding
interface ParentInterface
@ContributesBinding(Int::class, ParentInterface::class)
interface ContributingInterface : ParentInterface, CharSequence
"""
) {
assertThat(contributingInterface.hintBinding?.java).isEqualTo(contributingInterface)
assertThat(contributingInterface.hintBindingScope).isEqualTo(Int::class)
}
}

@Test fun `the contributed binding class must extend the bound type`() {
compile(
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,28 @@ class ContributesMultibindingGeneratorTest {
}
}

@Test fun `the bound type is not implied when explicitly defined`() {
compile(
"""
package com.squareup.test
import com.squareup.anvil.annotations.ContributesMultibinding
interface ParentInterface
@ContributesMultibinding(
scope = Int::class,
ignoreQualifier = true,
boundType = ParentInterface::class
)
interface ContributingInterface : ParentInterface, CharSequence
"""
) {
assertThat(contributingInterface.hintMultibinding?.java).isEqualTo(contributingInterface)
assertThat(contributingInterface.hintMultibindingScope).isEqualTo(Int::class)
}
}

@Test fun `the contributed multibinding class must extend the bound type`() {
compile(
"""
Expand Down

0 comments on commit 251bc2d

Please sign in to comment.