Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix gathering inherited properties #2481

Merged
merged 16 commits into from May 31, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default it uses 213 I think, which fails with some obscure problem (have a look at our discussion in the qodana channel).

I think this can stay until we update qodana and it uses 221+ by default. It's close to the version, so I'll remember to change it

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,80 @@
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(
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
properties: List<PropertyDescriptor>,
functions: List<FunctionDescriptor>
): DescriptorFunctionsHolder {
val fieldsByName = properties.associateBy { it.name.asString() }
val regularFunctions = mutableListOf<FunctionDescriptor>()
val accessors = mutableMapOf<PropertyDescriptor, MutableList<FunctionDescriptor>>()
functions.forEach { function ->
val possiblePropertyNamesForFunction = function.toPossiblePropertyNames()
val field = possiblePropertyNamesForFunction.firstNotNullOfOrNull { fieldsByName[it] }
if (field != null) {
accessors.getOrPut(field, ::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()
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
return listOfNotNull(javaPropName, kotlinPropName)
}

internal fun FunctionDescriptor.isGetterFor(property: PropertyDescriptor): Boolean {
return this.returnType == property.returnType
&& this.valueParameters.isEmpty()
&& !property.visibility.isPublicAPI
vmishenev marked this conversation as resolved.
Show resolved Hide resolved
&& 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
}