Report JAVA_TRANSIENT and JAVA_VOLATILE on KSBackingField when new features are enabled
diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt index dff4ca9..19f2b86 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt
@@ -169,6 +169,8 @@ set(value) { instance_prop.set(value) } + val instanceOrNull: ResolverAAImpl? + get() = instance_prop.get() private val ktModule_prop: ThreadLocal<KaSourceModule> = ThreadLocal() var ktModule: KaSourceModule @@ -334,24 +336,22 @@ else -> null } - private fun transientModifierIfApplicableTo(declaration: KSDeclaration): Modifier? = when (declaration) { - is KSPropertyDeclaration -> - if (declaration.jvmAccessFlag and Opcodes.ACC_TRANSIENT != 0) - Modifier.JAVA_TRANSIENT - else - null - - else -> null + private fun transientModifierIfApplicableTo(declaration: KSDeclaration): Modifier? { + val property = when (declaration) { + is KSBackingField -> declaration.property + is KSPropertyDeclaration if !shouldEnableNewFeatures() -> declaration + else -> return null + } + return if (property.jvmAccessFlag and Opcodes.ACC_TRANSIENT != 0) Modifier.JAVA_TRANSIENT else null } - private fun volatileModifierIfApplicableTo(declaration: KSDeclaration): Modifier? = when (declaration) { - is KSPropertyDeclaration -> - if (declaration.jvmAccessFlag and Opcodes.ACC_VOLATILE != 0) - Modifier.JAVA_VOLATILE - else - null - - else -> null + private fun volatileModifierIfApplicableTo(declaration: KSDeclaration): Modifier? { + val property = when (declaration) { + is KSBackingField -> declaration.property + is KSPropertyDeclaration if !shouldEnableNewFeatures() -> declaration + else -> return null + } + return if (property.jvmAccessFlag and Opcodes.ACC_VOLATILE != 0) Modifier.JAVA_VOLATILE else null } private fun strictModifierIfApplicableTo(declaration: KSDeclaration): Modifier? = when (declaration) {
diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSPropertyDeclarationJavaImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSPropertyDeclarationJavaImpl.kt index 9b6627d..01904d7 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSPropertyDeclarationJavaImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSPropertyDeclarationJavaImpl.kt
@@ -50,6 +50,20 @@ override val ktDeclarationSymbol: KaDeclarationSymbol get() = ktJavaFieldSymbol + /** + * The modifiers of this property as declared, including the [fieldOnlyModifiers]. + * + * N.B.: This is cached separately from [modifiers] because whether the latter reports the [fieldOnlyModifiers] + * depends on the processor that is currently running. Declarations are cached and shared by all processors of a + * round, so the decision cannot be cached along with them. + */ + protected open val allModifiers: Set<Modifier> by lazy { + super<AbstractKSDeclarationImpl>.modifiers + } + + override val modifiers: Set<Modifier> + get() = allModifiers.withoutFieldOnlyModifiers() + // Manual delegation for KSExpectActual to avoid eager evaluation in the class header private val expectActualImpl by lazy { KSExpectActualImpl(ktJavaFieldSymbol) } override val isActual: Boolean get() = expectActualImpl.isActual @@ -127,7 +141,7 @@ } private class KSPropertyDeclarationJavaAAImpl( - override val ktJavaFieldSymbol: KaJavaFieldSymbol + override val ktJavaFieldSymbol: KaJavaFieldSymbol, ) : KSPropertyDeclarationJavaImpl() { companion object : KSObjectCache<KaJavaFieldSymbol, KSPropertyDeclarationJavaAAImpl>() { fun getCached(symbol: KaJavaFieldSymbol) = cache.getOrPut(symbol) { @@ -179,7 +193,7 @@ override val isMutable: Boolean get() = !psiField.hasModifierProperty(PsiModifier.FINAL) - override val modifiers: Set<Modifier> by lazy { + override val allModifiers: Set<Modifier> by lazy { if (origin == Origin.JAVA) { return@lazy psiField.toKSModifiers() } @@ -217,3 +231,32 @@ } return result } + +/** + * JVM modifiers that apply to a field rather than to the property that owns it. + * + * Kotlin models both of them as annotations (`kotlin.jvm.Transient` and `kotlin.jvm.Volatile`) with a field use-site + * target, and Java declares them on the field itself. Neither of them can be applied to anything but a field. + */ +internal val fieldOnlyModifiers = setOf(Modifier.JAVA_TRANSIENT, Modifier.JAVA_VOLATILE) + +/** + * Removes the [fieldOnlyModifiers] if backing fields are enabled, in which case they are reported on the + * [KSBackingField] of a property instead of on the [KSPropertyDeclaration] itself. + * + * This keeps [KSPropertyDeclaration.modifiers], and therefore also `Resolver.effectiveJavaModifiers`, consistent + * across all origins: for [Origin.KOTLIN] the corresponding annotations are already moved to the backing field, and + * for [Origin.KOTLIN_LIB] and [Origin.JAVA_LIB] the access flags are only resolved for backing fields. + * + * Declarations are also read outside a processing round, e.g. while KSP computes the dirty files for incremental + * processing. There is no processor to opt in to backing fields in that case, so nothing is removed. + */ +internal fun Set<Modifier>.withoutFieldOnlyModifiers(): Set<Modifier> { + // Checked first to avoid both the thread local lookup and the allocation below for the common case. + val setsDoNotOverlap = fieldOnlyModifiers.none { modifier -> modifier in this } + if (setsDoNotOverlap) { + return this + } + + return if (ResolverAAImpl.instanceOrNull?.shouldEnableNewFeatures() == true) this - fieldOnlyModifiers else this +}
diff --git a/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPUnitTestSuite.kt b/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPUnitTestSuite.kt index cdddd8b..487c759 100644 --- a/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPUnitTestSuite.kt +++ b/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPUnitTestSuite.kt
@@ -529,7 +529,7 @@ @Bug("https://github.com/google/ksp/issues/3123", BugState.OPEN) @Bug("https://github.com/google/ksp/issues/3125", BugState.FIXED) - @Bug("https://github.com/google/ksp/issues/3155", BugState.OPEN) + @Bug("https://github.com/google/ksp/issues/3155", BugState.FIXED) @Bug("https://github.com/google/ksp/issues/3185", BugState.FIXED) @TestMetadata("javaModifiers.kt") @Test
diff --git a/kotlin-analysis-api/testData/javaModifiers.kt b/kotlin-analysis-api/testData/javaModifiers.kt index f1245d6..1bddd32 100644 --- a/kotlin-analysis-api/testData/javaModifiers.kt +++ b/kotlin-analysis-api/testData/javaModifiers.kt
@@ -22,11 +22,13 @@ // C.InnerC: PUBLIC : PUBLIC // C.NestedC.<init>: FINAL PUBLIC : FINAL PUBLIC // C.NestedC: JAVA_STATIC PUBLIC : JAVA_STATIC PUBLIC +// EXPECT CURRENT: C.i1: JAVA_STATIC JAVA_VOLATILE PROTECTED : JAVA_STATIC JAVA_VOLATILE PROTECTED // EXPECT NEXT: C.i1.field: JAVA_STATIC JAVA_VOLATILE PROTECTED : JAVA_STATIC JAVA_VOLATILE PROTECTED -// C.i1: JAVA_STATIC JAVA_VOLATILE PROTECTED : JAVA_STATIC JAVA_VOLATILE PROTECTED +// EXPECT NEXT: C.i1: JAVA_STATIC PROTECTED : JAVA_STATIC PROTECTED // C.intFun: JAVA_DEFAULT JAVA_SYNCHRONIZED : JAVA_DEFAULT JAVA_SYNCHRONIZED +// EXPECT CURRENT: C.s1: FINAL JAVA_TRANSIENT : FINAL JAVA_TRANSIENT // EXPECT NEXT: C.s1.field: FINAL JAVA_TRANSIENT : FINAL JAVA_TRANSIENT -// C.s1: FINAL JAVA_TRANSIENT : FINAL JAVA_TRANSIENT +// EXPECT NEXT: C.s1: FINAL : FINAL // EXPECT NEXT: C.staticStr.field: PRIVATE : PRIVATE // C.staticStr: PRIVATE : PRIVATE // C: ABSTRACT PUBLIC : ABSTRACT PUBLIC @@ -48,10 +50,12 @@ // DependencyOuterJavaClass.staticPublicField: FINAL JAVA_STATIC PUBLIC : FINAL JAVA_STATIC PUBLIC // DependencyOuterJavaClass.staticPublicMethod: FINAL JAVA_STATIC PUBLIC : FINAL JAVA_STATIC PUBLIC // DependencyOuterJavaClass.synchronizedFun: OPEN : JAVA_SYNCHRONIZED +// EXPECT CURRENT: DependencyOuterJavaClass.transientField: FINAL : FINAL JAVA_TRANSIENT // EXPECT NEXT: DependencyOuterJavaClass.transientField.field: JAVA_TRANSIENT : JAVA_TRANSIENT -// DependencyOuterJavaClass.transientField: FINAL : FINAL JAVA_TRANSIENT +// EXPECT NEXT: DependencyOuterJavaClass.transientField: FINAL : FINAL +// EXPECT CURRENT: DependencyOuterJavaClass.volatileField: FINAL : JAVA_VOLATILE // EXPECT NEXT: DependencyOuterJavaClass.volatileField.field: JAVA_VOLATILE : JAVA_VOLATILE -// DependencyOuterJavaClass.volatileField: FINAL : JAVA_VOLATILE +// EXPECT NEXT: DependencyOuterJavaClass.volatileField: FINAL : FINAL // DependencyOuterJavaClass: OPEN PUBLIC : PUBLIC // DependencyOuterKotlinClass.<init>: FINAL PUBLIC : FINAL PUBLIC // DependencyOuterKotlinClass.Companion.<init>: FINAL PRIVATE : FINAL PRIVATE @@ -75,10 +79,12 @@ // EXPECT NEXT: DependencyOuterKotlinClass.mutableProperty.field: PRIVATE : PRIVATE // DependencyOuterKotlinClass.mutableProperty: FINAL PUBLIC : FINAL PUBLIC // DependencyOuterKotlinClass.synchronizedFun: FINAL PUBLIC : FINAL JAVA_SYNCHRONIZED PUBLIC -// EXPECT NEXT: DependencyOuterKotlinClass.transientProperty.field: FINAL PRIVATE : FINAL PRIVATE -// DependencyOuterKotlinClass.transientProperty: FINAL PUBLIC : FINAL JAVA_TRANSIENT PUBLIC -// EXPECT NEXT: DependencyOuterKotlinClass.volatileProperty.field: PRIVATE : PRIVATE -// DependencyOuterKotlinClass.volatileProperty: FINAL PUBLIC : JAVA_VOLATILE PUBLIC +// EXPECT CURRENT: DependencyOuterKotlinClass.transientProperty: FINAL PUBLIC : FINAL JAVA_TRANSIENT PUBLIC +// EXPECT NEXT: DependencyOuterKotlinClass.transientProperty.field: FINAL PRIVATE : FINAL JAVA_TRANSIENT PRIVATE +// EXPECT NEXT: DependencyOuterKotlinClass.transientProperty: FINAL PUBLIC : FINAL PUBLIC +// EXPECT CURRENT: DependencyOuterKotlinClass.volatileProperty: FINAL PUBLIC : JAVA_VOLATILE PUBLIC +// EXPECT NEXT: DependencyOuterKotlinClass.volatileProperty.field: PRIVATE : JAVA_VOLATILE PRIVATE +// EXPECT NEXT: DependencyOuterKotlinClass.volatileProperty: FINAL PUBLIC : FINAL PUBLIC // DependencyOuterKotlinClass: OPEN PUBLIC : PUBLIC // HasTypeAliasFuns: Modifiers: [] // HasTypeAliasFuns: Visibility: PUBLIC @@ -103,10 +109,12 @@ // OuterJavaClass.staticPublicField: JAVA_STATIC PUBLIC : JAVA_STATIC PUBLIC // OuterJavaClass.staticPublicMethod: JAVA_STATIC PUBLIC : JAVA_STATIC PUBLIC // OuterJavaClass.synchronizedFun: JAVA_SYNCHRONIZED : JAVA_SYNCHRONIZED +// EXPECT CURRENT: OuterJavaClass.transientField: JAVA_TRANSIENT : JAVA_TRANSIENT // EXPECT NEXT: OuterJavaClass.transientField.field: JAVA_TRANSIENT : JAVA_TRANSIENT -// OuterJavaClass.transientField: JAVA_TRANSIENT : JAVA_TRANSIENT +// EXPECT NEXT: OuterJavaClass.transientField: : +// EXPECT CURRENT: OuterJavaClass.volatileField: JAVA_VOLATILE : JAVA_VOLATILE // EXPECT NEXT: OuterJavaClass.volatileField.field: JAVA_VOLATILE : JAVA_VOLATILE -// OuterJavaClass.volatileField: JAVA_VOLATILE : JAVA_VOLATILE +// EXPECT NEXT: OuterJavaClass.volatileField: : // OuterJavaClass: PUBLIC : PUBLIC // OuterKotlinClass.<init>: FINAL PUBLIC : FINAL PUBLIC // OuterKotlinClass.Companion.<init>: FINAL PUBLIC : FINAL PUBLIC @@ -142,10 +150,12 @@ // TypeAliasInLib: Modifiers: [FINAL, PUBLIC] // TypeAliasInLib: Visibility: PUBLIC // topLevelSynchronizedFun: FINAL PUBLIC : FINAL JAVA_SYNCHRONIZED PUBLIC -// EXPECT NEXT: topLevelTransientProperty.field: FINAL PRIVATE : FINAL PRIVATE -// topLevelTransientProperty: FINAL PUBLIC : FINAL JAVA_TRANSIENT PUBLIC -// EXPECT NEXT: topLevelVolatileProperty.field: PRIVATE : PRIVATE -// topLevelVolatileProperty: FINAL PUBLIC : JAVA_VOLATILE PUBLIC +// EXPECT CURRENT: topLevelTransientProperty: FINAL PUBLIC : FINAL JAVA_TRANSIENT PUBLIC +// EXPECT NEXT: topLevelTransientProperty.field: FINAL PRIVATE : FINAL JAVA_TRANSIENT PRIVATE +// EXPECT NEXT: topLevelTransientProperty: FINAL PUBLIC : FINAL PUBLIC +// EXPECT CURRENT: topLevelVolatileProperty: FINAL PUBLIC : JAVA_VOLATILE PUBLIC +// EXPECT NEXT: topLevelVolatileProperty.field: PRIVATE : JAVA_VOLATILE PRIVATE +// EXPECT NEXT: topLevelVolatileProperty: FINAL PUBLIC : FINAL PUBLIC // END // MODULE: module1 // FILE: ALib.kt