Handle more corner cases for inherited accessors

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

@@ -21,6 +21,7 @@ import org.jetbrains.dokka.links.Callable
 import org.jetbrains.dokka.model.*
 import org.jetbrains.dokka.model.AnnotationTarget
 import org.jetbrains.dokka.model.Nullable
+import org.jetbrains.dokka.model.Visibility
 import org.jetbrains.dokka.model.doc.*
 import org.jetbrains.dokka.model.properties.PropertyContainer
 import org.jetbrains.dokka.plugability.DokkaContext
@@ -43,6 +44,7 @@ import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
 import org.jetbrains.kotlin.idea.kdoc.findKDoc
 import org.jetbrains.kotlin.idea.kdoc.resolveKDocLink
 import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi
+import org.jetbrains.kotlin.load.java.descriptors.JavaPropertyDescriptor
 import org.jetbrains.kotlin.load.kotlin.toSourceElement
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
@@ -379,7 +381,7 @@ private class DokkaDescriptorVisitor(
         return coroutineScope {
             val descriptorsWithKind = scope.getDescriptorsWithKind()
 
-            val (regularFunctions, accessors) = splitFunctionsAndAccessors(
+            val (regularFunctions, accessors) = splitFunctionsAndInheritedAccessors(
                 properties = descriptorsWithKind.properties,
                 functions = descriptorsWithKind.functions
             )
@@ -427,11 +429,11 @@ private class DokkaDescriptorVisitor(
 
     /**
      * @param implicitAccessors getters/setters that are not part of the property descriptor, for instance
-     *                          average methods inherited from java sources
+     *                          average methods inherited from java sources that access the property
      */
     private suspend fun visitPropertyDescriptor(
         originalDescriptor: PropertyDescriptor,
-        implicitAccessors: List<FunctionDescriptor>,
+        implicitAccessors: DescriptorAccessorHolder?,
         parent: DRIWithPlatformInfo
     ): DProperty {
         val (dri, _) = originalDescriptor.createDRI()
@@ -451,9 +453,7 @@ private class DokkaDescriptorVisitor(
                 }
 
         suspend fun getImplicitAccessorGetter() =
-            implicitAccessors
-                .firstOrNull { it.isGetterFor(originalDescriptor) }
-                ?.let { visitFunctionDescriptor(it, parent) }
+            implicitAccessors?.getter?.let { visitFunctionDescriptor(it, parent) }
 
         // example - generated setter that comes with data classes
         suspend fun getDescriptorSetter() =
@@ -464,9 +464,7 @@ private class DokkaDescriptorVisitor(
                 }
 
         suspend fun getImplicitAccessorSetter() =
-            implicitAccessors
-                .firstOrNull { it.isSetterFor(originalDescriptor) }
-                ?.let { visitFunctionDescriptor(it, parent) }
+            implicitAccessors?.setter?.let { visitFunctionDescriptor(it, parent) }
 
         return coroutineScope {
             val generics = async { descriptor.typeParameters.parallelMap { it.toVariantTypeParameter() } }
@@ -480,7 +478,7 @@ private class DokkaDescriptorVisitor(
                 sources = actual,
                 getter = getDescriptorGetter() ?: getImplicitAccessorGetter(),
                 setter = getDescriptorSetter() ?: getImplicitAccessorSetter(),
-                visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(),
+                visibility = descriptor.getVisibility(implicitAccessors).toSourceSetDependent(),
                 documentation = descriptor.resolveDescriptorData(),
                 modifier = descriptor.modifier().toSourceSetDependent(),
                 type = descriptor.returnType!!.toBound(),
@@ -502,6 +500,19 @@ private class DokkaDescriptorVisitor(
         }
     }
 
+    private fun PropertyDescriptor.getVisibility(implicitAccessors: DescriptorAccessorHolder?): Visibility {
+        val isPrivateJavaPropertyWithPublicGetter =
+            this is JavaPropertyDescriptor
+                    && !this.visibility.isPublicAPI
+                    && implicitAccessors?.getter?.visibility?.isPublicAPI == true
+
+        return if (isPrivateJavaPropertyWithPublicGetter) {
+            KotlinVisibility.Public
+        } else {
+            return this.visibility.toDokkaVisibility()
+        }
+    }
+
     private fun CallableMemberDescriptor.createDRI(wasOverridenBy: DRI? = null): Pair<DRI, DRI?> =
         if (kind == CallableMemberDescriptor.Kind.DECLARATION || overriddenDescriptors.isEmpty())
             Pair(DRI.from(this), wasOverridenBy)
@@ -818,12 +829,11 @@ private class DokkaDescriptorVisitor(
 
     private suspend fun List<PropertyDescriptor>.visitProperties(
         parent: DRIWithPlatformInfo,
-        implicitAccessors: Map<PropertyDescriptor, List<FunctionDescriptor>> = emptyMap(),
+        implicitAccessors: Map<PropertyDescriptor, DescriptorAccessorHolder> = emptyMap(),
     ): List<DProperty> {
         return coroutineScope {
             parallelMap {
-                val propertyAccessors = implicitAccessors[it] ?: emptyList()
-                visitPropertyDescriptor(it, propertyAccessors, parent)
+                visitPropertyDescriptor(it, implicitAccessors[it], parent)
             }
         }
     }

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

@@ -10,29 +10,93 @@ import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName
 
 internal data class DescriptorFunctionsHolder(
     val regularFunctions: List<FunctionDescriptor>,
-    val accessors: Map<PropertyDescriptor, List<FunctionDescriptor>>
+    val accessors: Map<PropertyDescriptor, DescriptorAccessorHolder>
 )
 
-internal fun splitFunctionsAndAccessors(
+internal data class DescriptorAccessorHolder(
+    val getter: FunctionDescriptor? = null,
+    val setter: FunctionDescriptor? = null
+)
+
+/**
+ * Separate regular Kotlin/Java functions and inherited Java accessors
+ * to properly display properties inherited from Java.
+ *
+ * Take this example:
+ * ```
+ * // java
+ * public class JavaClass {
+ *     private int a = 1;
+ *     public int getA() { return a; }
+ *     public void setA(int a) { this.a = a; }
+ * }
+ *
+ * // kotlin
+ * class Bar : JavaClass() {
+ *     fun foo() {}
+ * }
+ * ```
+ *
+ * It should result in:
+ * - 1 regular function `foo`
+ * - Map a=[`getA`, `setA`]
+ */
+internal fun splitFunctionsAndInheritedAccessors(
     properties: List<PropertyDescriptor>,
     functions: List<FunctionDescriptor>
 ): DescriptorFunctionsHolder {
+    val (javaMethods, kotlinFunctions) = functions.partition { it is JavaMethodDescriptor }
+    if (javaMethods.isEmpty()) {
+        return DescriptorFunctionsHolder(regularFunctions = kotlinFunctions, emptyMap())
+    }
+
     val propertiesByName = properties.associateBy { it.name.asString() }
-    val regularFunctions = mutableListOf<FunctionDescriptor>()
-    val accessors = mutableMapOf<PropertyDescriptor, MutableList<FunctionDescriptor>>()
-    functions.forEach { function ->
+    val regularFunctions = ArrayList<FunctionDescriptor>(kotlinFunctions)
+
+    val accessors = mutableMapOf<PropertyDescriptor, DescriptorAccessorHolder>()
+    javaMethods.forEach { function ->
         val possiblePropertyNamesForFunction = function.toPossiblePropertyNames()
         val property = possiblePropertyNamesForFunction.firstNotNullOfOrNull { propertiesByName[it] }
         if (property != null && function.isAccessorFor(property)) {
-            accessors.getOrPut(property, ::mutableListOf).add(function)
+            accessors.compute(property) { prop, accessorHolder ->
+                if (function.isGetterFor(prop))
+                    accessorHolder?.copy(getter = function) ?: DescriptorAccessorHolder(getter = function)
+                else
+                    accessorHolder?.copy(setter = function) ?: DescriptorAccessorHolder(setter = function)
+            }
         } else {
             regularFunctions.add(function)
         }
     }
+
+    val accessorLookalikes = removeNonAccessorsReturning(accessors)
+    regularFunctions.addAll(accessorLookalikes)
+
     return DescriptorFunctionsHolder(regularFunctions, accessors)
 }
 
-internal fun FunctionDescriptor.toPossiblePropertyNames(): List<String> {
+/**
+ * If a field has no getter, it's not accessible as a property from Kotlin's perspective,
+ * but it still might have a setter lookalike. In this case, this "setter" should be just a regular function
+ *
+ * @return removed elements
+ */
+private fun removeNonAccessorsReturning(
+    propertyAccessors: MutableMap<PropertyDescriptor, DescriptorAccessorHolder>
+): List<FunctionDescriptor> {
+    val nonAccessors = mutableListOf<FunctionDescriptor>()
+    propertyAccessors.entries.removeIf { (_, accessors) ->
+        if (accessors.getter == null && accessors.setter != null) {
+            nonAccessors.add(accessors.setter)
+            true
+        } else {
+            false
+        }
+    }
+    return nonAccessors
+}
+
+private fun FunctionDescriptor.toPossiblePropertyNames(): List<String> {
     val stringName = this.name.asString()
     return when {
         JvmAbi.isSetterName(stringName) -> propertyNamesBySetMethodName(this.name).map { it.asString() }
@@ -41,7 +105,7 @@ internal fun FunctionDescriptor.toPossiblePropertyNames(): List<String> {
     }
 }
 
-internal fun propertyNamesByGetMethod(functionDescriptor: FunctionDescriptor): List<String> {
+private fun propertyNamesByGetMethod(functionDescriptor: FunctionDescriptor): List<String> {
     val stringName = functionDescriptor.name.asString()
     // In java, the convention for boolean property accessors is as follows:
     // - `private boolean active;`
@@ -63,21 +127,19 @@ internal fun propertyNamesByGetMethod(functionDescriptor: FunctionDescriptor): L
     return listOfNotNull(javaPropName, kotlinPropName)
 }
 
-internal fun FunctionDescriptor.isAccessorFor(property: PropertyDescriptor): Boolean {
-    return this.isGetterFor(property) || this.isSetterFor(property)
+private fun FunctionDescriptor.isAccessorFor(property: PropertyDescriptor): Boolean {
+    return (this.isGetterFor(property) || this.isSetterFor(property))
+            && !property.visibility.isPublicAPI
+            && this.visibility.isPublicAPI
 }
 
-internal fun FunctionDescriptor.isGetterFor(property: PropertyDescriptor): Boolean {
+private fun FunctionDescriptor.isGetterFor(property: PropertyDescriptor): Boolean {
     return this.returnType == property.returnType
             && this.valueParameters.isEmpty()
-            && !property.visibility.isPublicAPI
-            && this.visibility.isPublicAPI
 }
 
-internal fun FunctionDescriptor.isSetterFor(property: PropertyDescriptor): Boolean {
+private fun FunctionDescriptor.isSetterFor(property: PropertyDescriptor): Boolean {
     return this.valueParameters.size == 1
             && this.valueParameters[0].type == property.returnType
-            && !property.visibility.isPublicAPI
-            && this.visibility.isPublicAPI
 }
 

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

@@ -17,9 +17,9 @@ import org.jetbrains.dokka.analysis.KotlinAnalysis
 import org.jetbrains.dokka.analysis.PsiDocumentableSource
 import org.jetbrains.dokka.analysis.from
 import org.jetbrains.dokka.base.DokkaBase
-import org.jetbrains.dokka.base.translators.typeConstructorsBeingExceptions
 import org.jetbrains.dokka.base.translators.psi.parsers.JavaDocumentationParser
 import org.jetbrains.dokka.base.translators.psi.parsers.JavadocParser
+import org.jetbrains.dokka.base.translators.typeConstructorsBeingExceptions
 import org.jetbrains.dokka.base.translators.unquotedValue
 import org.jetbrains.dokka.links.*
 import org.jetbrains.dokka.model.*
@@ -40,12 +40,7 @@ import org.jetbrains.kotlin.asJava.elements.KtLightAbstractAnnotation
 import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys
 import org.jetbrains.kotlin.cli.jvm.config.JavaSourceRoot
 import org.jetbrains.kotlin.idea.base.utils.fqname.getKotlinFqName
-import org.jetbrains.kotlin.load.java.JvmAbi
-import org.jetbrains.kotlin.load.java.propertyNameByGetMethodName
-import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName
-import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.psiUtil.getChildOfType
-import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.utils.KotlinExceptionWithAttachments
 import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 import java.io.File
@@ -641,7 +636,7 @@ class DefaultPsiToDocumentableTranslator(
                 documentation = javadocParser.parseDocumentation(psi).toSourceSetDependent(),
                 expectPresentInSet = null,
                 sources = PsiDocumentableSource(psi).toSourceSetDependent(),
-                visibility = psi.getVisibility().toSourceSetDependent(),
+                visibility = psi.getVisibility(getter).toSourceSetDependent(),
                 type = getBound(psi.type),
                 receiver = null,
                 setter = setter,
@@ -666,6 +661,22 @@ class DefaultPsiToDocumentableTranslator(
             )
         }
 
+        private fun PsiField.getVisibility(getter: DFunction?): Visibility {
+            val psiVisibility = this.getVisibility()
+            val isPrivatePropertyWithPublicGetter = !psiVisibility.isPublicAPI()
+                    && getter?.visibility?.get(sourceSetData)?.isPublicAPI() == true
+
+            return if (isPrivatePropertyWithPublicGetter) JavaVisibility.Public else psiVisibility
+        }
+
+        private fun Visibility.isPublicAPI() = when (this) {
+            JavaVisibility.Public,
+            JavaVisibility.Protected,
+            KotlinVisibility.Public,
+            KotlinVisibility.Protected -> true
+            else -> false
+        }
+
         private fun Collection<PsiAnnotation>.toListOfAnnotations() =
             filter { it !is KtLightAbstractAnnotation }.mapNotNull { it.toAnnotation() }
 

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

@@ -28,9 +28,32 @@ internal fun splitFunctionsAndAccessors(fields: Array<PsiField>, methods: Array<
             regularFunctions.add(method)
         }
     }
+
+    val accessorLookalikes = removeNonAccessorsReturning(accessors)
+    regularFunctions.addAll(accessorLookalikes)
+
     return PsiFunctionsHolder(regularFunctions, accessors)
 }
 
+/**
+ * If a field has no getter, it's not accessible as a property from Kotlin's perspective,
+ * but it still might have a setter. In this case, this "setter" should be just a regular function
+ */
+private fun removeNonAccessorsReturning(
+    fieldAccessors: MutableMap<PsiField, MutableList<PsiMethod>>
+): List<PsiMethod> {
+    val nonAccessors = mutableListOf<PsiMethod>()
+    fieldAccessors.entries.removeIf { (field, methods) ->
+        if (methods.size == 1 && methods[0].isSetterFor(field)) {
+            nonAccessors.add(methods[0])
+            true
+        } else {
+            false
+        }
+    }
+    return nonAccessors
+}
+
 internal fun PsiMethod.getPossiblePropertyNamesForFunction(): List<String> {
     val jvmName = getAnnotation(DescriptorUtils.JVM_NAME.asString())?.findAttributeValue("name")?.text
     return jvmName?.let { listOf(jvmName) }