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>
  • Loading branch information
IgnatBeresnev and BarkingBad committed May 31, 2022
1 parent 8913c97 commit 623cf22
Show file tree
Hide file tree
Showing 14 changed files with 956 additions and 211 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/qodana.yml
Expand Up @@ -16,7 +16,7 @@ jobs:
id: qodana
uses: JetBrains/qodana-action@v5.1.0
with:
args: --baseline,qodana.sarif.json,--fail-threshold,0,--linter,jetbrains/qodana-jvm
args: --baseline,qodana.sarif.json,--fail-threshold,0,--linter,jetbrains/qodana-jvm:2022.1-eap
clone-finder:
runs-on: ubuntu-latest
steps:
Expand Down
7 changes: 7 additions & 0 deletions core/api/core.api
Expand Up @@ -1820,6 +1820,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
}

Large diffs are not rendered by default.

@@ -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 623cf22

Please sign in to comment.