Make java properties inherit visibility from getters

Kotlin · Jun 16, 2022 · b1acc1f · b1acc1f
1 parent 1f54363
commit b1acc1f
Show file tree

Hide file tree

Showing 10 changed files with 709 additions and 408 deletions.
diff --git a/...base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt b/...base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt
@@ -41,6 +41,7 @@ import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.annotations.Annotated
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
+import org.jetbrains.kotlin.descriptors.java.JavaVisibilities
 import org.jetbrains.kotlin.idea.kdoc.findKDoc
 import org.jetbrains.kotlin.idea.kdoc.resolveKDocLink
 import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi
@@ -501,16 +502,19 @@ private class DokkaDescriptorVisitor(
     }
 
     private fun PropertyDescriptor.getVisibility(implicitAccessors: DescriptorAccessorHolder?): Visibility {
-        val isPrivateJavaPropertyWithPublicGetter =
-            this is JavaPropertyDescriptor
-                    && !this.visibility.isPublicAPI
-                    && implicitAccessors?.getter?.visibility?.isPublicAPI == true
+        val isNonPublicJavaProperty = this is JavaPropertyDescriptor && !this.visibility.isPublicAPI
+        val visibility =
+            if (isNonPublicJavaProperty) {
+                // only try to take implicit getter's visibility if it's a java property
+                // because it's not guaranteed that implicit accessor will be used
+                // for the kotlin property, as it may have an explicit accessor of its own,
+                // i.e in data classes or with get() and set() are overridden
+                (implicitAccessors?.getter?.visibility ?: this.visibility)
+            } else {
+                this.visibility
+            }
 
-        return if (isPrivateJavaPropertyWithPublicGetter) {
-            KotlinVisibility.Public
-        } else {
-            return this.visibility.toDokkaVisibility()
-        }
+        return visibility.toDokkaVisibility()
     }
 
     private fun CallableMemberDescriptor.createDRI(wasOverridenBy: DRI? = null): Pair<DRI, DRI?> =
@@ -1161,11 +1165,14 @@ private class DokkaDescriptorVisitor(
             }) + ancestry.interfaces.map { TypeConstructorWithKind(it.typeConstructor, KotlinClassKindTypes.INTERFACE) }
     }
 
-    private fun DescriptorVisibility.toDokkaVisibility(): org.jetbrains.dokka.model.Visibility = when (this.delegate) {
+    private fun DescriptorVisibility.toDokkaVisibility(): Visibility = when (this.delegate) {
         Visibilities.Public -> KotlinVisibility.Public
         Visibilities.Protected -> KotlinVisibility.Protected
         Visibilities.Internal -> KotlinVisibility.Internal
         Visibilities.Private -> KotlinVisibility.Private
+        JavaVisibilities.ProtectedAndPackage -> KotlinVisibility.Protected
+        JavaVisibilities.ProtectedStaticVisibility -> KotlinVisibility.Protected
+        JavaVisibilities.PackageVisibility -> JavaVisibility.Default
         else -> KotlinVisibility.Public
     }
 

diff --git a/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt
@@ -103,16 +103,6 @@ class DefaultPsiToDocumentableTranslator(
 
         private val cachedBounds = hashMapOf<String, Bound>()
 
-        private fun PsiModifierListOwner.getVisibility() = modifierList?.let {
-            val ml = it.children.toList()
-            when {
-                ml.any { it.text == PsiKeyword.PUBLIC } || it.hasModifierProperty("public") -> JavaVisibility.Public
-                ml.any { it.text == PsiKeyword.PROTECTED } || it.hasModifierProperty("protected") -> JavaVisibility.Protected
-                ml.any { it.text == PsiKeyword.PRIVATE } || it.hasModifierProperty("private") -> JavaVisibility.Private
-                else -> JavaVisibility.Default
-            }
-        } ?: JavaVisibility.Default
-
         private val PsiMethod.hash: Int
             get() = "$returnType $name$parameterList".hashCode()
 
@@ -662,19 +652,7 @@ 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
+            return getter?.visibility?.get(sourceSetData) ?: this.getVisibility()
         }
 
         private fun Collection<PsiAnnotation>.toListOfAnnotations() =
@@ -751,3 +729,13 @@ class DefaultPsiToDocumentableTranslator(
             get() = getChildOfType<PsiJavaCodeReferenceElement>()?.resolve()
     }
 }
+
+internal fun PsiModifierListOwner.getVisibility() = modifierList?.let {
+    val ml = it.children.toList()
+    when {
+        ml.any { it.text == PsiKeyword.PUBLIC } || it.hasModifierProperty("public") -> JavaVisibility.Public
+        ml.any { it.text == PsiKeyword.PROTECTED } || it.hasModifierProperty("protected") -> JavaVisibility.Protected
+        ml.any { it.text == PsiKeyword.PRIVATE } || it.hasModifierProperty("private") -> JavaVisibility.Private
+        else -> JavaVisibility.Default
+    }
+} ?: JavaVisibility.Default
diff --git a/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt
@@ -3,6 +3,9 @@ package org.jetbrains.dokka.base.translators.psi
 import com.intellij.psi.PsiField
 import com.intellij.psi.PsiMethod
 import org.jetbrains.dokka.base.translators.firstNotNullOfOrNull
+import org.jetbrains.dokka.model.JavaVisibility
+import org.jetbrains.dokka.model.KotlinVisibility
+import org.jetbrains.dokka.model.Visibility
 import org.jetbrains.kotlin.load.java.JvmAbi
 import org.jetbrains.kotlin.load.java.propertyNameByGetMethodName
 import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName
@@ -69,7 +72,9 @@ internal fun PsiMethod.getPossiblePropertyNamesForFunction(): List<String> {
 }
 
 internal fun PsiMethod.isAccessorFor(field: PsiField): Boolean {
-    return this.isGetterFor(field) || this.isSetterFor(field)
+    return (this.isGetterFor(field) || this.isSetterFor(field))
+            && !field.getVisibility().isPublicAPI()
+            && this.getVisibility().isPublicAPI()
 }
 
 internal fun PsiMethod.isGetterFor(field: PsiField): Boolean {
@@ -79,3 +84,11 @@ internal fun PsiMethod.isGetterFor(field: PsiField): Boolean {
 internal fun PsiMethod.isSetterFor(field: PsiField): Boolean {
     return parameterList.getParameter(0)?.type == field.type
 }
+
+internal fun Visibility.isPublicAPI() = when(this) {
+    KotlinVisibility.Public,
+    KotlinVisibility.Protected,
+    JavaVisibility.Public,
+    JavaVisibility.Protected -> true
+    else -> false
+}