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

include inherited functions in Subcomponent Factory checks #1038

Merged
merged 3 commits into from
Jul 20, 2024
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
### Changed

### Deprecated
- `ClassReference.functions` has been deprecated in favor of `ClassReference.memberFunctions` and `ClassReference.declaredMemberFunctions`
- `ClassReference.properties` has been deprecated in favor of `ClassReference.memberProperties` and `ClassReference.declaredMemberProperties`

### Removed

Expand Down
8 changes: 8 additions & 0 deletions compiler-utils/api/compiler-utils.api
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,13 @@ public abstract class com/squareup/anvil/compiler/internal/reference/ClassRefere
public abstract fun getClassId ()Lorg/jetbrains/kotlin/name/ClassId;
public abstract fun getConstructors ()Ljava/util/List;
public abstract fun getContainingFileAsJavaFile ()Ljava/io/File;
public abstract fun getDeclaredMemberFunctions ()Ljava/util/List;
public abstract fun getDeclaredMemberProperties ()Ljava/util/List;
public abstract fun getFqName ()Lorg/jetbrains/kotlin/name/FqName;
public abstract fun getFunctions ()Ljava/util/List;
protected abstract fun getInnerClassesAndObjects ()Ljava/util/List;
public final fun getMemberFunctions ()Ljava/util/List;
public final fun getMemberProperties ()Ljava/util/List;
public abstract fun getModule ()Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;
public final fun getPackageFqName ()Lorg/jetbrains/kotlin/name/FqName;
public abstract fun getProperties ()Ljava/util/List;
Expand Down Expand Up @@ -223,6 +227,8 @@ public final class com/squareup/anvil/compiler/internal/reference/ClassReference
public final fun getClazz ()Lorg/jetbrains/kotlin/descriptors/ClassDescriptor;
public fun getConstructors ()Ljava/util/List;
public fun getContainingFileAsJavaFile ()Ljava/io/File;
public fun getDeclaredMemberFunctions ()Ljava/util/List;
public fun getDeclaredMemberProperties ()Ljava/util/List;
public fun getFqName ()Lorg/jetbrains/kotlin/name/FqName;
public fun getFunctions ()Ljava/util/List;
public fun getModule ()Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;
Expand All @@ -248,6 +254,8 @@ public final class com/squareup/anvil/compiler/internal/reference/ClassReference
public final fun getClazz ()Lorg/jetbrains/kotlin/psi/KtClassOrObject;
public fun getConstructors ()Ljava/util/List;
public fun getContainingFileAsJavaFile ()Ljava/io/File;
public fun getDeclaredMemberFunctions ()Ljava/util/List;
public fun getDeclaredMemberProperties ()Ljava/util/List;
public fun getFqName ()Lorg/jetbrains/kotlin/name/FqName;
public fun getFunctions ()Ljava/util/List;
public fun getModule ()Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public sealed class AnnotationArgumentReference {
}
is KtObjectDeclaration -> {
toClassReference(module)
.properties
.declaredMemberProperties
.mapNotNull { it.property as? KtProperty }
.findConstPropertyWithName(name)
?.initializer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,49 @@ public sealed class ClassReference : Comparable<ClassReference>, AnnotatedRefere
public val packageFqName: FqName get() = classId.packageFqName

public abstract val constructors: List<MemberFunctionReference>

@Deprecated(
"renamed to `declaredMemberFunctions`. " +
"Use `memberFunctions` to include inherited functions.",
replaceWith = ReplaceWith("declaredMemberFunctions"),
)
public abstract val functions: List<MemberFunctionReference>

/**
* All functions that are declared in this class, including overrides.
* This list does not include inherited functions that are not overridden by this class.
*/
public abstract val declaredMemberFunctions: List<MemberFunctionReference>

/**
* All functions declared in this class or any of its super-types.
*/
public val memberFunctions: List<MemberFunctionReference> by lazy(NONE) {
declaredMemberFunctions + directSuperTypeReferences()
.flatMap { it.asClassReference().memberFunctions }
}

@Deprecated(
"renamed to `declaredMemberProperties`. \n" +
"Use `memberProperties` to include inherited properties.",
replaceWith = ReplaceWith("declaredMemberProperties"),
)
public abstract val properties: List<MemberPropertyReference>

/**
* All properties that are declared in this class, including overrides.
* This list does not include inherited properties that are not overridden by this class.
*/
public abstract val declaredMemberProperties: List<MemberPropertyReference>

/**
* All properties declared in this class or any of its super-types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, the new api and comments are very clear

*/
public val memberProperties: List<MemberPropertyReference> by lazy(NONE) {
declaredMemberProperties + directSuperTypeReferences()
.flatMap { it.asClassReference().memberProperties }
}

public abstract val typeParameters: List<TypeParameterReference>

protected abstract val innerClassesAndObjects: List<ClassReference>
Expand Down Expand Up @@ -146,7 +187,13 @@ public sealed class ClassReference : Comparable<ClassReference>, AnnotatedRefere
clazz.containingFileAsJavaFile()
}

