Fix KSBackingFieldImpl.modifiers and KSDeclaration utilities (isLocal, isOpen, getVisibility)
diff --git a/api/src/main/kotlin/com/google/devtools/ksp/utils.kt b/api/src/main/kotlin/com/google/devtools/ksp/utils.kt index cb1836d..66d0ce8 100644 --- a/api/src/main/kotlin/com/google/devtools/ksp/utils.kt +++ b/api/src/main/kotlin/com/google/devtools/ksp/utils.kt
@@ -137,7 +137,9 @@ /** Check whether this is a local declaration, or namely, declared in a function. */ fun KSDeclaration.isLocal(): Boolean { - return this.parentDeclaration != null && this.parentDeclaration !is KSClassDeclaration + return this !is KSBackingField && + this.parentDeclaration != null && + this.parentDeclaration !is KSClassDeclaration } /** @@ -197,30 +199,25 @@ } ?: Visibility.PUBLIC } - this.isKotlinBackingField() -> Visibility.PRIVATE this.isLocal() -> Visibility.LOCAL this.modifiers.contains(Modifier.PRIVATE) -> Visibility.PRIVATE this.modifiers.contains(Modifier.PROTECTED) || this.modifiers.contains(Modifier.OVERRIDE) -> Visibility.PROTECTED this.modifiers.contains(Modifier.INTERNAL) -> Visibility.INTERNAL - // for synthetic origin from Java source, synthetic members follow visibility from parent to - // avoid - // package private synthetic members being mishandled as public. - this.origin == Origin.SYNTHETIC && this.parentDeclaration?.origin == Origin.JAVA -> - this.parentDeclaration!!.getVisibility() - - else -> - if (this.origin != Origin.JAVA && this.origin != Origin.JAVA_LIB) Visibility.PUBLIC + else -> { + val origin = + if (this.origin == Origin.SYNTHETIC) { + this.closestClassDeclaration()?.origin ?: this.origin + } else { + this.origin + } + if (origin != Origin.JAVA && origin != Origin.JAVA_LIB) Visibility.PUBLIC else Visibility.JAVA_PACKAGE + } } } -internal fun KSDeclaration.isKotlinBackingField(): Boolean = - this is KSBackingField && ( - origin == Origin.KOTLIN || origin == Origin.KOTLIN_LIB - ) - /** * get all super types for a class declaration Calling [getAllSuperTypes] requires type resolution * therefore is expensive and should be avoided if possible. @@ -281,7 +278,8 @@ } fun KSDeclaration.isOpen() = - !this.isLocal() && + this !is KSBackingField && + !this.isLocal() && !this.modifiers.contains(Modifier.FINAL) && ((this as? KSClassDeclaration)?.classKind == ClassKind.INTERFACE || this.modifiers.contains(Modifier.OVERRIDE) || @@ -328,13 +326,18 @@ return parents } - fun KSDeclaration.isVisibleInPrivate(other: KSDeclaration) = - (other.isLocal() && other.parentDeclarationsForLocal().contains(this.parentDeclaration)) || - this.parentDeclaration == other.parentDeclaration || - this.parentDeclaration == other || - (this.parentDeclaration == null && - other.parentDeclaration == null && - this.containingFile == other.containingFile) + fun KSDeclaration.isVisibleInPrivate(other: KSDeclaration): Boolean { + val receiver = if (this is KSBackingField) this.property else this + val target = if (other is KSBackingField) other.property else other + return if (receiver.parentDeclaration == null && target.parentDeclaration == null) + receiver.containingFile == target.containingFile + else + (target.isLocal() && + target.parentDeclarationsForLocal().contains(receiver.parentDeclaration)) || + receiver.parentDeclaration == target.parentDeclaration || + receiver.parentDeclaration == target || + receiver == target + } return when { // locals are limited to lexical scope
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 5edf178..5aba2b8 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
@@ -304,7 +304,7 @@ } private fun finalModifierIfApplicableTo(declaration: KSDeclaration): Modifier? = when (declaration.origin) { - Origin.KOTLIN if !declaration.isOpen() -> Modifier.FINAL + Origin.KOTLIN if declaration !is KSBackingField && !declaration.isOpen() -> Modifier.FINAL else -> null }
diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/AbstractKSDeclarationImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/AbstractKSDeclarationImpl.kt index 453da2a..5080cb8 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/AbstractKSDeclarationImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/AbstractKSDeclarationImpl.kt
@@ -36,7 +36,6 @@ import com.intellij.psi.PsiModifierListOwner import com.intellij.psi.util.parentOfType import org.jetbrains.kotlin.analysis.api.KaExperimentalApi -import org.jetbrains.kotlin.analysis.api.symbols.KaBackingFieldSymbol import org.jetbrains.kotlin.analysis.api.symbols.KaCallableSymbol import org.jetbrains.kotlin.analysis.api.symbols.KaClassLikeSymbol import org.jetbrains.kotlin.analysis.api.symbols.KaClassSymbol @@ -73,7 +72,6 @@ if (origin == Origin.JAVA_LIB || origin == Origin.KOTLIN_LIB || origin == Origin.SYNTHETIC) { when (val ktDeclarationSymbol = this.ktDeclarationSymbol) { is KaPropertySymbol -> ktDeclarationSymbol.toModifiers() - is KaBackingFieldSymbol -> ktDeclarationSymbol.toModifiers() is KaClassSymbol -> ktDeclarationSymbol.toModifiers() is KaFunctionSymbol -> ktDeclarationSymbol.toModifiers() is KaJavaFieldSymbol -> ktDeclarationSymbol.toModifiers()
diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSBackingFieldImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSBackingFieldImpl.kt index 0026474..4546e54 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSBackingFieldImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSBackingFieldImpl.kt
@@ -77,6 +77,20 @@ KSPropertyDeclarationImpl.getCached(kaBackingFieldSymbol.owningProperty) } + /** + * The modifiers of this backing field. + * + * N.B.: Unlike [AbstractKSDeclarationImpl.modifiers], these are always derived from the symbol instead of from + * the PSI for sources. A backing field is usually implicit and therefore has no PSI to read modifiers from, which + * would make them differ depending on whether the property was read from source or from a compiled artifact. + * + * [Modifier.FINAL] is included only when the backing field is immutable (`val`), matching [KSBackingFieldJavaImpl] + * and the underlying JVM field's `ACC_FINAL` flag. + */ + override val modifiers: Set<Modifier> by lazy { + kaBackingFieldSymbol.toModifiers() + } + override val qualifiedName: KSName? by lazy { // N.B.: kaBackingFieldSymbol.callableId is always null so we use the fqn of the property instead kaBackingFieldSymbol.owningProperty.callableId?.asSingleFqName()?.asString()?.let { propName -> @@ -113,5 +127,5 @@ } } -internal fun KaBackingFieldSymbol.toModifiers(): Set<Modifier> = - mutableSetOf(Modifier.PRIVATE, Modifier.FINAL) +private fun KaBackingFieldSymbol.toModifiers(): Set<Modifier> = + if (isVal) setOf(Modifier.PRIVATE, Modifier.FINAL) else setOf(Modifier.PRIVATE)
diff --git a/kotlin-analysis-api/testData/declarationUtil.kt b/kotlin-analysis-api/testData/declarationUtil.kt index 83f47c4..34090b6 100644 --- a/kotlin-analysis-api/testData/declarationUtil.kt +++ b/kotlin-analysis-api/testData/declarationUtil.kt
@@ -18,14 +18,14 @@ // TEST PROCESSOR: DeclarationUtilProcessor // EXPECTED: // Cls: internal -// EXPECT NEXT: Cls.b.field: local +// EXPECT NEXT: Cls.b.field: private // Cls.b: public open -// EXPECT NEXT: Cls.prop.field: local +// EXPECT NEXT: Cls.prop.field: private // Cls.prop: public open -// EXPECT NEXT: Cls.protectedProp.field: local +// EXPECT NEXT: Cls.protectedProp.field: private // Cls.protectedProp: protected open // Cls.abstractITFFun: public open -// EXPECT NEXT: Cls.pri.field: local +// EXPECT NEXT: Cls.pri.field: private // Cls.pri: private // Cls / <init>: public // Cls / <init>: public @@ -39,17 +39,17 @@ // ITF.nonAbstractITFFun / aa: local // NestedClassSubjects: public open // NestedClassSubjects.NestedDataClass: public -// EXPECT NEXT: NestedClassSubjects.NestedDataClass.field.field: local +// EXPECT NEXT: NestedClassSubjects.NestedDataClass.field.field: private // NestedClassSubjects.NestedDataClass.field: public // NestedClassSubjects.NestedDataClass.component1: public // NestedClassSubjects.NestedDataClass.copy: public // NestedClassSubjects.NestedDataClass / <init>: public // NestedClassSubjects.NestedFinalClass: public -// EXPECT NEXT: NestedClassSubjects.NestedFinalClass.field.field: local +// EXPECT NEXT: NestedClassSubjects.NestedFinalClass.field.field: private // NestedClassSubjects.NestedFinalClass.field: public // NestedClassSubjects.NestedFinalClass / <init>: public // NestedClassSubjects.NestedOpenClass: public open -// EXPECT NEXT: NestedClassSubjects.NestedOpenClass.field.field: local +// EXPECT NEXT: NestedClassSubjects.NestedOpenClass.field.field: private // NestedClassSubjects.NestedOpenClass.field: public // NestedClassSubjects.NestedOpenClass / <init>: public // NestedClassSubjects.NestedInterface: public open
diff --git a/kotlin-analysis-api/testData/javaModifiers.kt b/kotlin-analysis-api/testData/javaModifiers.kt index bfc79e6..1953a39 100644 --- a/kotlin-analysis-api/testData/javaModifiers.kt +++ b/kotlin-analysis-api/testData/javaModifiers.kt
@@ -72,10 +72,12 @@ // DependencyOuterKotlinClass.DependencyInnerKotlinClass: FINAL INNER PUBLIC : FINAL PUBLIC // DependencyOuterKotlinClass.DependencyNestedKotlinClass.<init>: FINAL PUBLIC : FINAL PUBLIC // DependencyOuterKotlinClass.DependencyNestedKotlinClass: OPEN PUBLIC : PUBLIC +// 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: FINAL PRIVATE : FINAL PRIVATE +// EXPECT NEXT: DependencyOuterKotlinClass.volatileProperty.field: PRIVATE : PRIVATE // DependencyOuterKotlinClass.volatileProperty: FINAL PUBLIC : FINAL JAVA_VOLATILE PUBLIC // DependencyOuterKotlinClass: OPEN PUBLIC : PUBLIC // HasTypeAliasFuns: Modifiers: [] @@ -103,16 +105,16 @@ // OuterJavaClass: PUBLIC : PUBLIC // OuterKotlinClass.<init>: FINAL PUBLIC : FINAL PUBLIC // OuterKotlinClass.Companion.<init>: FINAL PUBLIC : FINAL PUBLIC -// EXPECT NEXT: OuterKotlinClass.Companion.companionField.field: : FINAL PRIVATE +// EXPECT NEXT: OuterKotlinClass.Companion.companionField.field: FINAL PRIVATE : FINAL PRIVATE // OuterKotlinClass.Companion.companionField: CONST : FINAL PUBLIC // OuterKotlinClass.Companion.companionMethod: : FINAL PUBLIC -// EXPECT NEXT: OuterKotlinClass.Companion.customJvmStaticCompanionField.field: : FINAL PRIVATE +// EXPECT NEXT: OuterKotlinClass.Companion.customJvmStaticCompanionField.field: FINAL PRIVATE : FINAL PRIVATE // OuterKotlinClass.Companion.customJvmStaticCompanionField: : FINAL JAVA_STATIC PUBLIC // OuterKotlinClass.Companion.customJvmStaticCompanionMethod: : FINAL PUBLIC -// EXPECT NEXT: OuterKotlinClass.Companion.jvmStaticCompanionField.field: : FINAL PRIVATE +// EXPECT NEXT: OuterKotlinClass.Companion.jvmStaticCompanionField.field: FINAL PRIVATE : FINAL PRIVATE // OuterKotlinClass.Companion.jvmStaticCompanionField: : FINAL JAVA_STATIC PUBLIC // OuterKotlinClass.Companion.jvmStaticCompanionMethod: : FINAL JAVA_STATIC PUBLIC -// EXPECT NEXT: OuterKotlinClass.Companion.privateCompanionField.field: : FINAL PRIVATE +// EXPECT NEXT: OuterKotlinClass.Companion.privateCompanionField.field: FINAL PRIVATE : FINAL PRIVATE // OuterKotlinClass.Companion.privateCompanionField: PRIVATE : FINAL PRIVATE // OuterKotlinClass.Companion.privateCompanionMethod: PRIVATE : FINAL PRIVATE // OuterKotlinClass.Companion: : FINAL PUBLIC @@ -120,12 +122,14 @@ // OuterKotlinClass.InnerKotlinClass: INNER : FINAL PUBLIC // OuterKotlinClass.NestedKotlinClass.<init>: FINAL PUBLIC : FINAL PUBLIC // OuterKotlinClass.NestedKotlinClass: OPEN : PUBLIC +// EXPECT NEXT: OuterKotlinClass.mutableProperty.field: PRIVATE : PRIVATE +// OuterKotlinClass.mutableProperty: : FINAL PUBLIC // OuterKotlinClass.synchronizedFun: : FINAL JAVA_SYNCHRONIZED PUBLIC // EXPECT CURRENT: OuterKotlinClass.transientProperty: : FINAL JAVA_TRANSIENT PUBLIC // EXPECT CURRENT: OuterKotlinClass.volatileProperty: : FINAL JAVA_VOLATILE PUBLIC -// EXPECT NEXT: OuterKotlinClass.transientProperty.field: : FINAL JAVA_TRANSIENT PRIVATE +// EXPECT NEXT: OuterKotlinClass.transientProperty.field: FINAL PRIVATE : FINAL JAVA_TRANSIENT PRIVATE // EXPECT NEXT: OuterKotlinClass.transientProperty: : FINAL PUBLIC -// EXPECT NEXT: OuterKotlinClass.volatileProperty.field: : FINAL JAVA_VOLATILE PRIVATE +// EXPECT NEXT: OuterKotlinClass.volatileProperty.field: PRIVATE : JAVA_VOLATILE PRIVATE // EXPECT NEXT: OuterKotlinClass.volatileProperty: : FINAL PUBLIC // OuterKotlinClass: OPEN : PUBLIC // TypeAliasInKt: Modifiers: [PRIVATE] @@ -177,6 +181,8 @@ val customJvmStaticCompanionField:String = "" } + var mutableProperty: String = "" + @Transient val transientProperty: String = "" @@ -277,6 +283,8 @@ val customJvmStaticCompanionField:String = "" } + var mutableProperty: String = "" + @Transient val transientProperty: String = ""
diff --git a/kotlin-analysis-api/testData/visibilities.kt b/kotlin-analysis-api/testData/visibilities.kt index 124025c..ef0db1e 100644 --- a/kotlin-analysis-api/testData/visibilities.kt +++ b/kotlin-analysis-api/testData/visibilities.kt
@@ -43,6 +43,8 @@ // KtEnumWithVal: valueOf: PUBLIC // JavaAnnotation: value: PUBLIC // IntersectionC: property: PUBLIC +// EXPECT NEXT: JavaClass.javaPackageField.field: JAVA_PACKAGE,visible in A, B, D: true, false, true +// EXPECT NEXT: KotlinSubClass.y.field: PRIVATE,visible in A, B, D, KotlinSubClass: false, false, false, true // END // MODULE: lib
diff --git a/test-utils/src/main/kotlin/com/google/devtools/ksp/processor/VisibilityProcessor.kt b/test-utils/src/main/kotlin/com/google/devtools/ksp/processor/VisibilityProcessor.kt index 3d9ced2..fd73de8 100644 --- a/test-utils/src/main/kotlin/com/google/devtools/ksp/processor/VisibilityProcessor.kt +++ b/test-utils/src/main/kotlin/com/google/devtools/ksp/processor/VisibilityProcessor.kt
@@ -86,6 +86,15 @@ intersectionClass.getAllProperties().map { "${intersectionClass.simpleName.asString()}: ${it.simpleName.asString()}: ${it.getVisibility() }" }.forEach { results.add(it) } + javaClass.getAllProperties().mapNotNull { it.backingField }.map { + "${it.qualifiedName?.asString()}: ${it.getVisibility()},visible in A, B, D: " + + "${it.isVisibleFrom(symbolA)}, ${it.isVisibleFrom(symbolB)}, ${it.isVisibleFrom(symbolD)}" + }.forEach { results.add(it) } + kotlinSubClass.declarations.filterIsInstance<KSPropertyDeclaration>().mapNotNull { it.backingField }.map { + "${it.qualifiedName?.asString()}: ${it.getVisibility()},visible in A, B, D, KotlinSubClass: " + + "${it.isVisibleFrom(symbolA)}, ${it.isVisibleFrom(symbolB)}, " + + "${it.isVisibleFrom(symbolD)}, ${it.isVisibleFrom(kotlinSubClass)}" + }.forEach { results.add(it) } return emptyList() } }