From 623cf222ca2ac5e8b9628af5927956ecb6d44d1e Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Tue, 31 May 2022 15:34:37 +0200 Subject: Fix gathering inherited properties (#2481) * 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 --- .../kotlin/translators/CollectionExtensions.kt | 12 + .../DefaultDescriptorToDocumentableTranslator.kt | 163 +++++++-- .../DescriptorAccessorConventionUtil.kt | 83 +++++ .../psi/DefaultPsiToDocumentableTranslator.kt | 406 ++++++++++++--------- .../translators/psi/PsiAccessorConventionUtil.kt | 58 +++ plugins/base/src/test/kotlin/model/PropertyTest.kt | 7 + .../superFields/DescriptorSuperPropertiesTest.kt | 191 ++++++++++ .../test/kotlin/superFields/PsiSuperFieldsTest.kt | 186 ++++++++++ 8 files changed, 903 insertions(+), 203 deletions(-) create mode 100644 plugins/base/src/main/kotlin/translators/CollectionExtensions.kt create mode 100644 plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt create mode 100644 plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt create mode 100644 plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt create mode 100644 plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt (limited to 'plugins/base/src') diff --git a/plugins/base/src/main/kotlin/translators/CollectionExtensions.kt b/plugins/base/src/main/kotlin/translators/CollectionExtensions.kt new file mode 100644 index 00000000..0de4b5b1 --- /dev/null +++ b/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 Iterable.firstNotNullOfOrNull(transform: (T) -> R?): R? { + for (element in this) { + val result = transform(element) + if (result != null) { + return result + } + } + return null +} diff --git a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt index fa0a5b48..6c4d7a0d 100644 --- a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt +++ b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt @@ -150,8 +150,8 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind(true) - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) } + val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) } val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) } val typealiases = async { descriptorsWithKind.typealiases.visitTypealiases() } @@ -186,8 +186,8 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) } + val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) } val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) } val generics = async { descriptor.declaredTypeParameters.parallelMap { it.toVariantTypeParameter() } } @@ -227,8 +227,8 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) } + val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) } val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) } DObject( @@ -266,8 +266,8 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) } + val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) } val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) } val constructors = async { descriptor.constructors.parallelMap { visitConstructorDescriptor(it, driWithPlatform) } } @@ -306,8 +306,8 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) } + val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) } val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) } DEnumEntry( @@ -336,8 +336,8 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + val functions = async { descriptorsWithKind.functions.visitFunctions(driWithPlatform) } + val properties = async { descriptorsWithKind.properties.visitProperties(driWithPlatform) } val classlikes = async { descriptorsWithKind.classlikes.visitClasslikes(driWithPlatform) } val generics = async { descriptor.declaredTypeParameters.parallelMap { it.toVariantTypeParameter() } } val constructors = @@ -379,8 +379,13 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val functions = async { descriptorsWithKind.functions.visitFunctions() } - val properties = async { descriptorsWithKind.properties.visitProperties() } + 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 { @@ -420,14 +425,49 @@ private class DokkaDescriptorVisitor( } } - private suspend fun visitPropertyDescriptor(originalDescriptor: PropertyDescriptor): DProperty { - val (dri, inheritedFrom) = originalDescriptor.createDRI() + /** + * @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, + parent: DRIWithPlatformInfo + ): DProperty { + 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() + ?.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() + ?.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() } } @@ -438,12 +478,8 @@ private class DokkaDescriptorVisitor( visitReceiverParameterDescriptor(it, DRIWithPlatformInfo(dri, actual)) }, sources = actual, - getter = descriptor.accessors.filterIsInstance().singleOrNull()?.let { - visitPropertyAccessorDescriptor(it, descriptor, dri) - }, - setter = descriptor.accessors.filterIsInstance().singleOrNull()?.let { - visitPropertyAccessorDescriptor(it, descriptor, dri) - }, + getter = getDescriptorGetter() ?: getImplicitAccessorGetter(), + setter = getDescriptorSetter() ?: getImplicitAccessorSetter(), visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(), documentation = descriptor.resolveDescriptorData(), modifier = descriptor.modifier().toSourceSetDependent(), @@ -459,7 +495,7 @@ private class DokkaDescriptorVisitor( (descriptor.getAnnotationsWithBackingField() + descriptor.fileLevelAnnotations()).toSourceSetDependent() .toAnnotations(), descriptor.getDefaultValue()?.let { DefaultValue(it.toSourceSetDependent()) }, - InheritedMember(inheritedFrom.toSourceSetDependent()), + inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) }, ) ) ) @@ -472,8 +508,12 @@ private class DokkaDescriptorVisitor( else overriddenDescriptors.first().createDRI(DRI.from(this)) - private suspend fun visitFunctionDescriptor(originalDescriptor: FunctionDescriptor): DFunction { - val (dri, inheritedFrom) = originalDescriptor.createDRI() + private suspend fun visitFunctionDescriptor( + originalDescriptor: FunctionDescriptor, + parent: DRIWithPlatformInfo + ): DFunction { + val (dri, _) = originalDescriptor.createDRI() + val inheritedFrom = dri.getInheritedFromDRI(parent) val descriptor = originalDescriptor.getConcreteDescriptor() val isExpect = descriptor.isExpect val isActual = descriptor.isActual @@ -503,7 +543,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(), @@ -513,6 +553,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)) @@ -582,7 +635,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 @@ -634,14 +688,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(), @@ -657,12 +710,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.mapInheritedTagWrappers(): SourceSetDependent { + 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) = with(descriptor) { coroutineScope { @@ -740,11 +813,20 @@ private class DokkaDescriptorVisitor( ) } - private suspend fun List.visitFunctions(): List = - coroutineScope { parallelMap { visitFunctionDescriptor(it) } } + private suspend fun List.visitFunctions(parent: DRIWithPlatformInfo): List = + coroutineScope { parallelMap { visitFunctionDescriptor(it, parent) } } - private suspend fun List.visitProperties(): List = - coroutineScope { parallelMap { visitPropertyDescriptor(it) } } + private suspend fun List.visitProperties( + parent: DRIWithPlatformInfo, + implicitAccessors: Map> = emptyMap(), + ): List { + return coroutineScope { + parallelMap { + val propertyAccessors = implicitAccessors[it] ?: emptyList() + visitPropertyDescriptor(it, propertyAccessors, parent) + } + } + } private suspend fun List.visitClasslikes(parent: DRIWithPlatformInfo): List = coroutineScope { parallelMap { visitClassDescriptor(it, parent) } } @@ -896,11 +978,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) diff --git a/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt b/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt new file mode 100644 index 00000000..f182b9be --- /dev/null +++ b/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt @@ -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, + val accessors: Map> +) + +internal fun splitFunctionsAndAccessors( + properties: List, + functions: List +): DescriptorFunctionsHolder { + val propertiesByName = properties.associateBy { it.name.asString() } + val regularFunctions = mutableListOf() + val accessors = mutableMapOf>() + 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 { + 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 { + 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 +} + diff --git a/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt index f9199154..5b30543e 100644 --- a/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt +++ b/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt @@ -25,7 +25,6 @@ import org.jetbrains.dokka.links.* import org.jetbrains.dokka.model.* import org.jetbrains.dokka.model.AnnotationTarget import org.jetbrains.dokka.model.Nullable -import org.jetbrains.dokka.model.TypeConstructor import org.jetbrains.dokka.model.doc.DocumentationNode import org.jetbrains.dokka.model.doc.Param import org.jetbrains.dokka.model.properties.PropertyContainer @@ -87,14 +86,14 @@ class DefaultPsiToDocumentableTranslator( ) DModule( - context.configuration.moduleName, - psiFiles.parallelMapNotNull { it.safeAs() }.groupBy { it.packageName }.toList() + name = context.configuration.moduleName, + packages = psiFiles.parallelMapNotNull { it.safeAs() }.groupBy { it.packageName }.toList() .parallelMap { (packageName: String, psiFiles: List) -> docParser.parsePackage(packageName, psiFiles) }, - emptyMap(), - null, - setOf(sourceSet) + documentation = emptyMap(), + expectPresentInSet = null, + sourceSets = setOf(sourceSet) ) } } @@ -122,6 +121,9 @@ class DefaultPsiToDocumentableTranslator( private val PsiMethod.hash: Int get() = "$returnType $name$parameterList".hashCode() + private val PsiField.hash: Int + get() = "$type $name".hashCode() + private val PsiClassType.shouldBeIgnored: Boolean get() = isClass("java.lang.Enum") || isClass("java.lang.Object") @@ -148,19 +150,19 @@ class DefaultPsiToDocumentableTranslator( val annotations = packageInfo?.packageStatement?.annotationList?.annotations DPackage( - dri, - emptyList(), - emptyList(), - psiFiles.parallelMap { psiFile -> + dri = dri, + functions = emptyList(), + properties = emptyList(), + classlikes = psiFiles.parallelMap { psiFile -> coroutineScope { psiFile.classes.asIterable().parallelMap { parseClasslike(it, dri) } } }.flatten(), - emptyList(), - documentation, - null, - setOf(sourceSetData), - PropertyContainer.withAll( + typealiases = emptyList(), + documentation = documentation, + expectPresentInSet = null, + sourceSets = setOf(sourceSetData), + extra = PropertyContainer.withAll( annotations?.toList().orEmpty().toListOfAnnotations().toSourceSetDependent().toAnnotations() ) ) @@ -171,10 +173,16 @@ class DefaultPsiToDocumentableTranslator( val dri = parent.withClass(name.toString()) val superMethodsKeys = hashSetOf() val superMethods = mutableListOf>() + val superFieldsKeys = hashSetOf() + val superFields = mutableListOf>() methods.asIterable().parallelForEach { superMethodsKeys.add(it.hash) } /** - * Caution! This method mutates superMethodsKeys and superMethods + * Caution! This method mutates + * - superMethodsKeys + * - superMethods + * - superFieldsKeys + * - superKeys */ fun Array.getSuperTypesPsiClasses(): List> { forEach { type -> @@ -189,6 +197,13 @@ class DefaultPsiToDocumentableTranslator( superMethods.add(Pair(method, definedAt)) } } + it.fields.forEach { field -> + val hash = field.hash + if (!superFieldsKeys.contains(hash)) { + superFieldsKeys.add(hash) + superFields.add(Pair(field, definedAt)) + } + } } } return filter { !it.shouldBeIgnored }.mapNotNull { supertypePsi -> @@ -223,24 +238,60 @@ class DefaultPsiToDocumentableTranslator( } val ancestry: AncestryNode = traversePsiClassForAncestorsAndInheritedMembers(this) - val (regularFunctions, accessors) = splitFunctionsAndAccessors() + + val (regularFunctions, accessors) = splitFunctionsAndAccessors(psi.fields, psi.methods) + val (regularSuperFunctions, superAccessors) = splitFunctionsAndAccessors( + fields = superFields.map { it.first }.toTypedArray(), + methods = superMethods.map { it.first }.toTypedArray() + ) + + val regularSuperFunctionsKeys = regularSuperFunctions.map { it.hash }.toSet() + val regularSuperFunctionsWithDRI = superMethods.filter { it.first.hash in regularSuperFunctionsKeys } + + val superAccessorsWithDRI = superAccessors.mapValues { (field, methods) -> + val containsJvmField = field.annotations.mapNotNull { it.toAnnotation() }.any { it.isJvmField() } + if (containsJvmField) { + emptyList() + } else { + methods.mapNotNull { method -> superMethods.find { it.first.hash == method.hash } } + } + } + val overridden = regularFunctions.flatMap { it.findSuperMethods().toList() } val documentation = javadocParser.parseDocumentation(this).toSourceSetDependent() val allFunctions = async { - regularFunctions.parallelMapNotNull { + val parsedRegularFunctions = regularFunctions.parallelMapNotNull { if (!it.isConstructor) parseFunction( it, parentDRI = dri ) else null - } + superMethods.filter { it.first !in overridden }.parallelMap { parseFunction(it.first, inheritedFrom = it.second) } + } + val parsedSuperFunctions = regularSuperFunctionsWithDRI + .filter { it.first !in overridden } + .parallelMap { parseFunction(it.first, inheritedFrom = it.second) } + + parsedRegularFunctions + parsedSuperFunctions + } + val allFields = async { + val parsedFields = fields.toList().parallelMapNotNull { + parseField(it, accessors[it].orEmpty()) + } + val parsedSuperFields = superFields.parallelMapNotNull { (field, dri) -> + parseFieldWithInheritingAccessors( + field, + superAccessorsWithDRI[field].orEmpty(), + inheritedFrom = dri + ) + } + parsedFields + parsedSuperFields } val source = PsiDocumentableSource(this).toSourceSetDependent() val classlikes = async { innerClasses.asIterable().parallelMap { parseClasslike(it, dri) } } val visibility = getVisibility().toSourceSetDependent() val ancestors = (listOfNotNull(ancestry.superclass?.let { - it.typeConstructor.let { + it.typeConstructor.let { typeConstructor -> TypeConstructorWithKind( - it, + typeConstructor, JavaClassKindTypes.CLASS ) } @@ -251,103 +302,103 @@ class DefaultPsiToDocumentableTranslator( when { isAnnotationType -> DAnnotation( - name.orEmpty(), - dri, - documentation, - null, - source, - allFunctions.await(), - fields.mapNotNull { parseField(it, accessors[it].orEmpty()) }, - classlikes.await(), - visibility, - null, - constructors.map { parseFunction(it, true) }, - mapTypeParameters(dri), - setOf(sourceSetData), - false, - PropertyContainer.withAll( + name = name.orEmpty(), + dri = dri, + documentation = documentation, + expectPresentInSet = null, + sources = source, + functions = allFunctions.await(), + properties = allFields.await(), + classlikes = classlikes.await(), + visibility = visibility, + companion = null, + constructors = constructors.map { parseFunction(it, true) }, + generics = mapTypeParameters(dri), + sourceSets = setOf(sourceSetData), + isExpectActual = false, + extra = PropertyContainer.withAll( implementedInterfacesExtra, annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations() ) ) isEnum -> DEnum( - dri, - name.orEmpty(), - fields.filterIsInstance().map { entry -> + dri = dri, + name = name.orEmpty(), + entries = fields.filterIsInstance().map { entry -> DEnumEntry( - dri.withClass(entry.name).withEnumEntryExtra(), - entry.name, - javadocParser.parseDocumentation(entry).toSourceSetDependent(), - null, - emptyList(), - emptyList(), - emptyList(), - setOf(sourceSetData), - PropertyContainer.withAll( + dri = dri.withClass(entry.name).withEnumEntryExtra(), + name = entry.name, + documentation = javadocParser.parseDocumentation(entry).toSourceSetDependent(), + expectPresentInSet = null, + functions = emptyList(), + properties = emptyList(), + classlikes = emptyList(), + sourceSets = setOf(sourceSetData), + extra = PropertyContainer.withAll( implementedInterfacesExtra, annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations() ) ) }, - documentation, - null, - source, - allFunctions.await(), - fields.filter { it !is PsiEnumConstant }.map { parseField(it, accessors[it].orEmpty()) }, - classlikes.await(), - visibility, - null, - constructors.map { parseFunction(it, true) }, - ancestors, - setOf(sourceSetData), - false, - PropertyContainer.withAll( + documentation = documentation, + expectPresentInSet = null, + sources = source, + functions = allFunctions.await(), + properties = fields.filter { it !is PsiEnumConstant }.map { parseField(it, accessors[it].orEmpty()) }, + classlikes = classlikes.await(), + visibility = visibility, + companion = null, + constructors = constructors.map { parseFunction(it, true) }, + supertypes = ancestors, + sourceSets = setOf(sourceSetData), + isExpectActual = false, + extra = PropertyContainer.withAll( implementedInterfacesExtra, annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations() ) ) isInterface -> DInterface( - dri, - name.orEmpty(), - documentation, - null, - source, - allFunctions.await(), - fields.mapNotNull { parseField(it, accessors[it].orEmpty()) }, - classlikes.await(), - visibility, - null, - mapTypeParameters(dri), - ancestors, - setOf(sourceSetData), - false, - PropertyContainer.withAll( + dri = dri, + name = name.orEmpty(), + documentation = documentation, + expectPresentInSet = null, + sources = source, + functions = allFunctions.await(), + properties = allFields.await(), + classlikes = classlikes.await(), + visibility = visibility, + companion = null, + generics = mapTypeParameters(dri), + supertypes = ancestors, + sourceSets = setOf(sourceSetData), + isExpectActual = false, + extra = PropertyContainer.withAll( implementedInterfacesExtra, annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations() ) ) else -> DClass( - dri, - name.orEmpty(), - constructors.map { parseFunction(it, true) }, - allFunctions.await(), - fields.mapNotNull { parseField(it, accessors[it].orEmpty()) }, - classlikes.await(), - source, - visibility, - null, - mapTypeParameters(dri), - ancestors, - documentation, - null, - modifiers, - setOf(sourceSetData), - false, - PropertyContainer.withAll( + dri = dri, + name = name.orEmpty(), + constructors = constructors.map { parseFunction(it, true) }, + functions = allFunctions.await(), + properties = allFields.await(), + classlikes = classlikes.await(), + sources = source, + visibility = visibility, + companion = null, + generics = mapTypeParameters(dri), + supertypes = ancestors, + documentation = documentation, + expectPresentInSet = null, + modifier = modifiers, + sourceSets = setOf(sourceSetData), + isExpectActual = false, + extra = PropertyContainer.withAll( implementedInterfacesExtra, annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations(), @@ -372,40 +423,40 @@ class DefaultPsiToDocumentableTranslator( } ?: DRI.from(psi) val docs = javadocParser.parseDocumentation(psi) return DFunction( - dri, - psi.name, - isConstructor, - psi.parameterList.parameters.map { psiParameter -> + dri = dri, + name = psi.name, + isConstructor = isConstructor, + parameters = psi.parameterList.parameters.map { psiParameter -> DParameter( - dri.copy(target = dri.target.nextTarget()), - psiParameter.name, - DocumentationNode( + dri = dri.copy(target = dri.target.nextTarget()), + name = psiParameter.name, + documentation = DocumentationNode( listOfNotNull(docs.firstChildOfTypeOrNull { it.name == psiParameter.name }) ).toSourceSetDependent(), - null, - getBound(psiParameter.type), - setOf(sourceSetData), - PropertyContainer.withAll( + expectPresentInSet = null, + type = getBound(psiParameter.type), + sourceSets = setOf(sourceSetData), + extra = PropertyContainer.withAll( psiParameter.annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations() ) ) }, - docs.toSourceSetDependent(), - null, - PsiDocumentableSource(psi).toSourceSetDependent(), - psi.getVisibility().toSourceSetDependent(), - psi.returnType?.let { getBound(type = it) } ?: Void, - psi.mapTypeParameters(dri), - null, - psi.getModifier().toSourceSetDependent(), - setOf(sourceSetData), - false, - psi.additionalExtras().let { + documentation = docs.toSourceSetDependent(), + expectPresentInSet = null, + sources = PsiDocumentableSource(psi).toSourceSetDependent(), + visibility = psi.getVisibility().toSourceSetDependent(), + type = psi.returnType?.let { getBound(type = it) } ?: Void, + generics = psi.mapTypeParameters(dri), + receiver = null, + modifier = psi.getModifier().toSourceSetDependent(), + sourceSets = setOf(sourceSetData), + isExpectActual = false, + extra = psi.additionalExtras().let { PropertyContainer.withAll( - InheritedMember(inheritedFrom.toSourceSetDependent()), + inheritedFrom?.let { InheritedMember(it.toSourceSetDependent()) }, it.toSourceSetDependent().toAdditionalModifiers(), (psi.annotations.toList() .toListOfAnnotations() + it.toListOfAnnotations()).toSourceSetDependent() @@ -437,6 +488,20 @@ class DefaultPsiToDocumentableTranslator( Annotations.Annotation(DRI("kotlin.jvm", "JvmStatic"), emptyMap()) } + /** + * Workaround for getting JvmField Kotlin annotation in PSIs + */ + private fun Collection.findJvmFieldAnnotation(): Annotations.Annotation? { + val anyJvmFieldAnnotation = this.any { + it.qualifiedName == "$JVM_FIELD_PACKAGE_NAME.$JVM_FIELD_CLASS_NAMES" + } + return if (anyJvmFieldAnnotation) { + Annotations.Annotation(DRI(JVM_FIELD_PACKAGE_NAME, JVM_FIELD_CLASS_NAMES), emptyMap()) + } else { + null + } + } + private fun PsiTypeParameter.annotations(): PropertyContainer = this.annotations.toList().toListOfAnnotations().annotations() private fun PsiType.annotations(): PropertyContainer = this.annotations.toList().toListOfAnnotations().annotations() @@ -521,14 +586,14 @@ class DefaultPsiToDocumentableTranslator( } return typeParameters.map { type -> DTypeParameter( - dri.copy(target = dri.target.nextTarget()), - type.name.orEmpty(), - null, - javadocParser.parseDocumentation(type).toSourceSetDependent(), - null, - mapBounds(type.bounds), - setOf(sourceSetData), - PropertyContainer.withAll( + dri = dri.copy(target = dri.target.nextTarget()), + name = type.name.orEmpty(), + presentableName = null, + documentation = javadocParser.parseDocumentation(type).toSourceSetDependent(), + expectPresentInSet = null, + bounds = mapBounds(type.bounds), + sourceSets = setOf(sourceSetData), + extra = PropertyContainer.withAll( type.annotations.toList().toListOfAnnotations().toSourceSetDependent() .toAnnotations() ) @@ -536,53 +601,66 @@ class DefaultPsiToDocumentableTranslator( } } - private fun PsiMethod.getPropertyNameForFunction() = - getAnnotation(DescriptorUtils.JVM_NAME.asString())?.findAttributeValue("name")?.text - ?: when { - JvmAbi.isGetterName(name) -> propertyNameByGetMethodName(Name.identifier(name))?.asString() - JvmAbi.isSetterName(name) -> propertyNamesBySetMethodName(Name.identifier(name)).firstOrNull() - ?.asString() - else -> null - } + private fun parseFieldWithInheritingAccessors( + psi: PsiField, + accessors: List>, + inheritedFrom: DRI + ): DProperty { + val getter = accessors + .firstOrNull { (method, _) -> method.isGetterFor(psi) } + ?.let { (method, dri) -> parseFunction(method, inheritedFrom = dri) } + + val setter = accessors + .firstOrNull { (method, _) -> method.isSetterFor(psi) } + ?.let { (method, dri) -> parseFunction(method, inheritedFrom = dri) } + + return parseField( + psi = psi, + getter = getter, + setter = setter, + inheritedFrom = inheritedFrom + ) + } - private fun PsiClass.splitFunctionsAndAccessors(): Pair, MutableMap>> { - val fieldNames = fields.associateBy { it.name } - val accessors = mutableMapOf>() - val regularMethods = mutableListOf() - methods.forEach { method -> - val field = method.getPropertyNameForFunction()?.let { name -> fieldNames[name] } - if (field != null) { - accessors.getOrPut(field, ::mutableListOf).add(method) - } else { - regularMethods.add(method) - } - } - return regularMethods to accessors + private fun parseField(psi: PsiField, accessors: List, inheritedFrom: DRI? = null): DProperty { + val getter = accessors.firstOrNull { it.isGetterFor(psi) }?.let { parseFunction(it) } + val setter = accessors.firstOrNull { it.isSetterFor(psi) }?.let { parseFunction(it) } + return parseField( + psi = psi, + getter = getter, + setter = setter, + inheritedFrom = inheritedFrom + ) } - private fun parseField(psi: PsiField, accessors: List): DProperty { + private fun parseField(psi: PsiField, getter: DFunction?, setter: DFunction?, inheritedFrom: DRI? = null): DProperty { val dri = DRI.from(psi) return DProperty( - dri, - psi.name, - javadocParser.parseDocumentation(psi).toSourceSetDependent(), - null, - PsiDocumentableSource(psi).toSourceSetDependent(), - psi.getVisibility().toSourceSetDependent(), - getBound(psi.type), - null, - accessors.firstOrNull { it.hasParameters() }?.let { parseFunction(it) }, - accessors.firstOrNull { it.returnType == psi.type }?.let { parseFunction(it) }, - psi.getModifier().toSourceSetDependent(), - setOf(sourceSetData), - emptyList(), - false, - psi.additionalExtras().let { + dri = dri, + name = psi.name, + documentation = javadocParser.parseDocumentation(psi).toSourceSetDependent(), + expectPresentInSet = null, + sources = PsiDocumentableSource(psi).toSourceSetDependent(), + visibility = psi.getVisibility().toSourceSetDependent(), + type = getBound(psi.type), + receiver = null, + setter = setter, + getter = getter, + modifier = psi.getModifier().toSourceSetDependent(), + sourceSets = setOf(sourceSetData), + generics = emptyList(), + isExpectActual = false, + extra = psi.additionalExtras().let { + val psiAnnotations = psi.annotations.toList() + val parsedAnnotations = psiAnnotations.toListOfAnnotations() + val extraModifierAnnotations = it.toListOfAnnotations() + val jvmFieldAnnotation = psiAnnotations.findJvmFieldAnnotation() + val annotations = parsedAnnotations + extraModifierAnnotations + listOfNotNull(jvmFieldAnnotation) + PropertyContainer.withAll( + inheritedFrom?.let { inheritedFrom -> InheritedMember(inheritedFrom.toSourceSetDependent()) }, it.toSourceSetDependent().toAdditionalModifiers(), - (psi.annotations.toList() - .toListOfAnnotations() + it.toListOfAnnotations()).toSourceSetDependent() - .toAnnotations() + annotations.toSourceSetDependent().toAnnotations() ) } ) diff --git a/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt new file mode 100644 index 00000000..c2ab8c03 --- /dev/null +++ b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt @@ -0,0 +1,58 @@ +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.kotlin.load.java.JvmAbi +import org.jetbrains.kotlin.load.java.propertyNameByGetMethodName +import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.resolve.DescriptorUtils + + +internal data class PsiFunctionsHolder( + val regularFunctions: List, + val accessors: Map> +) + +internal fun splitFunctionsAndAccessors(fields: Array, methods: Array): PsiFunctionsHolder { + val fieldsByName = fields.associateBy { it.name } + val regularFunctions = mutableListOf() + val accessors = mutableMapOf>() + methods.forEach { method -> + val possiblePropertyNamesForFunction = method.getPossiblePropertyNamesForFunction() + val field = possiblePropertyNamesForFunction.firstNotNullOfOrNull { fieldsByName[it] } + if (field != null && method.isAccessorFor(field)) { + accessors.getOrPut(field, ::mutableListOf).add(method) + } else { + regularFunctions.add(method) + } + } + return PsiFunctionsHolder(regularFunctions, accessors) +} + +internal fun PsiMethod.getPossiblePropertyNamesForFunction(): List { + val jvmName = getAnnotation(DescriptorUtils.JVM_NAME.asString())?.findAttributeValue("name")?.text + return jvmName?.let { listOf(jvmName) } + ?: when { + JvmAbi.isGetterName(name) -> listOfNotNull( + propertyNameByGetMethodName(Name.identifier(name))?.asString() + ) + JvmAbi.isSetterName(name) -> { + propertyNamesBySetMethodName(Name.identifier(name)).map { it.asString() } + } + else -> listOf() + } +} + +internal fun PsiMethod.isAccessorFor(field: PsiField): Boolean { + return this.isGetterFor(field) || this.isSetterFor(field) +} + +internal fun PsiMethod.isGetterFor(field: PsiField): Boolean { + return this.returnType == field.type && !this.hasParameters() +} + +internal fun PsiMethod.isSetterFor(field: PsiField): Boolean { + return parameterList.getParameter(0)?.type == field.type +} diff --git a/plugins/base/src/test/kotlin/model/PropertyTest.kt b/plugins/base/src/test/kotlin/model/PropertyTest.kt index 17f526f3..dc35d621 100644 --- a/plugins/base/src/test/kotlin/model/PropertyTest.kt +++ b/plugins/base/src/test/kotlin/model/PropertyTest.kt @@ -1,10 +1,13 @@ package model +import org.jetbrains.dokka.links.Callable +import org.jetbrains.dokka.links.DRI import org.jetbrains.dokka.model.* import org.junit.jupiter.api.Test import utils.AbstractModelTest import utils.assertNotNull import utils.name +import kotlin.test.assertEquals class PropertyTest : AbstractModelTest("/src/main/kotlin/property/Test.kt", "property") { @@ -158,6 +161,10 @@ class PropertyTest : AbstractModelTest("/src/main/kotlin/property/Test.kt", "pro with(getter.assertNotNull("Getter")) { type.name equals "Int" } + extra[InheritedMember]?.inheritedFrom?.values?.single()?.run { + classNames equals "Foo" + callable equals null + } } } } diff --git a/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt new file mode 100644 index 00000000..a6dd4350 --- /dev/null +++ b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt @@ -0,0 +1,191 @@ +package superFields + +import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest +import org.jetbrains.dokka.links.DRI +import org.jetbrains.dokka.model.InheritedMember +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class DescriptorSuperPropertiesTest : BaseAbstractTest() { + + private val commonTestConfiguration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + analysisPlatform = "jvm" + name = "jvm" + } + } + } + + @Test + fun `kotlin inheriting java should append only getter`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | private int a = 1; + | public int getA() { return a; } + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + commonTestConfiguration + ) { + this.documentablesTransformationStage = { module -> + val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + + val property = kotlinProperties.single { it.name == "a" } + val propertyInheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), propertyInheritedFrom) + + assertNull(property.setter) + assertNotNull(property.getter) + + val getterInheritedFrom = property.getter?.extra?.get(InheritedMember)?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), getterInheritedFrom) + } + } + } + + @Test + fun `kotlin inheriting java should append getter and setter`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | private int a = 1; + | public int getA() { return a; } + | public void setA(int a) { this.a = a; } + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val property = kotlinProperties.single { it.name == "a" } + property.extra[InheritedMember]?.inheritedFrom?.values?.single()?.run { + assertEquals( + DRI(packageName = "test", classNames = "A"), + this + ) + } + + val getter = property.getter + assertNotNull(getter) + assertEquals("getA", getter.name) + val getterInheritedFrom = getter.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), getterInheritedFrom) + + val setter = property.setter + assertNotNull(setter) + assertEquals("setA", setter.name) + val setterInheritedFrom = setter.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), setterInheritedFrom) + } + } + } + + @Test + fun `should have special getter and setter names for boolean property inherited from java`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | private boolean bool = true; + | public boolean isBool() { return bool; } + | public void setBool(boolean bool) { this.bool = bool; } + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val boolProperty = kotlinProperties.single { it.name == "bool" } + + val getter = boolProperty.getter + assertNotNull(getter) + assertEquals("isBool", getter.name) + + val setter = boolProperty.setter + assertNotNull(setter) + assertEquals("setBool", setter.name) + } + } + } + + @Test + fun `kotlin inheriting java should not append anything since field is public`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | public int a = 1; + | public int getA() { return a; } + | public void setA(int a) { this.a = a; } + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val property = kotlinProperties.single { it.name == "a" } + + assertNull(property.getter) + assertNull(property.setter) + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } + } + + @Test + fun `should preserve regular functions that look like accessors, but are not accessors`() { + testInline( + """ + |/src/test/A.kt + |package test + |class A { + | val v = 0 + | fun setV() { println(10) } // no arg + | fun getV(): String { return "s" } // wrong return type + |} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val testClass = module.packages.single().classlikes.single { it.name == "A" } + val setterLookalike = testClass.functions.firstOrNull { it.name == "setV" } + assertNotNull(setterLookalike) { + "Expected regular function not found, wrongly categorized as setter?" + } + + val getterLookalike = testClass.functions.firstOrNull { it.name == "getV" } + assertNotNull(getterLookalike) { + "Expected regular function not found, wrongly categorized as getter?" + } + } + } + } +} diff --git a/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt b/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt new file mode 100644 index 00000000..025c9b06 --- /dev/null +++ b/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt @@ -0,0 +1,186 @@ +package superFields + +import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest +import org.jetbrains.dokka.links.DRI +import org.jetbrains.dokka.model.Annotations +import org.jetbrains.dokka.model.InheritedMember +import org.jetbrains.dokka.model.isJvmField +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals + + +class PsiSuperFieldsTest : BaseAbstractTest() { + + private val commonTestConfiguration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + analysisPlatform = "jvm" + name = "jvm" + } + } + } + + @Test + fun `java inheriting java`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | public int a = 1; + |} + | + |/src/test/B.java + |package test; + |public class B extends A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val property = inheritorProperties.single { it.name == "a" } + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } + } + + @Test + fun `java inheriting kotlin common case`() { + testInline( + """ + |/src/test/A.kt + |package test + |open class A { + | var a: Int = 1 + |} + | + |/src/test/B.java + |package test; + |public class B extends A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val property = inheritorProperties.single { it.name == "a" } + + assertNotNull(property.getter) + assertNotNull(property.setter) + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } + } + + @Test + fun `java inheriting kotlin with boolean property`() { + testInline( + """ + |/src/test/A.kt + |package test + |open class A { + | var isActive: Boolean = true + |} + | + |/src/test/B.java + |package test; + |public class B extends A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val property = inheritorProperties.single { it.name == "isActive" } + + assertNotNull(property.getter) + assertEquals("isActive", property.getter?.name) + + assertNotNull(property.setter) + assertEquals("setActive", property.setter?.name) + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } + } + + @Test + fun `java inheriting kotlin with @JvmField should not inherit accessors`() { + testInline( + """ + |/src/test/A.kt + |package test + |open class A { + | @kotlin.jvm.JvmField + | var a: Int = 1 + |} + | + |/src/test/B.java + |package test; + |public class B extends A {} + """.trimIndent(), + dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + analysisPlatform = "jvm" + name = "jvm" + classpath += jvmStdlibPath!! // needed for JvmField + } + } + } + ) { + documentablesMergingStage = { module -> + val inheritorProperties = module.packages.single().classlikes.single { it.name == "B" }.properties + val property = inheritorProperties.single { it.name == "a" } + + assertNull(property.getter) + assertNull(property.setter) + + val jvmFieldAnnotation = property.extra[Annotations]?.directAnnotations?.values?.single()?.find { + it.isJvmField() + } + assertNotNull(jvmFieldAnnotation) + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } + } + + @Test + fun `should preserve regular functions that look like accessors, but are not accessors`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | public int a = 1; + | public void setA() { } // no arg + | public String getA() { return "s"; } // wrong return type + |} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val testClass = module.packages.single().classlikes.single { it.name == "A" } + + val setterLookalike = testClass.functions.firstOrNull { it.name == "setA" } + assertNotNull(setterLookalike) { + "Expected regular function not found, wrongly categorized as setter?" + } + + val getterLookalike = testClass.functions.firstOrNull { it.name == "getA" } + assertNotNull(getterLookalike) { + "Expected regular function not found, wrongly categorized as getter?" + } + } + } + } +} -- cgit