Skip to content

Commit

Permalink
Fix gathering inherited properties (#2481)
Browse files Browse the repository at this point in the history
* 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>

(cherry picked from commit 623cf22)
  • Loading branch information
IgnatBeresnev committed Jun 1, 2022
1 parent aa25a2a commit c307a60
Show file tree
Hide file tree
Showing 14 changed files with 934 additions and 195 deletions.
Empty file added .github/workflows/qodana.yml
Empty file.
7 changes: 7 additions & 0 deletions core/api/core.api
Expand Up @@ -1837,6 +1837,13 @@ public final class org/jetbrains/dokka/model/JavaVisibility$Public : org/jetbrai
public static final field INSTANCE Lorg/jetbrains/dokka/model/JavaVisibility$Public;
}

public final class org/jetbrains/dokka/model/JvmFieldKt {
public static final field JVM_FIELD_CLASS_NAMES Ljava/lang/String;
public static final field JVM_FIELD_PACKAGE_NAME Ljava/lang/String;
public static final fun isJvmField (Lorg/jetbrains/dokka/links/DRI;)Z
public static final fun isJvmField (Lorg/jetbrains/dokka/model/Annotations$Annotation;)Z
}

public final class org/jetbrains/dokka/model/JvmNameKt {
public static final fun isJvmName (Lorg/jetbrains/dokka/links/DRI;)Z
public static final fun isJvmName (Lorg/jetbrains/dokka/model/Annotations$Annotation;)Z
Expand Down
10 changes: 10 additions & 0 deletions core/src/main/kotlin/model/JvmField.kt
@@ -0,0 +1,10 @@
package org.jetbrains.dokka.model

import org.jetbrains.dokka.links.DRI

const val JVM_FIELD_PACKAGE_NAME = "kotlin.jvm"
const val JVM_FIELD_CLASS_NAMES = "JvmField"

fun DRI.isJvmField(): Boolean = packageName == JVM_FIELD_PACKAGE_NAME && classNames == JVM_FIELD_CLASS_NAMES

fun Annotations.Annotation.isJvmField(): Boolean = dri.isJvmField()
12 changes: 12 additions & 0 deletions plugins/base/src/main/kotlin/translators/CollectionExtensions.kt
@@ -0,0 +1,12 @@
package org.jetbrains.dokka.base.translators

// TODO [beresnev] remove this copy-paste and use the same method from stdlib instead after updating to 1.5
internal inline fun <T, R : Any> Iterable<T>.firstNotNullOfOrNull(transform: (T) -> R?): R? {
for (element in this) {
val result = transform(element)
if (result != null) {
return result
}
}
return null
}
Expand Up @@ -385,8 +385,13 @@ private class DokkaDescriptorVisitor(
return coroutineScope {
val descriptorsWithKind = scope.getDescriptorsWithKind()

val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) }
val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) }
val (regularFunctions, accessors) = splitFunctionsAndAccessors(
properties = descriptorsWithKind.properties,
functions = descriptorsWithKind.functions
)

val functions = async { regularFunctions.visitFunctions(driWithPlatform) }
val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform, accessors) }
val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) }
val generics = async { descriptor.declaredTypeParameters.parallelMap { it.toVariantTypeParameter() } }
val constructors = async {
Expand Down Expand Up @@ -426,17 +431,49 @@ private class DokkaDescriptorVisitor(
}
}

