Skip to content

Commit

Permalink
Fix incorrectly labeling java properties as val/var
Browse files Browse the repository at this point in the history
Fixes #2539
  • Loading branch information
IgnatBeresnev committed Jun 16, 2022
1 parent 5896a48 commit be2ee8b
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 10 deletions.
7 changes: 7 additions & 0 deletions core/api/core.api
Expand Up @@ -1770,6 +1770,13 @@ public final class org/jetbrains/dokka/model/Invariance : org/jetbrains/dokka/mo
public fun toString ()Ljava/lang/String;
}

public final class org/jetbrains/dokka/model/IsVar : org/jetbrains/dokka/model/properties/ExtraProperty, org/jetbrains/dokka/model/properties/ExtraProperty$Key {
public static final field INSTANCE Lorg/jetbrains/dokka/model/IsVar;
public fun getKey ()Lorg/jetbrains/dokka/model/properties/ExtraProperty$Key;
public synthetic fun mergeStrategyFor (Ljava/lang/Object;Ljava/lang/Object;)Lorg/jetbrains/dokka/model/properties/MergeStrategy;
public fun mergeStrategyFor (Lorg/jetbrains/dokka/model/IsVar;Lorg/jetbrains/dokka/model/IsVar;)Lorg/jetbrains/dokka/model/properties/MergeStrategy;
}

public final class org/jetbrains/dokka/model/JavaClassKindTypes : java/lang/Enum, org/jetbrains/dokka/model/ClassKind {
public static final field ANNOTATION_CLASS Lorg/jetbrains/dokka/model/JavaClassKindTypes;
public static final field CLASS Lorg/jetbrains/dokka/model/JavaClassKindTypes;
Expand Down
10 changes: 10 additions & 0 deletions core/src/main/kotlin/model/documentableProperties.kt
Expand Up @@ -39,6 +39,16 @@ object ObviousMember : ExtraProperty<Documentable>, ExtraProperty.Key<Documentab
override val key: ExtraProperty.Key<Documentable, *> = this
}

/**
* Whether this [DProperty] is `var` or `val`, should be present both in Kotlin and in Java properties
*
* In case of properties that came from `Java`, [IsVar] is added if
* the java field has no accessors at all (plain field) or has a setter
*/
object IsVar : ExtraProperty<DProperty>, ExtraProperty.Key<DProperty, IsVar> {
override val key: ExtraProperty.Key<DProperty, *> = this
}

data class CheckedExceptions(val exceptions: SourceSetDependent<List<DRI>>) : ExtraProperty<Documentable>, ExtraProperty.Key<Documentable, ObviousMember> {
companion object : ExtraProperty.Key<Documentable, CheckedExceptions> {
override fun mergeStrategyFor(left: CheckedExceptions, right: CheckedExceptions) =
Expand Down
Expand Up @@ -261,7 +261,7 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
if (it is JavaModifier.Empty) KotlinModifier.Open else it
}?.name?.let { keyword("$it ") }
p.modifiers()[sourceSet]?.toSignatureString()?.let { keyword(it) }
p.setter?.let { keyword("var ") } ?: keyword("val ")
if (p.isMutable()) keyword("var ") else keyword("val ")
list(p.generics, prefix = "<", suffix = "> ",
separatorStyles = mainStyles + TokenStyle.Punctuation,
surroundingCharactersStyle = mainStyles + TokenStyle.Operator) {
Expand All @@ -279,6 +279,10 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
}
}

private fun DProperty.isMutable(): Boolean {
return this.extra[IsVar] != null || this.setter != null
}

