Fix gathering inherited properties (#2481)

* Fix gathering inherited properties in PSI

* Refacotr KaJ transformer. Change wrapping TagWrapper for getters and setters.

* Add logic to merge inherited properties in kotlin from java sources.

* Remove getters and setters from JvmField properties for DObject, DEnum, DInterface in KaJ.

* Unify InheritedMember DRI logic.

* Fix gathering docs obtained from inheriting java sources in descriptors

* Apply requested changes.

* Resolve rebase conflicts

* Use 221 for qodana analysis

* Move accessors generation into DefaultDescriptorToDocumentableTranslator

* Fix special "is" case for accessors and refactor logic in general

* Remove ambiguous import after rebasing

* Remove unused imports and format code

* Apply review comment suggestions

* Preserve previously lost accessor lookalikes

* Extract a variable and correct a typo

Co-authored-by: Andrzej Ratajczak <andrzej.ratajczak98@gmail.com>

3 files changed, 384 insertions, 0 deletions

diff --git a/plugins/base/src/test/kotlin/model/PropertyTest.kt b/plugins/base/src/test/kotlin/model/PropertyTest.kt
index 17f526f3..dc35d621 100644
--- a/plugins/base/src/test/kotlin/model/PropertyTest.kt
+++ b/plugins/base/src/test/kotlin/model/PropertyTest.kt
@@ -1,10 +1,13 @@
 package model
 
+import org.jetbrains.dokka.links.Callable
+import org.jetbrains.dokka.links.DRI
 import org.jetbrains.dokka.model.*
 import org.junit.jupiter.api.Test
 import utils.AbstractModelTest
 import utils.assertNotNull
 import utils.name
+import kotlin.test.assertEquals
 
 class PropertyTest : AbstractModelTest("/src/main/kotlin/property/Test.kt", "property") {
 
@@ -158,6 +161,10 @@ class PropertyTest : AbstractModelTest("/src/main/kotlin/property/Test.kt", "pro
                     with(getter.assertNotNull("Getter")) {
                         type.name equals "Int"
                     }
+                    extra[InheritedMember]?.inheritedFrom?.values?.single()?.run {
+                        classNames equals "Foo"
+                        callable equals null
+                    }
                 }
             }
         }