/**
* @param implicitAccessors getters/setters that are not part of the property descriptor, for instance
* average methods inherited from java sources
*/
private suspend fun visitPropertyDescriptor(
originalDescriptor: PropertyDescriptor,
implicitAccessors: List<FunctionDescriptor>,
parent: DRIWithPlatformInfo
): DProperty {
val (dri, inheritedFrom) = originalDescriptor.createDRI()
val (dri, _) = originalDescriptor.createDRI()
val inheritedFrom = dri.getInheritedFromDRI(parent)
val descriptor = originalDescriptor.getConcreteDescriptor()
val isExpect = descriptor.isExpect
val isActual = descriptor.isActual

val actual = originalDescriptor.createSources()

// example - generated getter that comes with data classes
suspend fun getDescriptorGetter() =
descriptor.accessors
.firstIsInstanceOrNull<PropertyGetterDescriptor>()
?.let {
visitPropertyAccessorDescriptor(it, descriptor, dri, inheritedFrom)
}

suspend fun getImplicitAccessorGetter() =
implicitAccessors
.firstOrNull { it.isGetterFor(originalDescriptor) }
?.let { visitFunctionDescriptor(it, parent) }

// example - generated setter that comes with data classes
suspend fun getDescriptorSetter() =
descriptor.accessors
.firstIsInstanceOrNull<PropertySetterDescriptor>()
?.let {
visitPropertyAccessorDescriptor(it, descriptor, dri, inheritedFrom)
}

suspend fun getImplicitAccessorSetter() =
implicitAccessors
.firstOrNull { it.isSetterFor(originalDescriptor) }
?.let { visitFunctionDescriptor(it, parent) }

return coroutineScope {
val generics = async { descriptor.typeParameters.parallelMap { it.toVariantTypeParameter() } }

Expand All @@ -447,12 +484,8 @@ private class DokkaDescriptorVisitor(
visitReceiverParameterDescriptor(it, DRIWithPlatformInfo(dri, actual))
},
sources = actual,
getter = descriptor.accessors.filterIsInstance<PropertyGetterDescriptor>().singleOrNull()?.let {
visitPropertyAccessorDescriptor(it, descriptor, dri)
},
setter = descriptor.accessors.filterIsInstance<PropertySetterDescriptor>().singleOrNull()?.let {
visitPropertyAccessorDescriptor(it, descriptor, dri)
},
getter = getDescriptorGetter() ?: getImplicitAccessorGetter(),
setter = getDescriptorSetter() ?: getImplicitAccessorSetter(),
visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(),
documentation = descriptor.resolveDescriptorData(),
modifier = descriptor.modifier().toSourceSetDependent(),
Expand All @@ -468,7 +501,7 @@ private class DokkaDescriptorVisitor(
(descriptor.getAnnotationsWithBackingField() + descriptor.fileLevelAnnotations()).toSourceSetDependent()
.toAnnotations(),
descriptor.getDefaultValue()?.let { DefaultValue(it) },
InheritedMember(inheritedFrom.toSourceSetDependent()),
inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) },
)
)
)
Expand All @@ -485,7 +518,8 @@ private class DokkaDescriptorVisitor(
originalDescriptor: FunctionDescriptor,
parent: DRIWithPlatformInfo
): DFunction {
val (dri, inheritedFrom) = originalDescriptor.createDRI()
val (dri, _) = originalDescriptor.createDRI()
val inheritedFrom = dri.getInheritedFromDRI(parent)
val descriptor = originalDescriptor.getConcreteDescriptor()
val isExpect = descriptor.isExpect
val isActual = descriptor.isActual
Expand Down Expand Up @@ -515,7 +549,7 @@ private class DokkaDescriptorVisitor(
sourceSets = setOf(sourceSet),
isExpectActual = (isExpect || isActual),
extra = PropertyContainer.withAll(
InheritedMember(inheritedFrom.toSourceSetDependent()),
inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) },
descriptor.additionalExtras().toSourceSetDependent().toAdditionalModifiers(),
(descriptor.getAnnotations() + descriptor.fileLevelAnnotations()).toSourceSetDependent()
.toAnnotations(),
Expand All @@ -525,6 +559,19 @@ private class DokkaDescriptorVisitor(
}
}

/**
* `createDRI` returns the DRI of the exact element and potential DRI of an element that is overriding it
* (It can be also FAKE_OVERRIDE which is in fact just inheritance of the symbol)
*
* Looking at what PSIs do, they give the DRI of the element within the classnames where it is actually
* declared and inheritedFrom as the same DRI but truncated callable part.
* Therefore, we set callable to null and take the DRI only if it is indeed coming from different class.
*/
private fun DRI.getInheritedFromDRI(parent: DRIWithPlatformInfo): DRI? {
return this.copy(callable = null)
.takeIf { parent.dri.classNames != this.classNames || parent.dri.packageName != this.packageName }
}

