Skip to content

Commit

Permalink
Handle final fields in Java
Browse files Browse the repository at this point in the history
  • Loading branch information
IgnatBeresnev committed Jun 17, 2022
1 parent be2ee8b commit dba2829
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 21 deletions.
Expand Up @@ -469,19 +469,9 @@ private class DokkaDescriptorVisitor(

return coroutineScope {
val generics = async { descriptor.typeParameters.parallelMap { it.toVariantTypeParameter() } }

val getter = getDescriptorGetter() ?: getImplicitAccessorGetter()
val setter = getDescriptorSetter() ?: getImplicitAccessorSetter()

val isVar = if (descriptor is JavaPropertyDescriptor) {
// java properties are `var` by default from descriptor's point of view,
// as there's no binding of fields to getters/setter.
// java property should be var if it has no accessors at all or has a setter
(getter == null && setter == null) || (getter != null && setter != null)
} else {
descriptor.isVar
}

DProperty(
dri = dri,
name = descriptor.name.asString(),
Expand All @@ -507,13 +497,26 @@ private class DokkaDescriptorVisitor(
.toAnnotations(),
descriptor.getDefaultValue()?.let { DefaultValue(it.toSourceSetDependent()) },
inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) },
takeIf { isVar }?.let { IsVar },
takeIf { descriptor.isVar(getter, setter) }?.let { IsVar },
)
)
)
}
}

private fun PropertyDescriptor.isVar(getter: DFunction?, setter: DFunction?): Boolean {
return if (this is JavaPropertyDescriptor) {
// in Java, concepts of extensibility and mutability are mixed into a single `final` modifier
// in Kotlin, it's different - val/var controls mutability and open modifier controls extensibility
// so when inheriting Java properties, you can end up with a final var - non extensible mutable prop
val isMutable = this.isVar
// non-final java property should be var if it has no accessors at all or has a setter
(isMutable && getter == null && setter == null) || (getter != null && setter != null)
} else {
this.isVar
}
}

private fun PropertyDescriptor.getVisibility(implicitAccessors: DescriptorAccessorHolder?): Visibility {
val isNonPublicJavaProperty = this is JavaPropertyDescriptor && !this.visibility.isPublicAPI
val visibility =
Expand Down
Expand Up @@ -621,9 +621,10 @@ class DefaultPsiToDocumentableTranslator(
private fun parseField(psi: PsiField, getter: DFunction?, setter: DFunction?, inheritedFrom: DRI? = null): DProperty {
val dri = DRI.from(psi)

// java property without accessors should be var
// setter should be non null when inheriting kotlin vars
val isVar = (getter == null && setter == null) || (getter != null && setter != null)
// non-final java field without accessors should be a var
// setter should be not null when inheriting kotlin vars
val isMutable = !psi.hasModifierProperty("final")
val isVar = (isMutable && getter == null && setter == null) || (getter != null && setter != null)

return DProperty(
dri = dri,
Expand Down
Expand Up @@ -331,4 +331,29 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {
}
}
}

@Test
fun `should mark final property inherited from java as val`() {
testInline(
"""
|/src/test/A.java
|package test;
|public class A {
| public final int a = 1;
|}
|
|/src/test/B.kt
|package test
|class B : A {}
""".trimIndent(),
commonTestConfiguration
) {
documentablesMergingStage = { module ->
val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
val property = kotlinProperties.single { it.name == "a" }

assertNull(property.extra[IsVar])
}
}
}
}
23 changes: 17 additions & 6 deletions plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt
Expand Up @@ -58,6 +58,7 @@ class PsiSuperFieldsTest : BaseAbstractTest() {
|package test
|open class A {
| var a: Int = 1
| val b: Int = 2
|}
|
|/src/test/B.java
Expand All @@ -68,15 +69,25 @@ class PsiSuperFieldsTest : BaseAbstractTest() {
) {
documentablesMergingStage = { module ->
val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
val property = inheritorProperties.single { it.name == "a" }
inheritorProperties.single { it.name == "a" }.let { mutableProperty ->
assertNotNull(mutableProperty.getter)
assertNotNull(mutableProperty.setter)

assertNotNull(property.getter)
assertNotNull(property.setter)
val inheritedFrom = mutableProperty.extra[InheritedMember]?.inheritedFrom?.values?.single()
assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)

val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
assertNotNull(mutableProperty.extra[IsVar])
}

assertNotNull(property.extra[IsVar])
inheritorProperties.single { it.name == "b" }.let { immutableProperty ->
assertNotNull(immutableProperty.getter)
assertNull(immutableProperty.setter)

val inheritedFrom = immutableProperty.extra[InheritedMember]?.inheritedFrom?.values?.single()
assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)

assertNull(immutableProperty.extra[IsVar])
}
}
}
}
Expand Down
Expand Up @@ -376,7 +376,7 @@ class DefaultPsiToDocumentableTranslatorTest : BaseAbstractTest() {
}

@Test
fun `should add IsVar for java field without any accessors`() {
fun `should add IsVar for non-final java field without any accessors`() {
testInline(
"""
|/src/main/java/test/A.java
Expand All @@ -395,4 +395,25 @@ class DefaultPsiToDocumentableTranslatorTest : BaseAbstractTest() {
}
}
}

@Test
fun `should not add IsVar for final java field`() {
testInline(
"""
|/src/main/java/test/A.java
|package test;
|public class A {
| public final int a = 2;
|}
""".trimIndent(),
configuration
) {
documentablesMergingStage = { module ->
val testedClass = module.packages.single().classlikes.single { it.name == "A" }

val publicFinal = testedClass.properties.single { it.name == "a" }
assertNull(publicFinal.extra[IsVar])
}
}
}
}

0 comments on commit dba2829

Please sign in to comment.