diff --git a/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt
new file mode 100644
index 00000000..a6dd4350
--- /dev/null
+++ b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt
@@ -0,0 +1,191 @@
+package superFields
+
+import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
+import org.jetbrains.dokka.links.DRI
+import org.jetbrains.dokka.model.InheritedMember
+import org.junit.jupiter.api.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
+
+class DescriptorSuperPropertiesTest : BaseAbstractTest() {
+
+    private val commonTestConfiguration = dokkaConfiguration {
+        sourceSets {
+            sourceSet {
+                sourceRoots = listOf("src/")
+                analysisPlatform = "jvm"
+                name = "jvm"
+            }
+        }
+    }
+
+    @Test
+    fun `kotlin inheriting java should append only getter`() {
+        testInline(
+            """
+            |/src/test/A.java
+            |package test;
+            |public class A {
+            |   private int a = 1;
+            |   public int getA() { return a; }
+            |}
+            |
+            |/src/test/B.kt
+            |package test
+            |class B : A {}
+        """.trimIndent(),
+            commonTestConfiguration
+        ) {
+            this.documentablesTransformationStage = { module ->
+                val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
+
+                val property = kotlinProperties.single { it.name == "a" }
+                val propertyInheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), propertyInheritedFrom)
+
+                assertNull(property.setter)
+                assertNotNull(property.getter)
+
+                val getterInheritedFrom = property.getter?.extra?.get(InheritedMember)?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), getterInheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `kotlin inheriting java should append getter and setter`() {
+        testInline(
+            """
+            |/src/test/A.java
+            |package test;
+            |public class A {
+            |   private int a = 1;
+            |   public int getA() { return a; }
+            |   public void setA(int a) { this.a = a; }
+            |}
+            |
+            |/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" }
+                property.extra[InheritedMember]?.inheritedFrom?.values?.single()?.run {
+                    assertEquals(
+                        DRI(packageName = "test", classNames = "A"),
+                        this
+                    )
+                }
+
+                val getter = property.getter
+                assertNotNull(getter)
+                assertEquals("getA", getter.name)
+                val getterInheritedFrom = getter.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), getterInheritedFrom)
+
+                val setter = property.setter
+                assertNotNull(setter)
+                assertEquals("setA", setter.name)
+                val setterInheritedFrom = setter.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), setterInheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `should have special getter and setter names for boolean property inherited from java`() {
+        testInline(
+            """
+            |/src/test/A.java
+            |package test;
+            |public class A {
+            |   private boolean bool = true;
+            |   public boolean isBool() { return bool; }
+            |   public void setBool(boolean bool) { this.bool = bool; }
+            |}
+            |
+            |/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 boolProperty = kotlinProperties.single { it.name == "bool" }
+
+                val getter = boolProperty.getter
+                assertNotNull(getter)
+                assertEquals("isBool", getter.name)
+
+                val setter = boolProperty.setter
+                assertNotNull(setter)
+                assertEquals("setBool", setter.name)
+            }
+        }
+    }
+
+    @Test
+    fun `kotlin inheriting java should not append anything since field is public`() {
+        testInline(
+            """
+            |/src/test/A.java
+            |package test;
+            |public class A {
+            |   public int a = 1;
+            |   public int getA() { return a; }
+            |   public void setA(int a) { this.a = a; }
+            |}
+            |
+            |/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.getter)
+                assertNull(property.setter)
+
+                val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `should preserve regular functions that look like accessors, but are not accessors`() {
+        testInline(
+            """
+            |/src/test/A.kt
+            |package test
+            |class A {
+            |    val v = 0
+            |    fun setV() { println(10) } // no arg
+            |    fun getV(): String { return "s" } // wrong return type
+            |}
+        """.trimIndent(),
+            commonTestConfiguration
+        ) {
+            documentablesMergingStage = { module ->
+                val testClass = module.packages.single().classlikes.single { it.name == "A" }
+                val setterLookalike = testClass.functions.firstOrNull { it.name == "setV" }
+                assertNotNull(setterLookalike) {
+                    "Expected regular function not found, wrongly categorized as setter?"
+                }
+
+                val getterLookalike = testClass.functions.firstOrNull { it.name == "getV" }
+                assertNotNull(getterLookalike) {
+                    "Expected regular function not found, wrongly categorized as getter?"
+                }
+            }
+        }
+    }
+}
diff --git a/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt b/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt
new file mode 100644
index 00000000..025c9b06
--- /dev/null
+++ b/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt
@@ -0,0 +1,186 @@
+package superFields
+
+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.isJvmField
+import org.junit.jupiter.api.Assertions.assertNotNull
+import org.junit.jupiter.api.Assertions.assertNull
+import org.junit.jupiter.api.Test
+import kotlin.test.assertEquals
+
+
+class PsiSuperFieldsTest : BaseAbstractTest() {
+
+    private val commonTestConfiguration = dokkaConfiguration {
+        sourceSets {
+            sourceSet {
+                sourceRoots = listOf("src/")
+                analysisPlatform = "jvm"
+                name = "jvm"
+            }
+        }
+    }
+
+    @Test
+    fun `java inheriting java`() {
+        testInline(
+            """
+            |/src/test/A.java
+            |package test;
+            |public class A {
+            |    public int a = 1;
+            |}
+            |
+            |/src/test/B.java
+            |package test;
+            |public class B extends A {}
+        """.trimIndent(),
+            commonTestConfiguration
+        ) {
+            documentablesMergingStage = { module ->
+                val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
+                val property = inheritorProperties.single { it.name == "a" }
+
+                val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `java inheriting kotlin common case`() {
+        testInline(
+            """
+            |/src/test/A.kt
+            |package test
+            |open class A {
+            |    var a: Int = 1
+            |}
+            |
+            |/src/test/B.java
+            |package test;
+            |public class B extends A {}
+        """.trimIndent(),
+            commonTestConfiguration
+        ) {
+            documentablesMergingStage = { module ->
+                val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
+                val property = inheritorProperties.single { it.name == "a" }
+
+                assertNotNull(property.getter)
+                assertNotNull(property.setter)
+
+                val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `java inheriting kotlin with boolean property`() {
+        testInline(
+            """
+            |/src/test/A.kt
+            |package test
+            |open class A {
+            |    var isActive: Boolean = true
+            |}
+            |
+            |/src/test/B.java
+            |package test;
+            |public class B extends A {}
+        """.trimIndent(),
+            commonTestConfiguration
+        ) {
+            documentablesMergingStage = { module ->
+                val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
+                val property = inheritorProperties.single { it.name == "isActive" }
+
+                assertNotNull(property.getter)
+                assertEquals("isActive", property.getter?.name)
+
+                assertNotNull(property.setter)
+                assertEquals("setActive", property.setter?.name)
+
+                val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `java inheriting kotlin with @JvmField should not inherit accessors`() {
+        testInline(
+            """
+            |/src/test/A.kt
+            |package test
+            |open class A {
+            |    @kotlin.jvm.JvmField
+            |    var a: Int = 1
+            |}
+            |
+            |/src/test/B.java
+            |package test;
+            |public class B extends A {}
+        """.trimIndent(),
+            dokkaConfiguration {
+                sourceSets {
+                    sourceSet {
+                        sourceRoots = listOf("src/")
+                        analysisPlatform = "jvm"
+                        name = "jvm"
+                        classpath += jvmStdlibPath!! // needed for JvmField
+                    }
+                }
+            }
+        ) {
+            documentablesMergingStage = { module ->
+                val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties
+                val property = inheritorProperties.single { it.name == "a" }
+
+                assertNull(property.getter)
+                assertNull(property.setter)
+
+                val jvmFieldAnnotation = property.extra[Annotations]?.directAnnotations?.values?.single()?.find {
+                    it.isJvmField()
+                }
+                assertNotNull(jvmFieldAnnotation)
+
+                val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single()
+                assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom)
+            }
+        }
+    }
+
+    @Test
+    fun `should preserve regular functions that look like accessors, but are not accessors`() {
+        testInline(
+            """
+            |/src/test/A.java
+            |package test;
+            |public class A {
+            |    public int a = 1;
+            |    public void setA() { } // no arg
+            |    public String getA() { return "s"; } // wrong return type
+            |}
+        """.trimIndent(),
+            commonTestConfiguration
+        ) {
+            documentablesMergingStage = { module ->
+                val testClass = module.packages.single().classlikes.single { it.name == "A" }
+
+                val setterLookalike = testClass.functions.firstOrNull { it.name == "setA" }
+                assertNotNull(setterLookalike) {
+                    "Expected regular function not found, wrongly categorized as setter?"
+                }
+
+                val getterLookalike = testClass.functions.firstOrNull { it.name == "getA" }
+                assertNotNull(getterLookalike) {
+                    "Expected regular function not found, wrongly categorized as getter?"
+                }
+            }
+        }
+    }
+}