override val functions: List<MemberFunctionReference.Psi> by lazy(NONE) {
@Deprecated(
"renamed to `declaredMemberFunctions`. Use `memberFunctions` to include inherited functions.",
replaceWith = ReplaceWith("declaredMemberFunctions"),
)
override val functions: List<MemberFunctionReference.Psi> get() = declaredMemberFunctions

override val declaredMemberFunctions: List<MemberFunctionReference.Psi> by lazy(NONE) {
clazz
.children
.filterIsInstance<KtClassBody>()
Expand All @@ -158,7 +205,13 @@ public sealed class ClassReference : Comparable<ClassReference>, AnnotatedRefere
clazz.annotationEntries.map { it.toAnnotationReference(this, module) }
}

override val properties: List<MemberPropertyReference.Psi> by lazy(NONE) {
@Deprecated(
"renamed to `declaredMemberProperties`. \nUse `memberProperties` to include inherited properties.",
replaceWith = ReplaceWith("declaredMemberProperties"),
)
override val properties: List<MemberPropertyReference.Psi> get() = declaredMemberProperties

override val declaredMemberProperties: List<MemberPropertyReference.Psi> by lazy(NONE) {
buildList {
// Order kind of matters here, since the Descriptor APIs will list body/member properties
// before the constructor properties.
Expand Down Expand Up @@ -263,9 +316,14 @@ public sealed class ClassReference : Comparable<ClassReference>, AnnotatedRefere
)
}

override val functions: List<MemberFunctionReference.Descriptor> by lazy(NONE) {
@Deprecated(
"renamed to `declaredMemberFunctions`. Use `memberFunctions` to include inherited functions.",
replaceWith = ReplaceWith("declaredMemberFunctions"),
)
override val functions: List<MemberFunctionReference.Descriptor> get() = declaredMemberFunctions
override val declaredMemberFunctions: List<MemberFunctionReference.Descriptor> by lazy(NONE) {
clazz.unsubstitutedMemberScope
.getContributedDescriptors(kindFilter = DescriptorKindFilter.FUNCTIONS)
.getDescriptorsFiltered(kindFilter = DescriptorKindFilter.FUNCTIONS)
.filterIsInstance<FunctionDescriptor>()
.filterNot { it is ConstructorDescriptor }
.map { it.toFunctionReference(this) }
Expand All @@ -275,14 +333,19 @@ public sealed class ClassReference : Comparable<ClassReference>, AnnotatedRefere
clazz.annotations.map { it.toAnnotationReference(this, module) }
}

override val properties: List<MemberPropertyReference.Descriptor> by lazy(NONE) {
@Deprecated(
"renamed to `declaredMemberProperties`. \nUse `memberProperties` to include inherited properties.",
replaceWith = ReplaceWith("declaredMemberProperties"),
)
override val properties: List<MemberPropertyReference.Descriptor>
get() = declaredMemberProperties

override val declaredMemberProperties: List<MemberPropertyReference.Descriptor> by lazy(NONE) {
clazz.unsubstitutedMemberScope
.getDescriptorsFiltered(kindFilter = DescriptorKindFilter.VARIABLES)
.filterIsInstance<PropertyDescriptor>()
.filter {
// Remove inherited properties that aren't overridden in this class.
it.kind == DECLARATION
}
// Remove inherited properties that aren't overridden in this class.
.filter { it.kind == DECLARATION }
.map { it.toPropertyReference(this) }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ public fun PropertyDescriptor.toPropertyReference(
internal fun MemberPropertyReference.toDescriptorOrNull(): Descriptor? {
return when (this) {
is Descriptor -> this
is Psi -> declaringClass.toDescriptorReferenceOrNull()?.properties?.find { it.name == name }
is Psi -> declaringClass.toDescriptorReferenceOrNull()
?.declaredMemberProperties
?.find { it.name == name }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ internal object ContributesSubcomponentCodeGen : AnvilApplicabilityChecker {
)
}

val functions = componentInterface.functions
val functions = componentInterface.memberFunctions
.filter { it.returnType().asClassReference() == this }

if (functions.size >= 2) {
Expand Down Expand Up @@ -377,7 +377,7 @@ internal object ContributesSubcomponentCodeGen : AnvilApplicabilityChecker {
)
}

val functions = factory.functions
val functions = factory.memberFunctions
.let { functions ->
if (factory.isInterface()) {
functions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ internal class ContributesSubcomponentHandlerGenerator(
)
}

val functions = componentInterface.functions
val functions = componentInterface.memberFunctions
.filter { it.isAbstract() && it.visibility() == PUBLIC }
.filter {
val returnType = it.returnType().asClassReference()
Expand Down Expand Up @@ -337,7 +337,7 @@ internal class ContributesSubcomponentHandlerGenerator(
)
}

val createComponentFunctions = factory.functions
val createComponentFunctions = factory.memberFunctions
.filter { it.isAbstract() }
.filter { it.returnType().asClassReference().fqName == contributionFqName }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ internal object AssistedFactoryCodeGen : AnvilApplicabilityChecker {
val assistedFunctions = allSuperTypeClassReferences(includeSelf = true)
.distinctBy { it.fqName }
.flatMap { clazz ->
clazz.functions
clazz.declaredMemberFunctions
.filter {
it.isAbstract() &&
(it.visibility() == Visibility.PUBLIC || it.visibility() == Visibility.PROTECTED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ internal object BindsMethodValidator : AnvilApplicabilityChecker {
.forEach { clazz ->
(clazz.companionObjects() + clazz)
.asSequence()
.flatMap { it.functions }
.flatMap { it.declaredMemberFunctions }
.filter { it.isAnnotatedWith(daggerBindsFqName) }
.also { functions ->
assertNoDuplicateFunctions(clazz, functions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ private fun ClassReference.declaredMemberInjectParameters(
superParameters: List<Parameter>,
implementingClass: ClassReference,
): List<MemberInjectParameter> {
return properties
return declaredMemberProperties
.filter { it.isAnnotatedWith(injectFqName) }
.filter { it.visibility() != PRIVATE }
.fold(listOf()) { acc, property ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ internal object MapKeyCreatorCodeGen : AnvilApplicabilityChecker {
if (clazz.isAnnotationClass()) {
val added = creatorsToGenerate.add(clazz)
if (added) {
for (property in clazz.properties) {
for (property in clazz.declaredMemberProperties) {
val type = property.type().asClassReferenceOrNull()
if (type?.isAnnotationClass() == true) {
visitAnnotations(type)
Expand All @@ -254,7 +254,7 @@ internal object MapKeyCreatorCodeGen : AnvilApplicabilityChecker {
}
.toSortedMap()
.map { (className, clazz) ->
val properties = clazz.properties
val properties = clazz.declaredMemberProperties
.map { AnnotationProperty(it) }
.associateBy { it.name }
generateCreatorFunction(className, properties)
Expand All @@ -276,7 +276,7 @@ internal object MapKeyCreatorCodeGen : AnvilApplicabilityChecker {
className: ClassName,
annotationClass: ClassReference,
): FunSpec {
val properties = annotationClass.properties
val properties = annotationClass.declaredMemberProperties
.map { AnnotationProperty(it) }
.associateBy { it.name }
return generateCreatorFunction(className, properties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ internal object MembersInjectorCodeGen : AnvilApplicabilityChecker {
// 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
// (clazz.memberInjectParameters() will do this).
clazz.properties
clazz.declaredMemberProperties
.filter { it.visibility() != Visibility.PRIVATE }
.any { it.isAnnotatedWith(injectFqName) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ internal object ProvidesMethodFactoryCodeGen : AnvilApplicabilityChecker {
.asSequence()

val functions = types
.flatMap { it.functions }
.flatMap { it.declaredMemberFunctions }
.filter { it.isAnnotatedWith(daggerProvidesFqName) }
.onEach { function ->
checkFunctionIsNotAbstract(clazz, function)
Expand All @@ -261,7 +261,7 @@ internal object ProvidesMethodFactoryCodeGen : AnvilApplicabilityChecker {
}

val properties = types
.flatMap { it.properties }
.flatMap { it.declaredMemberProperties }
.filter { property ->
// Must be `@get:Provides`.
property.getterAnnotations.any { it.fqName == daggerProvidesFqName }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public class RealAnvilModuleDescriptor private constructor(
}

return classAndCompanions.firstNotNullOfOrNull { clazz ->
clazz.properties.firstOrNull { it.name == shortName }
clazz.declaredMemberProperties.firstOrNull { it.name == shortName }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,44 @@ class ContributesSubcomponentGeneratorTest(
}
}

@Test fun `a factory function may be defined in a super interface`() {
compile(
"""
package com.squareup.test

import com.squareup.anvil.annotations.ContributesSubcomponent
import com.squareup.anvil.annotations.ContributesSubcomponent.Factory
import com.squareup.anvil.annotations.ContributesTo
import com.squareup.anvil.annotations.MergeComponent

@ContributesSubcomponent(Any::class, parentScope = Unit::class)
interface SubcomponentInterface {
@ContributesTo(Unit::class)
interface AnyParentComponent {
fun createFactory(): ComponentFactory
}

interface Creator {
fun createComponent(): SubcomponentInterface
}

@Factory
interface ComponentFactory : Creator
}

@MergeComponent(Unit::class)
interface ComponentInterface
""",
mode = mode,
) {
assertThat(subcomponentInterface.hintSubcomponent?.java).isEqualTo(subcomponentInterface)
assertThat(subcomponentInterface.hintSubcomponentParentScope).isEqualTo(Unit::class)

assertThat(subcomponentInterface.componentFactoryInterface.methods.map { it.name })
.containsExactly("createComponent")
}
}

@Test
fun `using Dagger's @Subcomponent_Factory is an error`() {
compile(
Expand Down Expand Up @@ -616,6 +654,9 @@ class ContributesSubcomponentGeneratorTest(
private val Class<*>.parentComponentInterface: Class<*>
get() = classLoader.loadClass("$canonicalName\$AnyParentComponent")

private val Class<*>.componentFactoryInterface: Class<*>
get() = classLoader.loadClass("$canonicalName\$ComponentFactory")

private val JvmCompilationResult.subcomponentInterface1: Class<*>
get() = classLoader.loadClass("com.squareup.test.SubcomponentInterface1")

Expand Down
Loading
Loading