Skip to content

Commit

Permalink
use the resolved value of const arguments in propagated annotation …
Browse files Browse the repository at this point in the history
…arguments

We have a good deal of logic around parsing out the primitive values of `const`
parameters, but that logic is not recursive.
So, if a `const` references another `const` in its initializer, we wound up
just copying the text of that initializer into the generated annotations,
as though it's just a String.
This problem was isolated to the PSI side of parsing,
since the Descriptor APIs are able to parse out the final, primitive value.
Our type resolution logic always defaults to the PSI models.

Now, when we need to parse out the primitive values from a `PropertyReference`,
we first try to resolve the Descriptor version with a `PropertyDescriptor`.
That isn't possible if the annotation is referencing a `const`
property that was generated in the same round of compilation.
In that event, the parsing will fall back to the PSI models.
Anvil doesn't actually generate a `const`
and then use it in an annotation, so this seems like a reasonable compromise.

fixes #938
  • Loading branch information
RBusarow committed Mar 27, 2024
1 parent 7506c36 commit 1d7207e
Show file tree
Hide file tree
Showing 17 changed files with 792 additions and 95 deletions.
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ ij_kotlin_extends_list_wrap = on_every_item
ij_kotlin_field_annotation_wrap = normal
ij_kotlin_finally_on_new_line = false
ij_kotlin_if_rparen_on_new_line = true
ij_kotlin_import_nested_classes = true
ij_kotlin_import_nested_classes = false
ij_kotlin_imports_layout = *,java.**,javax.**,kotlin.**,^
ij_kotlin_insert_whitespaces_in_simple_one_line_method = true
ij_kotlin_keep_blank_lines_before_right_brace = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package com.squareup.anvil.conventions
import com.rickbusarow.kgx.buildDir
import com.rickbusarow.kgx.extras
import com.rickbusarow.kgx.fromInt
import com.rickbusarow.kgx.getValue
import com.rickbusarow.kgx.javaExtension
import com.rickbusarow.kgx.provideDelegate
import com.squareup.anvil.conventions.utils.isInAnvilBuild
import com.squareup.anvil.conventions.utils.isInAnvilIncludedBuild
import com.squareup.anvil.conventions.utils.isInAnvilRootBuild
Expand Down Expand Up @@ -201,6 +203,29 @@ abstract class BasePlugin : Plugin<Project> {

task.maxParallelForks = Runtime.getRuntime().availableProcessors()

task.useJUnitPlatform {
it.includeEngines("junit-jupiter", "junit-vintage")
}

val testImplementation by target.configurations

testImplementation.dependencies.addLater(target.libs.junit.jupiter.engine)
testImplementation.dependencies.addLater(target.libs.junit.vintage.engine)

task.systemProperties.putAll(
mapOf(
// remove parentheses from test display names
"junit.jupiter.displayname.generator.default" to
"org.junit.jupiter.api.DisplayNameGenerator\$Simple",

// Allow unit tests to run in parallel
// https://junit.org/junit5/docs/snapshot/user-guide/#writing-tests-parallel-execution-config-properties
"junit.jupiter.execution.parallel.enabled" to true,
"junit.jupiter.execution.parallel.mode.default" to "concurrent",
"junit.jupiter.execution.parallel.mode.classes.default" to "concurrent",
),
)

task.jvmArgs(
// Fixes illegal reflective operation warnings during tests. It's a Kotlin issue.
// https://github.com/pinterest/ktlint/issues/1618
Expand All @@ -218,7 +243,6 @@ abstract class BasePlugin : Plugin<Project> {
"--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
"--illegal-access=permit",
)

task.testLogging { logging ->
Expand Down
8 changes: 7 additions & 1 deletion compiler-utils/api/compiler-utils.api
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ public final class com/squareup/anvil/compiler/internal/FqNameKt {
public static final fun classIdBestGuess (Lorg/jetbrains/kotlin/name/FqName;)Lorg/jetbrains/kotlin/name/ClassId;
public static final fun descendant (Lorg/jetbrains/kotlin/name/FqName;Ljava/lang/String;)Lorg/jetbrains/kotlin/name/FqName;
public static final fun getFqName (Lkotlin/reflect/KClass;)Lorg/jetbrains/kotlin/name/FqName;
public static final fun parents (Lorg/jetbrains/kotlin/name/FqName;)Lkotlin/sequences/Sequence;
public static final fun parentsWithSelf (Lorg/jetbrains/kotlin/name/FqName;)Lkotlin/sequences/Sequence;
public static final fun safePackageString (Ljava/lang/String;ZZZ)Ljava/lang/String;
public static final fun safePackageString (Lorg/jetbrains/kotlin/name/FqName;ZZ)Ljava/lang/String;
public static synthetic fun safePackageString$default (Ljava/lang/String;ZZZILjava/lang/Object;)Ljava/lang/String;
Expand Down Expand Up @@ -147,9 +149,11 @@ public abstract interface class com/squareup/anvil/compiler/internal/reference/A
public abstract fun getClassReference (Lorg/jetbrains/kotlin/psi/KtClassOrObject;)Lcom/squareup/anvil/compiler/internal/reference/ClassReference$Psi;
public abstract fun getClassReferenceOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lcom/squareup/anvil/compiler/internal/reference/ClassReference;
public abstract fun getTopLevelFunctionReferences (Lorg/jetbrains/kotlin/psi/KtFile;)Ljava/util/List;
public abstract fun getTopLevelPropertyReferenceOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lcom/squareup/anvil/compiler/internal/reference/PropertyReference;
public abstract fun getTopLevelPropertyReferences (Lorg/jetbrains/kotlin/psi/KtFile;)Ljava/util/List;
public abstract fun resolveClassIdOrNull (Lorg/jetbrains/kotlin/name/ClassId;)Lorg/jetbrains/kotlin/name/FqName;
public abstract fun resolveFqNameOrNull (Lorg/jetbrains/kotlin/name/FqName;Lorg/jetbrains/kotlin/incremental/components/LookupLocation;)Lorg/jetbrains/kotlin/descriptors/ClassDescriptor;
public abstract fun resolvePropertyReferenceOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lcom/squareup/anvil/compiler/internal/reference/PropertyReference;
public abstract fun resolveTypeAliasFqNameOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lorg/jetbrains/kotlin/descriptors/TypeAliasDescriptor;
}

Expand Down Expand Up @@ -358,7 +362,7 @@ public final class com/squareup/anvil/compiler/internal/reference/MemberFunction
public abstract class com/squareup/anvil/compiler/internal/reference/MemberPropertyReference : com/squareup/anvil/compiler/internal/reference/AnnotatedReference, com/squareup/anvil/compiler/internal/reference/PropertyReference {
public fun equals (Ljava/lang/Object;)Z
public abstract fun getDeclaringClass ()Lcom/squareup/anvil/compiler/internal/reference/ClassReference;
public final fun getMemberName ()Lcom/squareup/kotlinpoet/MemberName;
public fun getMemberName ()Lcom/squareup/kotlinpoet/MemberName;
public fun getModule ()Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;
protected abstract fun getType ()Lcom/squareup/anvil/compiler/internal/reference/TypeReference;
public fun hashCode ()I
Expand Down Expand Up @@ -449,6 +453,7 @@ public final class com/squareup/anvil/compiler/internal/reference/ParameterRefer
public abstract interface class com/squareup/anvil/compiler/internal/reference/PropertyReference {
public abstract fun getFqName ()Lorg/jetbrains/kotlin/name/FqName;
public abstract fun getGetterAnnotations ()Ljava/util/List;
public abstract fun getMemberName ()Lcom/squareup/kotlinpoet/MemberName;
public abstract fun getModule ()Lcom/squareup/anvil/compiler/internal/reference/AnvilModuleDescriptor;
public abstract fun getName ()Ljava/lang/String;
public abstract fun getSetterAnnotations ()Ljava/util/List;
Expand Down Expand Up @@ -521,6 +526,7 @@ public final class com/squareup/anvil/compiler/internal/reference/TopLevelFuncti

public abstract class com/squareup/anvil/compiler/internal/reference/TopLevelPropertyReference : com/squareup/anvil/compiler/internal/reference/AnnotatedReference, com/squareup/anvil/compiler/internal/reference/PropertyReference {
public fun equals (Ljava/lang/Object;)Z
public fun getMemberName ()Lcom/squareup/kotlinpoet/MemberName;
protected abstract fun getType ()Lcom/squareup/anvil/compiler/internal/reference/TypeReference;
public fun hashCode ()I
public fun isAnnotatedWith (Lorg/jetbrains/kotlin/name/FqName;)Z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import dagger.MapKey
import dagger.Provides
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.name.parentOrNull
import javax.inject.Inject
import javax.inject.Qualifier
import javax.inject.Scope
Expand All @@ -39,6 +40,28 @@ internal val mergeModulesFqName = MergeModules::class.fqName

internal val anyFqName = Any::class.fqName

/**
* Generates a sequence of [FqName] starting from the current FqName and including its parents
* up to the root. The sequence will include the current FqName as well.
*/
@ExperimentalAnvilApi
public fun FqName.parentsWithSelf(): Sequence<FqName> {
return generateSequence(this) { it.parentOrNull() }
.map {
it.toUnsafe()
// The top-most parent is an FqName with the text "<root>",
// whereas the actual FqName.ROOT is an empty string. We want the empty string.
if (parent().isRoot) FqName.ROOT else it
}
}

/**
* Generates a sequence of [FqName] starting from the current FqName and including its parents
* up to the root. The sequence will not include the current FqName.
*/
@ExperimentalAnvilApi
public fun FqName.parents(): Sequence<FqName> = parentsWithSelf().drop(1)

@ExperimentalAnvilApi
public fun FqName.descendant(segments: String): FqName =
if (isRoot) FqName(segments) else FqName("${asString()}.$segments")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,20 +206,18 @@ public sealed class AnnotationArgumentReference {
}

fun resolvePrimitiveConstant(fqName: FqName): Any? {
// This could be a constant from a primitive type, e.g. Int.MAX_VALUE
val classFqName = fqName.parent()
val constantName = fqName.shortName().asString()

// If this constant is coming from a companion object, then we'll find it this way.
classFqName.toClassReferenceOrNull(module)
?.let { if (it.isObject()) listOf(it) else it.companionObjects() }
?.flatMap { it.properties }
?.singleOrNull { it.name == constantName }
module.resolvePropertyReferenceOrNull(fqName)
// Prefer descriptor types for this since the parsing is already done.
// We won't be able to resolve a descriptor
// if the reference was also generated in this round,
// but Anvil itself doesn't generate consts and then use them as annotation arguments.
?.let { it.toDescriptorOrNull() ?: it }
?.let { property ->
return when (property) {
is MemberPropertyReference.Descriptor ->
is PropertyReference.Descriptor ->
property.property.compileTimeInitializer?.value
is MemberPropertyReference.Psi ->
is PropertyReference.Psi ->
// A PropertyReference.property may also be a KtParameter if it's in a constructor,
// but if we're here we're in an object, so the property must be a KtProperty.
(property.property as KtProperty).initializer?.let { parsePrimitiveType(it.text) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ public interface AnvilModuleDescriptor : ModuleDescriptor {

public fun getTopLevelPropertyReferences(ktFile: KtFile): List<TopLevelPropertyReference.Psi>

public fun getTopLevelPropertyReferenceOrNull(fqName: FqName): PropertyReference?

public fun resolvePropertyReferenceOrNull(fqName: FqName): PropertyReference?

public fun getClassReference(clazz: KtClassOrObject): Psi

public fun getClassReference(descriptor: ClassDescriptor): Descriptor
Expand All @@ -52,7 +56,10 @@ internal inline fun ModuleDescriptor.asAnvilModuleDescriptor(): AnvilModuleDescr
@ExperimentalAnvilApi
public fun FqName.canResolveFqName(
module: ModuleDescriptor,
): Boolean = module.asAnvilModuleDescriptor().resolveClassIdOrNull(classIdBestGuess()) != null
): Boolean = module.asAnvilModuleDescriptor().run {
resolveClassIdOrNull(this@canResolveFqName.classIdBestGuess()) != null ||
resolvePropertyReferenceOrNull(this@canResolveFqName) != null
}

@ExperimentalAnvilApi
public fun Collection<KtFile>.classAndInnerClassReferences(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public sealed class MemberPropertyReference : AnnotatedReference, PropertyRefere

public override val module: AnvilModuleDescriptor get() = declaringClass.module

public val memberName: MemberName get() = MemberName(declaringClass.asClassName(), name)
public override val memberName: MemberName get() = MemberName(declaringClass.asClassName(), name)

protected abstract val type: TypeReference?

Expand Down Expand Up @@ -201,3 +201,10 @@ public fun KtProperty.toPropertyReference(
public fun PropertyDescriptor.toPropertyReference(
declaringClass: ClassReference.Descriptor,
): Descriptor = Descriptor(this, declaringClass)

internal fun MemberPropertyReference.toDescriptorOrNull(): Descriptor? {
return when (this) {
is Descriptor -> this
is Psi -> declaringClass.toDescriptorReferenceOrNull()?.properties?.find { it.name == name }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ package com.squareup.anvil.compiler.internal.reference

import com.squareup.anvil.annotations.ExperimentalAnvilApi
import com.squareup.anvil.compiler.api.AnvilCompilationException
import com.squareup.anvil.compiler.internal.reference.PropertyReference.Descriptor
import com.squareup.anvil.compiler.internal.reference.PropertyReference.Psi
import com.squareup.kotlinpoet.MemberName
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtCallableDeclaration
Expand All @@ -15,6 +14,7 @@ public sealed interface PropertyReference {
public val module: AnvilModuleDescriptor

public val name: String
public val memberName: MemberName

public val setterAnnotations: List<AnnotationReference>
public val getterAnnotations: List<AnnotationReference>
Expand All @@ -38,19 +38,30 @@ public sealed interface PropertyReference {
}
}

internal fun ClassReference.Psi.toDescriptorReferenceOrNull(): ClassReference.Descriptor? {
return module.resolveFqNameOrNull(fqName)?.toClassReference(module)
}

internal fun PropertyReference.toDescriptorOrNull(): PropertyReference.Descriptor? {
return when (this) {
is MemberPropertyReference -> toDescriptorOrNull()
is TopLevelPropertyReference -> toDescriptorOrNull()
}
}

@ExperimentalAnvilApi
@Suppress("FunctionName")
public fun AnvilCompilationExceptionPropertyReference(
propertyReference: PropertyReference,
message: String,
cause: Throwable? = null,
): AnvilCompilationException = when (propertyReference) {
is Psi -> AnvilCompilationException(
is PropertyReference.Psi -> AnvilCompilationException(
element = propertyReference.property,
message = message,
cause = cause,
)
is Descriptor -> AnvilCompilationException(
is PropertyReference.Descriptor -> AnvilCompilationException(
propertyDescriptor = propertyReference.property,
message = message,
cause = cause,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ package com.squareup.anvil.compiler.internal.reference

import com.squareup.anvil.annotations.ExperimentalAnvilApi
import com.squareup.anvil.compiler.api.AnvilCompilationException
import com.squareup.anvil.compiler.internal.getContributedPropertyOrNull
import com.squareup.anvil.compiler.internal.reference.TopLevelPropertyReference.Descriptor
import com.squareup.anvil.compiler.internal.reference.TopLevelPropertyReference.Psi
import com.squareup.anvil.compiler.internal.reference.Visibility.INTERNAL
import com.squareup.anvil.compiler.internal.reference.Visibility.PRIVATE
import com.squareup.anvil.compiler.internal.reference.Visibility.PROTECTED
import com.squareup.anvil.compiler.internal.reference.Visibility.PUBLIC
import com.squareup.anvil.compiler.internal.requireFqName
import com.squareup.kotlinpoet.MemberName
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.descriptors.annotations.AnnotationUseSiteTarget.PROPERTY_GETTER
Expand All @@ -29,6 +31,13 @@ public sealed class TopLevelPropertyReference : AnnotatedReference, PropertyRefe

protected abstract val type: TypeReference?

override val memberName: MemberName by lazy(NONE) {
MemberName(
packageName = fqName.parent().asString(),
simpleName = name,
)
}

public override fun typeOrNull(): TypeReference? = type

override fun toString(): String = "$fqName"
Expand Down Expand Up @@ -194,3 +203,11 @@ public fun KtProperty.toTopLevelPropertyReference(
public fun PropertyDescriptor.toTopLevelPropertyReference(
module: AnvilModuleDescriptor,
): Descriptor = Descriptor(property = this, module = module)

internal fun TopLevelPropertyReference.toDescriptorOrNull(): Descriptor? {
return when (this) {
is Descriptor -> this
is Psi -> fqName.getContributedPropertyOrNull(module)
?.toTopLevelPropertyReference(module)
}
}
2 changes: 2 additions & 0 deletions compiler/api/compiler.api
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ public final class com/squareup/anvil/compiler/codegen/reference/RealAnvilModule
public fun getStableName ()Lorg/jetbrains/kotlin/name/Name;
public fun getSubPackagesOf (Lorg/jetbrains/kotlin/name/FqName;Lkotlin/jvm/functions/Function1;)Ljava/util/Collection;
public fun getTopLevelFunctionReferences (Lorg/jetbrains/kotlin/psi/KtFile;)Ljava/util/List;
public fun getTopLevelPropertyReferenceOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lcom/squareup/anvil/compiler/internal/reference/PropertyReference;
public fun getTopLevelPropertyReferences (Lorg/jetbrains/kotlin/psi/KtFile;)Ljava/util/List;
public fun isValid ()Z
public fun resolveClassIdOrNull (Lorg/jetbrains/kotlin/name/ClassId;)Lorg/jetbrains/kotlin/name/FqName;
public fun resolveFqNameOrNull (Lorg/jetbrains/kotlin/name/FqName;Lorg/jetbrains/kotlin/incremental/components/LookupLocation;)Lorg/jetbrains/kotlin/descriptors/ClassDescriptor;
public fun resolvePropertyReferenceOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lcom/squareup/anvil/compiler/internal/reference/PropertyReference;
public fun resolveTypeAliasFqNameOrNull (Lorg/jetbrains/kotlin/name/FqName;)Lorg/jetbrains/kotlin/descriptors/TypeAliasDescriptor;
public fun shouldSeeInternalsOf (Lorg/jetbrains/kotlin/descriptors/ModuleDescriptor;)Z
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,8 @@ dependencies {
testImplementation(libs.kotlin.reflect)
testImplementation(libs.ksp.compilerPlugin)
testImplementation(libs.truth)

testRuntimeOnly(libs.kotest.assertions.core.jvm)
testRuntimeOnly(libs.junit.vintage.engine)
testRuntimeOnly(libs.junit.jupiter.engine)
}
Loading

0 comments on commit 1d7207e

Please sign in to comment.