diff options
14 files changed, 956 insertions, 211 deletions
diff --git a/.github/workflows/qodana.yml b/.github/workflows/qodana.yml index 0eee8bd8..74b168e8 100644 --- a/.github/workflows/qodana.yml +++ b/.github/workflows/qodana.yml @@ -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: diff --git a/core/api/core.api b/core/api/core.api index 2a35b178..11dd1c45 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -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 diff --git a/core/src/main/kotlin/model/JvmField.kt b/core/src/main/kotlin/model/JvmField.kt new file mode 100644 index 00000000..623115e0 --- /dev/null +++ b/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() 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 <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 +} 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<FunctionDescriptor>, + 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<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() } } @@ -438,12 +478,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(), @@ -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<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) = with(descriptor) { coroutineScope { @@ -740,11 +813,20 @@ private class DokkaDescriptorVisitor( ) } - private suspend fun List<FunctionDescriptor>.visitFunctions(): List<DFunction> = - coroutineScope { parallelMap { visitFunctionDescriptor(it) } } + private suspend fun List<FunctionDescriptor>.visitFunctions(parent: DRIWithPlatformInfo): List<DFunction> = + coroutineScope { parallelMap { visitFunctionDescriptor(it, parent) } } - private suspend fun List<PropertyDescriptor>.visitProperties(): List<DProperty> = - coroutineScope { parallelMap { visitPropertyDescriptor(it) } } + 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) } } @@ -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<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 +} + 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<PsiJavaFile>() }.groupBy { it.packageName }.toList() + name = context.configuration.moduleName, + packages = psiFiles.parallelMapNotNull { it.safeAs<PsiJavaFile>() }.groupBy { it.packageName }.toList() .parallelMap { (packageName: String, psiFiles: List<PsiJavaFile>) -> 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<Int>() val superMethods = mutableListOf<Pair<PsiMethod, DRI>>() + val superFieldsKeys = hashSetOf<Int>() + val superFields = mutableListOf<Pair<PsiField, DRI>>() methods.asIterable().parallelForEach { superMethodsKeys.add(it.hash) } /** - * Caution! This method mutates superMethodsKeys and superMethods + * Caution! This method mutates + * - superMethodsKeys + * - superMethods + * - superFieldsKeys + * - superKeys */ fun Array<PsiClassType>.getSuperTypesPsiClasses(): List<Pair<PsiClass, JavaClassKindTypes>> { 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<PsiEnumConstant>().map { entry -> + dri = dri, + name = name.orEmpty(), + entries = fields.filterIsInstance<PsiEnumConstant>().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<Param> { 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<PsiAnnotation>.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 <T : AnnotationTarget> PsiTypeParameter.annotations(): PropertyContainer<T> = this.annotations.toList().toListOfAnnotations().annotations() private fun <T : AnnotationTarget> PsiType.annotations(): PropertyContainer<T> = 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<Pair<PsiMethod, DRI>>, + 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<MutableList<PsiMethod>, MutableMap<PsiField, MutableList<PsiMethod>>> { - val fieldNames = fields.associateBy { it.name } - val accessors = mutableMapOf<PsiField, MutableList<PsiMethod>>() - val regularMethods = mutableListOf<PsiMethod>() - 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<PsiMethod>, 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<PsiMethod>): 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<PsiMethod>, + val accessors: Map<PsiField, List<PsiMethod>> +) + +internal fun splitFunctionsAndAccessors(fields: Array<PsiField>, methods: Array<PsiMethod>): PsiFunctionsHolder { + val fieldsByName = fields.associateBy { it.name } + val regularFunctions = mutableListOf<PsiMethod>() + val accessors = mutableMapOf<PsiField, MutableList<PsiMethod>>() + 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<String> { + 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?" + } + } + } + } +} diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt index 1fbc21ea..22348ce1 100644 --- a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt +++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt @@ -71,7 +71,7 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { AnnotationTarget.ANNOTATION_CLASS::class.java.methods.any { it.name == "describeConstable" } testIndexPages(commonTestQuery) { indexPages -> - assertEquals(if (hasAdditionalFunction()) 41 else 40, indexPages.sumBy { it.elements.size }) + assertEquals(if (hasAdditionalFunction()) 32 else 31, indexPages.sumBy { it.elements.size }) } } diff --git a/plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt b/plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt index 1e3bd800..8ff35781 100644 --- a/plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt +++ b/plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt @@ -87,7 +87,7 @@ internal fun DProperty.asJava(isTopLevel: Boolean = false, relocateToClass: Stri visibility = visibility.mapValues { if (isTopLevel && isConst) { JavaVisibility.Public - } else if (jvmField() != null) { + } else if (jvmField() != null || (getter == null && setter == null)) { it.value.asJava() } else { it.value.propertyVisibilityAsJava() @@ -275,7 +275,7 @@ internal fun DClass.functionsInJava(): List<DFunction> = .flatMap { property -> listOfNotNull(property.getter, property.setter) } .plus(functions) .filterNot { it.hasJvmSynthetic() } - .flatMap { it.asJava(dri.classNames ?: name) } + .flatMap { it.asJava(it.dri.classNames ?: it.name) } private fun DTypeParameter.asJava(): DTypeParameter = copy( variantTypeParameter = variantTypeParameter.withDri(dri.possiblyAsJava()), @@ -317,7 +317,7 @@ internal fun DEnum.asJava(): DEnum = copy( functions = functions .plus( properties - .filterNot { it.hasJvmSynthetic() } + .filter { it.jvmField() == null && !it.hasJvmSynthetic() } .flatMap { listOf(it.getter, it.setter) } ) .filterNotNull() @@ -335,7 +335,7 @@ internal fun DObject.asJava(): DObject = copy( functions = functions .plus( properties - .filterNot { it.hasJvmSynthetic() } + .filter { it.jvmField() == null && !it.hasJvmSynthetic() } .flatMap { listOf(it.getter, it.setter) } ) .filterNotNull() @@ -373,7 +373,7 @@ internal fun DInterface.asJava(): DInterface = copy( functions = functions .plus( properties - .filterNot { it.hasJvmSynthetic() } + .filter { it.jvmField() == null && !it.hasJvmSynthetic() } .flatMap { listOf(it.getter, it.setter) } ) .filterNotNull() diff --git a/plugins/kotlin-as-java/src/test/kotlin/JvmFieldTest.kt b/plugins/kotlin-as-java/src/test/kotlin/JvmFieldTest.kt index 2f490421..99ea017b 100644 --- a/plugins/kotlin-as-java/src/test/kotlin/JvmFieldTest.kt +++ b/plugins/kotlin-as-java/src/test/kotlin/JvmFieldTest.kt @@ -5,6 +5,7 @@ import org.jetbrains.dokka.model.JavaVisibility import org.junit.jupiter.api.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull class JvmFieldTest : BaseAbstractTest() { val configuration = dokkaConfiguration { @@ -78,4 +79,31 @@ class JvmFieldTest : BaseAbstractTest() { } } } -}
\ No newline at end of file + + @Test + fun `object jvmfield property should have no getters`(){ + testInline( + """ + |/src/main/kotlin/kotlinAsJavaPlugin/sample.kt + |package kotlinAsJavaPlugin + |object MyObject { + | @JvmField + | val property: String = TODO() + |} + """.trimMargin(), + configuration, + ) { + documentablesTransformationStage = { module -> + val classLike = module.packages.flatMap { it.classlikes }.first() + val property = classLike.properties.singleOrNull { it.name == "property" } + assertNotNull(property) + assertEquals( + emptyList(), + classLike.functions.map { it.name } + ) + assertNull(property.getter) + assertNull(property.setter) + } + } + } +} |