-
Notifications
You must be signed in to change notification settings - Fork 409
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
IgnatBeresnev
merged 16 commits into
master
from
fix-gathering-inherited-properties-rebased
May 31, 2022
Merged
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
a296d85
Fix gathering inherited properties in PSI
BarkingBad 77d4b9f
Refacotr KaJ transformer. Change wrapping TagWrapper for getters and …
BarkingBad 3836153
Add logic to merge inherited properties in kotlin from java sources.
BarkingBad 1d9353c
Remove getters and setters from JvmField properties for DObject, DEnu…
BarkingBad c30b625
Unify InheritedMember DRI logic.
BarkingBad 13f730f
Fix gathering docs obtained from inheriting java sources in descriptors
BarkingBad 5ad83b3
Apply requested changes.
BarkingBad 59eed77
Resolve rebase conflicts
IgnatBeresnev dc15d19
Use 221 for qodana analysis
IgnatBeresnev 3ddbc8b
Move accessors generation into DefaultDescriptorToDocumentableTranslator
IgnatBeresnev 9077369
Fix special "is" case for accessors and refactor logic in general
IgnatBeresnev c8ac79b
Remove ambiguous import after rebasing
IgnatBeresnev 1c33cf3
Remove unused imports and format code
IgnatBeresnev dcb6657
Apply review comment suggestions
IgnatBeresnev de38d81
Preserve previously lost accessor lookalikes
IgnatBeresnev d236974
Extract a variable and correct a typo
IgnatBeresnev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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
12
plugins/base/src/main/kotlin/translators/CollectionExtensions.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} |
165 changes: 126 additions & 39 deletions
165
...base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt
Large diffs are not rendered by default.
Oops, something went wrong.
79 changes: 79 additions & 0 deletions
79
plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
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 | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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