Fix incorrectly labeling java properties as val/var

Fixes #2539

core/api/core.api

@@ -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;

core/src/main/kotlin/model/documentableProperties.kt

@@ -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) =

plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt

@@ -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) {
@@ -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")

plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt

@@ -469,6 +469,8 @@ private class DokkaDescriptorVisitor(
 
         return coroutineScope {
             val generics = async { descriptor.typeParameters.parallelMap { it.toVariantTypeParameter() } }
+            val getter = getDescriptorGetter() ?: getImplicitAccessorGetter()
+            val setter = getDescriptorSetter() ?: getImplicitAccessorSetter()
 
             DProperty(
                 dri = dri,
@@ -477,8 +479,8 @@ private class DokkaDescriptorVisitor(
                     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(),
@@ -495,12 +497,26 @@ private class DokkaDescriptorVisitor(
                             .toAnnotations(),
                         descriptor.getDefaultValue()?.let { DefaultValue(it.toSourceSetDependent()) },
                         inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) },
+                        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 =

plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt

@@ -620,6 +620,12 @@ class DefaultPsiToDocumentableTranslator(
 
         private fun parseField(psi: PsiField, getter: DFunction?, setter: DFunction?, inheritedFrom: DRI? = null): DProperty {
             val dri = DRI.from(psi)
+
+            // 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,
                 name = psi.name,
@@ -645,7 +651,8 @@ class DefaultPsiToDocumentableTranslator(
                     PropertyContainer.withAll(
                         inheritedFrom?.let { inheritedFrom -> InheritedMember(inheritedFrom.toSourceSetDependent()) },
                         it.toSourceSetDependent().toAdditionalModifiers(),
-                        annotations.toSourceSetDependent().toAnnotations()
+                        annotations.toSourceSetDependent().toAnnotations(),
+                        takeIf { isVar }?.let { IsVar }
                     )
                 }
             )

plugins/base/src/test/kotlin/signatures/FunctionalTypeConstructorsSignatureTest.kt

@@ -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
                 )
             }
@@ -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
                 )
             }

plugins/base/src/test/kotlin/signatures/InheritedAccessorsSignatureTest.kt

@@ -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
                 )
             }
@@ -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
                     )
                 }

plugins/base/src/test/kotlin/signatures/SignatureTest.kt

@@ -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
+                    )
+                }
+            }
+        }
+    }
 }

plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt

@@ -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
@@ -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])
             }
         }
     }
@@ -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])
             }
         }
     }
@@ -162,6 +167,8 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {
                 val setter = boolProperty.setter
                 assertNotNull(setter)
                 assertEquals("setBool", setter.name)
+
+                assertNotNull(boolProperty.extra[IsVar])
             }
         }
     }
@@ -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])
             }
         }
     }
@@ -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])
             }
         }
     }
@@ -315,6 +326,33 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() {
 
                 val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
                 assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+
+                assertNotNull(property.extra[IsVar])
+            }
+        }
+    }
+
+    @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])
             }
         }
     }

plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt

@@ -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
@@ -58,6 +58,7 @@ class PsiSuperFieldsTest : BaseAbstractTest() {
             |package test
             |open class A {
             |    var a: Int = 1
+            |    val b: Int = 2
             |}
             |
             |/src/test/B.java
@@ -68,13 +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])
+                }
+
+                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])
+                }
             }
         }
     }
@@ -107,6 +120,8 @@ class PsiSuperFieldsTest : BaseAbstractTest() {
 
                 val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
                 assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+
+                assertNotNull(property.extra[IsVar])
             }
         }
     }
@@ -151,6 +166,8 @@ class PsiSuperFieldsTest : BaseAbstractTest() {
 
                 val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
                 assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+
+                assertNotNull(property.extra[IsVar])
             }
         }
     }