suspend fun visitConstructorDescriptor(descriptor: ConstructorDescriptor, parent: DRIWithPlatformInfo): DFunction {
val name = descriptor.constructedClass.name.toString()
val dri = parent.dri.copy(callable = Callable.from(descriptor, name))
Expand Down Expand Up @@ -594,7 +641,8 @@ private class DokkaDescriptorVisitor(
private suspend fun visitPropertyAccessorDescriptor(
descriptor: PropertyAccessorDescriptor,
propertyDescriptor: PropertyDescriptor,
parent: DRI
parent: DRI,
inheritedFrom: DRI? = null
): DFunction {
val dri = parent.copy(callable = Callable.from(descriptor))
val isGetter = descriptor is PropertyGetterDescriptor
Expand Down Expand Up @@ -646,14 +694,13 @@ private class DokkaDescriptorVisitor(

return coroutineScope {
val generics = async { descriptor.typeParameters.parallelMap { it.toVariantTypeParameter() } }

DFunction(
dri,
name,
isConstructor = false,
parameters = parameters,
visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(),
documentation = descriptor.resolveDescriptorData(),
documentation = descriptor.resolveDescriptorData().mapInheritedTagWrappers(),
type = descriptor.returnType!!.toBound(),
generics = generics.await(),
modifier = descriptor.modifier().toSourceSetDependent(),
Expand All @@ -669,12 +716,32 @@ private class DokkaDescriptorVisitor(
isExpectActual = (isExpect || isActual),
extra = PropertyContainer.withAll(
descriptor.additionalExtras().toSourceSetDependent().toAdditionalModifiers(),
descriptor.getAnnotations().toSourceSetDependent().toAnnotations()
descriptor.getAnnotations().toSourceSetDependent().toAnnotations(),
inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) }
)
)
}
}

/**
* Workaround for a problem with inheriting parent TagWrappers of the wrong type.
*
* For instance, if you annotate a class with `@property`, kotlin compiler will propagate
* this tag to the property and its getters and setters. In case of getters and setters,
* it's more correct to display propagated docs as description instead of property
*/
private fun SourceSetDependent<DocumentationNode>.mapInheritedTagWrappers(): SourceSetDependent<DocumentationNode> {
return this.mapValues { (_, value) ->
val mappedChildren = value.children.map {
when (it) {
is Property -> Description(it.root)
else -> it
}
}
value.copy(children = mappedChildren)
}
}

private suspend fun visitTypeAliasDescriptor(descriptor: TypeAliasDescriptor, parent: DRIWithPlatformInfo?) =
with(descriptor) {
coroutineScope {
Expand Down Expand Up @@ -754,8 +821,17 @@ private class DokkaDescriptorVisitor(
private suspend fun List<FunctionDescriptor>.visitFunctions(parent: DRIWithPlatformInfo): List<DFunction> =
coroutineScope { parallelMap { visitFunctionDescriptor(it, parent) } }

private suspend fun List<PropertyDescriptor>.visitProperties(parent: DRIWithPlatformInfo): List<DProperty> =
coroutineScope { parallelMap { visitPropertyDescriptor(it, parent) } }
private suspend fun List<PropertyDescriptor>.visitProperties(
parent: DRIWithPlatformInfo,
implicitAccessors: Map<PropertyDescriptor, List<FunctionDescriptor>> = emptyMap(),
): List<DProperty> {
return coroutineScope {
parallelMap {
val propertyAccessors = implicitAccessors[it] ?: emptyList()
visitPropertyDescriptor(it, propertyAccessors, parent)
}
}
}

private suspend fun List<ClassDescriptor>.visitClasslikes(parent: DRIWithPlatformInfo): List<DClasslike> =
coroutineScope { parallelMap { visitClassDescriptor(it, parent) } }
Expand Down Expand Up @@ -904,11 +980,14 @@ private class DokkaDescriptorVisitor(
)
} ?: getJavaDocs())?.takeIf { it.children.isNotEmpty() }

private fun DeclarationDescriptor.getJavaDocs() = (this as? CallableDescriptor)
?.overriddenDescriptors
?.mapNotNull { it.findPsi() as? PsiNamedElement }
?.firstOrNull()
?.let { javadocParser.parseDocumentation(it) }
private fun DeclarationDescriptor.getJavaDocs(): DocumentationNode? {
val overriddenDescriptors = (this as? CallableDescriptor)?.overriddenDescriptors ?: emptyList()
val allDescriptors = overriddenDescriptors + listOf(this)
return allDescriptors
.mapNotNull { it.findPsi() as? PsiNamedElement }
.firstOrNull()
?.let { javadocParser.parseDocumentation(it) }
}

private suspend fun ClassDescriptor.companion(dri: DRIWithPlatformInfo): DObject? = companionObjectDescriptor?.let {
objectDescriptor(it, dri)
Expand Down
@@ -0,0 +1,83 @@
package org.jetbrains.dokka.base.translators.descriptors

import org.jetbrains.dokka.base.translators.firstNotNullOfOrNull
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.load.java.JvmAbi
import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor
import org.jetbrains.kotlin.load.java.propertyNameByGetMethodName
import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName

internal data class DescriptorFunctionsHolder(
val regularFunctions: List<FunctionDescriptor>,
val accessors: Map<PropertyDescriptor, List<FunctionDescriptor>>
)

internal fun splitFunctionsAndAccessors(
properties: List<PropertyDescriptor>,
functions: List<FunctionDescriptor>
): DescriptorFunctionsHolder {
val propertiesByName = properties.associateBy { it.name.asString() }
val regularFunctions = mutableListOf<FunctionDescriptor>()
val accessors = mutableMapOf<PropertyDescriptor, MutableList<FunctionDescriptor>>()
functions.forEach { function ->
val possiblePropertyNamesForFunction = function.toPossiblePropertyNames()
val property = possiblePropertyNamesForFunction.firstNotNullOfOrNull { propertiesByName[it] }
if (property != null && function.isAccessorFor(property)) {
accessors.getOrPut(property, ::mutableListOf).add(function)
} else {
regularFunctions.add(function)
}
}
return DescriptorFunctionsHolder(regularFunctions, accessors)
}

internal fun FunctionDescriptor.toPossiblePropertyNames(): List<String> {
val stringName = this.name.asString()
return when {
JvmAbi.isSetterName(stringName) -> propertyNamesBySetMethodName(this.name).map { it.asString() }
JvmAbi.isGetterName(stringName) -> propertyNamesByGetMethod(this)
else -> listOf()
}
}

internal 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;`
// - `private boolean isActive();`
//
// Whereas in Kotlin, because there are no explicit accessors, the convention is
// - `val isActive: Boolean`
//
// This makes it difficult to guess the name of the accessor property in case of Java
val javaPropName = if (functionDescriptor is JavaMethodDescriptor && JvmAbi.startsWithIsPrefix(stringName)) {
val javaPropName = stringName.removePrefix("is").let { newName ->
newName.replaceFirst(newName[0], newName[0].toLowerCase())
}
javaPropName
} else {
null
}
val kotlinPropName = propertyNameByGetMethodName(functionDescriptor.name)?.asString()
return listOfNotNull(javaPropName, kotlinPropName)
}

internal fun FunctionDescriptor.isAccessorFor(property: PropertyDescriptor): Boolean {
return this.isGetterFor(property) || this.isSetterFor(property)
}

internal 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 {
return this.valueParameters.size == 1
&& this.valueParameters[0].type == property.returnType
&& !property.visibility.isPublicAPI
&& this.visibility.isPublicAPI
}

0 comments on commit c307a60

Please sign in to comment.