private fun PageContentBuilder.DocumentableContentBuilder.highlightValue(expr: Expression) = when (expr) {
is IntegerConstant -> constant(expr.value.toString())
is FloatConstant -> constant(expr.value.toString() + "f")
Expand Down
Expand Up @@ -470,15 +470,27 @@ 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(),
receiver = descriptor.extensionReceiverParameter?.let {
visitReceiverParameterDescriptor(it, DRIWithPlatformInfo(dri, actual))
},
sources = actual,
getter = getDescriptorGetter() ?: getImplicitAccessorGetter(),
setter = getDescriptorSetter() ?: getImplicitAccessorSetter(),
getter = getter,
setter = setter,
visibility = descriptor.getVisibility(implicitAccessors).toSourceSetDependent(),
documentation = descriptor.resolveDescriptorData(),
modifier = descriptor.modifier().toSourceSetDependent(),
Expand All @@ -495,6 +507,7 @@ private class DokkaDescriptorVisitor(
.toAnnotations(),
descriptor.getDefaultValue()?.let { DefaultValue(it.toSourceSetDependent()) },
inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) },
takeIf { isVar }?.let { IsVar },
)
)
)
Expand Down
Expand Up @@ -620,6 +620,11 @@ 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)

return DProperty(
dri = dri,
name = psi.name,
Expand All @@ -645,7 +650,8 @@ class DefaultPsiToDocumentableTranslator(
PropertyContainer.withAll(
inheritedFrom?.let { inheritedFrom -> InheritedMember(inheritedFrom.toSourceSetDependent()) },
it.toSourceSetDependent().toAdditionalModifiers(),
annotations.toSourceSetDependent().toAnnotations()
annotations.toSourceSetDependent().toAnnotations(),
takeIf { isVar }?.let { IsVar }
)
}
)
Expand Down
Expand Up @@ -275,7 +275,7 @@ class FunctionalTypeConstructorsSignatureTest : BaseAbstractTest() {
) {
renderingStage = { _, _ ->
writerPlugin.writer.renderedContent("root/example/-java-class/index.html").lastSignature().match(
"open val ", A("javaFunction"), ": (", A("Integer"), ") -> ", A("String"), Span(),
"open var ", A("javaFunction"), ": (", A("Integer"), ") -> ", A("String"), Span(),
ignoreSpanWithTokenStyle = true
)
}
Expand All @@ -301,7 +301,7 @@ class FunctionalTypeConstructorsSignatureTest : BaseAbstractTest() {
) {
renderingStage = { _, _ ->
writerPlugin.writer.renderedContent("root/example/-java-class/index.html").lastSignature().match(
"open val ", A("kotlinFunction"), ": (", A("Integer"), ") -> ", A("String"), Span(),
"open var ", A("kotlinFunction"), ": (", A("Integer"), ") -> ", A("String"), Span(),
ignoreSpanWithTokenStyle = true
)
}
Expand Down
Expand Up @@ -213,7 +213,7 @@ class InheritedAccessorsSignatureTest : BaseAbstractTest() {

val property = signatures[4]
property.match(
"val ", A("a"), ":", A("Int"), Span(),
"var ", A("a"), ":", A("Int"), Span(),
ignoreSpanWithTokenStyle = true
)
}
Expand Down Expand Up @@ -421,7 +421,7 @@ class InheritedAccessorsSignatureTest : BaseAbstractTest() {

val property = signatures[2]
property.match(
"protected val ", A("protectedProperty"), ":", A("Int"), Span(),
"protected var ", A("protectedProperty"), ":", A("Int"), Span(),
ignoreSpanWithTokenStyle = true
)
}
Expand Down
44 changes: 44 additions & 0 deletions plugins/base/src/test/kotlin/signatures/SignatureTest.kt
Expand Up @@ -951,4 +951,48 @@ class SignatureTest : BaseAbstractTest() {
}
}
}

@Test
fun `java property without accessors should be var`() {
val writerPlugin = TestOutputWriterPlugin()
testInline(
"""
|/src/test/JavaClass.java
|package test;
|public class JavaClass {
| public int property = 0;
|}
|
|/src/test/KotlinClass.kt
|package test
|open class KotlinClass : JavaClass() { }
""".trimIndent(),
configuration,
pluginOverrides = listOf(writerPlugin)
) {
renderingStage = { _, _ ->
writerPlugin.writer.renderedContent("root/test/-kotlin-class/index.html").let { kotlinClassContent ->
val signatures = kotlinClassContent.signature().toList()
assertEquals(3, signatures.size, "Expected 2 signatures: class signature, constructor and property")

val property = signatures[2]
property.match(
"var ", A("property"), ":", A("Int"), Span(),
ignoreSpanWithTokenStyle = true
)
}

writerPlugin.writer.renderedContent("root/test/-java-class/index.html").let { kotlinClassContent ->
val signatures = kotlinClassContent.signature().toList()
assertEquals(2, signatures.size, "Expected 2 signatures: class signature and property")

val property = signatures[1]
property.match(
"open var ", A("property"), ":", A("Int"), Span(),
ignoreSpanWithTokenStyle = true
)
}
}
}
}
}
Expand Up @@ -4,6 +4,7 @@ import org.jetbrains.dokka.DokkaConfiguration
import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.model.InheritedMember
import org.jetbrains.dokka.model.IsVar
import org.jetbrains.dokka.model.KotlinVisibility
import org.junit.jupiter.api.Test
import kotlin.test.assertEquals
Expand Down Expand Up @@ -51,6 +52,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {

val getterInheritedFrom = property.getter?.extra?.get(InheritedMember)?.inheritedFrom?.values?.single()
assertEquals(DRI(packageName = "test", classNames = "A"), getterInheritedFrom)

assertNull(property.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -129,6 +132,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {
assertEquals("setA", setter.name)
val setterInheritedFrom = setter.extra[InheritedMember]?.inheritedFrom?.values?.single()
assertEquals(DRI(packageName = "test", classNames = "A"), setterInheritedFrom)

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -162,6 +167,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {
val setter = boolProperty.setter
assertNotNull(setter)
assertEquals("setBool", setter.name)

assertNotNull(boolProperty.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -211,6 +218,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {

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

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -267,6 +276,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {

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

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -315,6 +326,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {

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

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down
@@ -1,10 +1,10 @@
package superFields

import org.jetbrains.dokka.DokkaConfiguration
import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.model.Annotations
import org.jetbrains.dokka.model.InheritedMember
import org.jetbrains.dokka.model.IsVar
import org.jetbrains.dokka.model.isJvmField
import org.junit.jupiter.api.Assertions.assertNotNull
import org.junit.jupiter.api.Assertions.assertNull
Expand Down Expand Up @@ -75,6 +75,8 @@ class PsiSuperFieldsTest : BaseAbstractTest() {

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

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -107,6 +109,8 @@ class PsiSuperFieldsTest : BaseAbstractTest() {

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

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down Expand Up @@ -151,6 +155,8 @@ class PsiSuperFieldsTest : BaseAbstractTest() {

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

assertNotNull(property.extra[IsVar])
}
}
}
Expand Down
Expand Up @@ -9,6 +9,7 @@ import org.jetbrains.dokka.model.doc.P
import org.jetbrains.dokka.model.doc.Text
import org.junit.Assert
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test
import kotlin.test.assertNotNull
Expand Down Expand Up @@ -735,6 +736,34 @@ class DefaultDescriptorToDocumentableTranslatorTest : BaseAbstractTest() {
}
}
}

@Test
fun `should correctly add IsVar extra for properties`() {
testInline(
"""
|/src/main/kotlin/A.kt
|package test
|class A {
| public var mutable: Int = 0
| public val immutable: Int = 0
|}
""".trimIndent(),
configuration
) {
documentablesMergingStage = { module ->
val testClass = module.packages.single().classlikes.single { it.name == "A" }
assertEquals(2, testClass.properties.size)

val mutable = testClass.properties[0]
assertEquals("mutable", mutable.name)
assertNotNull(mutable.extra[IsVar])

val immutable = testClass.properties[1]
assertEquals("immutable", immutable.name)
assertNull(immutable.extra[IsVar])
}
}
}
}

private sealed class TestSuite {
Expand Down

0 comments on commit be2ee8b

Please sign in to comment.