From 1f54363f0c377331c2755786824abe3abff7fc2d Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Mon, 13 Jun 2022 15:44:13 +0200 Subject: [PATCH] Handle more corner cases for inherited accessors --- ...faultDescriptorToDocumentableTranslator.kt | 36 ++- .../DescriptorAccessorConventionUtil.kt | 94 +++++- .../psi/DefaultPsiToDocumentableTranslator.kt | 25 +- .../psi/PsiAccessorConventionUtil.kt | 23 ++ .../test/kotlin/signatures/SignatureTest.kt | 292 +++++++++++++++++- .../DescriptorSuperPropertiesTest.kt | 72 ++++- .../signatures/JavaSignatureProvider.kt | 2 +- .../KotlinAsJavaDocumentableTransformer.kt | 2 +- 8 files changed, 499 insertions(+), 47 deletions(-) diff --git a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt index bfd755cd4f..27cdcf66e1 100644 --- a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt +++ b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt @@ -21,6 +21,7 @@ import org.jetbrains.dokka.links.Callable import org.jetbrains.dokka.model.* import org.jetbrains.dokka.model.AnnotationTarget import org.jetbrains.dokka.model.Nullable +import org.jetbrains.dokka.model.Visibility import org.jetbrains.dokka.model.doc.* import org.jetbrains.dokka.model.properties.PropertyContainer import org.jetbrains.dokka.plugability.DokkaContext @@ -43,6 +44,7 @@ import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor import org.jetbrains.kotlin.idea.kdoc.findKDoc import org.jetbrains.kotlin.idea.kdoc.resolveKDocLink import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi +import org.jetbrains.kotlin.load.java.descriptors.JavaPropertyDescriptor import org.jetbrains.kotlin.load.kotlin.toSourceElement import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* @@ -379,7 +381,7 @@ private class DokkaDescriptorVisitor( return coroutineScope { val descriptorsWithKind = scope.getDescriptorsWithKind() - val (regularFunctions, accessors) = splitFunctionsAndAccessors( + val (regularFunctions, accessors) = splitFunctionsAndInheritedAccessors( properties = descriptorsWithKind.properties, functions = descriptorsWithKind.functions ) @@ -427,11 +429,11 @@ private class DokkaDescriptorVisitor( /** * @param implicitAccessors getters/setters that are not part of the property descriptor, for instance - * average methods inherited from java sources + * average methods inherited from java sources that access the property */ private suspend fun visitPropertyDescriptor( originalDescriptor: PropertyDescriptor, - implicitAccessors: List, + implicitAccessors: DescriptorAccessorHolder?, parent: DRIWithPlatformInfo ): DProperty { val (dri, _) = originalDescriptor.createDRI() @@ -451,9 +453,7 @@ private class DokkaDescriptorVisitor( } suspend fun getImplicitAccessorGetter() = - implicitAccessors - .firstOrNull { it.isGetterFor(originalDescriptor) } - ?.let { visitFunctionDescriptor(it, parent) } + implicitAccessors?.getter?.let { visitFunctionDescriptor(it, parent) } // example - generated setter that comes with data classes suspend fun getDescriptorSetter() = @@ -464,9 +464,7 @@ private class DokkaDescriptorVisitor( } suspend fun getImplicitAccessorSetter() = - implicitAccessors - .firstOrNull { it.isSetterFor(originalDescriptor) } - ?.let { visitFunctionDescriptor(it, parent) } + implicitAccessors?.setter?.let { visitFunctionDescriptor(it, parent) } return coroutineScope { val generics = async { descriptor.typeParameters.parallelMap { it.toVariantTypeParameter() } } @@ -480,7 +478,7 @@ private class DokkaDescriptorVisitor( sources = actual, getter = getDescriptorGetter() ?: getImplicitAccessorGetter(), setter = getDescriptorSetter() ?: getImplicitAccessorSetter(), - visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(), + visibility = descriptor.getVisibility(implicitAccessors).toSourceSetDependent(), documentation = descriptor.resolveDescriptorData(), modifier = descriptor.modifier().toSourceSetDependent(), type = descriptor.returnType!!.toBound(), @@ -502,6 +500,19 @@ private class DokkaDescriptorVisitor( } } + private fun PropertyDescriptor.getVisibility(implicitAccessors: DescriptorAccessorHolder?): Visibility { + val isPrivateJavaPropertyWithPublicGetter = + this is JavaPropertyDescriptor + && !this.visibility.isPublicAPI + && implicitAccessors?.getter?.visibility?.isPublicAPI == true + + return if (isPrivateJavaPropertyWithPublicGetter) { + KotlinVisibility.Public + } else { + return this.visibility.toDokkaVisibility() + } + } + private fun CallableMemberDescriptor.createDRI(wasOverridenBy: DRI? = null): Pair = if (kind == CallableMemberDescriptor.Kind.DECLARATION || overriddenDescriptors.isEmpty()) Pair(DRI.from(this), wasOverridenBy) @@ -818,12 +829,11 @@ private class DokkaDescriptorVisitor( private suspend fun List.visitProperties( parent: DRIWithPlatformInfo, - implicitAccessors: Map> = emptyMap(), + implicitAccessors: Map = emptyMap(), ): List { return coroutineScope { parallelMap { - val propertyAccessors = implicitAccessors[it] ?: emptyList() - visitPropertyDescriptor(it, propertyAccessors, parent) + visitPropertyDescriptor(it, implicitAccessors[it], parent) } } } diff --git a/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt b/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt index f182b9bedd..292dbfcad5 100644 --- a/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt +++ b/plugins/base/src/main/kotlin/translators/descriptors/DescriptorAccessorConventionUtil.kt @@ -10,29 +10,93 @@ import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName internal data class DescriptorFunctionsHolder( val regularFunctions: List, - val accessors: Map> + val accessors: Map ) -internal fun splitFunctionsAndAccessors( +internal data class DescriptorAccessorHolder( + val getter: FunctionDescriptor? = null, + val setter: FunctionDescriptor? = null +) + +/** + * Separate regular Kotlin/Java functions and inherited Java accessors + * to properly display properties inherited from Java. + * + * Take this example: + * ``` + * // java + * public class JavaClass { + * private int a = 1; + * public int getA() { return a; } + * public void setA(int a) { this.a = a; } + * } + * + * // kotlin + * class Bar : JavaClass() { + * fun foo() {} + * } + * ``` + * + * It should result in: + * - 1 regular function `foo` + * - Map a=[`getA`, `setA`] + */ +internal fun splitFunctionsAndInheritedAccessors( properties: List, functions: List ): DescriptorFunctionsHolder { + val (javaMethods, kotlinFunctions) = functions.partition { it is JavaMethodDescriptor } + if (javaMethods.isEmpty()) { + return DescriptorFunctionsHolder(regularFunctions = kotlinFunctions, emptyMap()) + } + val propertiesByName = properties.associateBy { it.name.asString() } - val regularFunctions = mutableListOf() - val accessors = mutableMapOf>() - functions.forEach { function -> + val regularFunctions = ArrayList(kotlinFunctions) + + val accessors = mutableMapOf() + javaMethods.forEach { function -> val possiblePropertyNamesForFunction = function.toPossiblePropertyNames() val property = possiblePropertyNamesForFunction.firstNotNullOfOrNull { propertiesByName[it] } if (property != null && function.isAccessorFor(property)) { - accessors.getOrPut(property, ::mutableListOf).add(function) + accessors.compute(property) { prop, accessorHolder -> + if (function.isGetterFor(prop)) + accessorHolder?.copy(getter = function) ?: DescriptorAccessorHolder(getter = function) + else + accessorHolder?.copy(setter = function) ?: DescriptorAccessorHolder(setter = function) + } } else { regularFunctions.add(function) } } + + val accessorLookalikes = removeNonAccessorsReturning(accessors) + regularFunctions.addAll(accessorLookalikes) + return DescriptorFunctionsHolder(regularFunctions, accessors) } -internal fun FunctionDescriptor.toPossiblePropertyNames(): List { +/** + * If a field has no getter, it's not accessible as a property from Kotlin's perspective, + * but it still might have a setter lookalike. In this case, this "setter" should be just a regular function + * + * @return removed elements + */ +private fun removeNonAccessorsReturning( + propertyAccessors: MutableMap +): List { + val nonAccessors = mutableListOf() + propertyAccessors.entries.removeIf { (_, accessors) -> + if (accessors.getter == null && accessors.setter != null) { + nonAccessors.add(accessors.setter) + true + } else { + false + } + } + return nonAccessors +} + +private fun FunctionDescriptor.toPossiblePropertyNames(): List { val stringName = this.name.asString() return when { JvmAbi.isSetterName(stringName) -> propertyNamesBySetMethodName(this.name).map { it.asString() } @@ -41,7 +105,7 @@ internal fun FunctionDescriptor.toPossiblePropertyNames(): List { } } -internal fun propertyNamesByGetMethod(functionDescriptor: FunctionDescriptor): List { +private fun propertyNamesByGetMethod(functionDescriptor: FunctionDescriptor): List { val stringName = functionDescriptor.name.asString() // In java, the convention for boolean property accessors is as follows: // - `private boolean active;` @@ -63,21 +127,19 @@ internal fun propertyNamesByGetMethod(functionDescriptor: FunctionDescriptor): L return listOfNotNull(javaPropName, kotlinPropName) } -internal fun FunctionDescriptor.isAccessorFor(property: PropertyDescriptor): Boolean { - return this.isGetterFor(property) || this.isSetterFor(property) +private fun FunctionDescriptor.isAccessorFor(property: PropertyDescriptor): Boolean { + return (this.isGetterFor(property) || this.isSetterFor(property)) + && !property.visibility.isPublicAPI + && this.visibility.isPublicAPI } -internal fun FunctionDescriptor.isGetterFor(property: PropertyDescriptor): Boolean { +private 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 { +private 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 c20073a47c..a716da34f0 100644 --- a/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt +++ b/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt @@ -17,9 +17,9 @@ import org.jetbrains.dokka.analysis.KotlinAnalysis import org.jetbrains.dokka.analysis.PsiDocumentableSource import org.jetbrains.dokka.analysis.from import org.jetbrains.dokka.base.DokkaBase -import org.jetbrains.dokka.base.translators.typeConstructorsBeingExceptions import org.jetbrains.dokka.base.translators.psi.parsers.JavaDocumentationParser import org.jetbrains.dokka.base.translators.psi.parsers.JavadocParser +import org.jetbrains.dokka.base.translators.typeConstructorsBeingExceptions import org.jetbrains.dokka.base.translators.unquotedValue import org.jetbrains.dokka.links.* import org.jetbrains.dokka.model.* @@ -40,12 +40,7 @@ import org.jetbrains.kotlin.asJava.elements.KtLightAbstractAnnotation import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys import org.jetbrains.kotlin.cli.jvm.config.JavaSourceRoot import org.jetbrains.kotlin.idea.base.utils.fqname.getKotlinFqName -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.psi.psiUtil.getChildOfType -import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.utils.KotlinExceptionWithAttachments import org.jetbrains.kotlin.utils.addToStdlib.safeAs import java.io.File @@ -641,7 +636,7 @@ class DefaultPsiToDocumentableTranslator( documentation = javadocParser.parseDocumentation(psi).toSourceSetDependent(), expectPresentInSet = null, sources = PsiDocumentableSource(psi).toSourceSetDependent(), - visibility = psi.getVisibility().toSourceSetDependent(), + visibility = psi.getVisibility(getter).toSourceSetDependent(), type = getBound(psi.type), receiver = null, setter = setter, @@ -666,6 +661,22 @@ class DefaultPsiToDocumentableTranslator( ) } + private fun PsiField.getVisibility(getter: DFunction?): Visibility { + val psiVisibility = this.getVisibility() + val isPrivatePropertyWithPublicGetter = !psiVisibility.isPublicAPI() + && getter?.visibility?.get(sourceSetData)?.isPublicAPI() == true + + return if (isPrivatePropertyWithPublicGetter) JavaVisibility.Public else psiVisibility + } + + private fun Visibility.isPublicAPI() = when (this) { + JavaVisibility.Public, + JavaVisibility.Protected, + KotlinVisibility.Public, + KotlinVisibility.Protected -> true + else -> false + } + private fun Collection.toListOfAnnotations() = filter { it !is KtLightAbstractAnnotation }.mapNotNull { it.toAnnotation() } diff --git a/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt index c2ab8c03b3..7ca02107c9 100644 --- a/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt +++ b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt @@ -28,9 +28,32 @@ internal fun splitFunctionsAndAccessors(fields: Array, methods: Array< regularFunctions.add(method) } } + + val accessorLookalikes = removeNonAccessorsReturning(accessors) + regularFunctions.addAll(accessorLookalikes) + return PsiFunctionsHolder(regularFunctions, accessors) } +/** + * If a field has no getter, it's not accessible as a property from Kotlin's perspective, + * but it still might have a setter. In this case, this "setter" should be just a regular function + */ +private fun removeNonAccessorsReturning( + fieldAccessors: MutableMap> +): List { + val nonAccessors = mutableListOf() + fieldAccessors.entries.removeIf { (field, methods) -> + if (methods.size == 1 && methods[0].isSetterFor(field)) { + nonAccessors.add(methods[0]) + true + } else { + false + } + } + return nonAccessors +} + internal fun PsiMethod.getPossiblePropertyNamesForFunction(): List { val jvmName = getAnnotation(DescriptorUtils.JVM_NAME.asString())?.findAttributeValue("name")?.text return jvmName?.let { listOf(jvmName) } diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt index 59665b8cb5..ddb90bff57 100644 --- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt +++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt @@ -15,7 +15,10 @@ class SignatureTest : BaseAbstractTest() { sourceSets { sourceSet { sourceRoots = listOf("src/") - classpath = listOf(commonStdlibPath ?: throw IllegalStateException("Common stdlib is not found"), jvmStdlibPath ?: throw IllegalStateException("JVM stdlib is not found")) + classpath = listOf( + commonStdlibPath ?: throw IllegalStateException("Common stdlib is not found"), + jvmStdlibPath ?: throw IllegalStateException("JVM stdlib is not found") + ) externalDocumentationLinks = listOf(stdlibExternalDocumentationLink) } } @@ -510,6 +513,293 @@ class SignatureTest : BaseAbstractTest() { } } + + @Test + fun `should collapse accessor functions inherited from java into the property`() { + val writerPlugin = TestOutputWriterPlugin() + 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(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + writerPlugin.writer.renderedContent("root/test/-b/index.html").let { kotlinClassContent -> + val signatures = kotlinClassContent.signature().toList() + assertEquals( + 3, signatures.size, + "Expected 3 signatures: class signature, constructor and property" + ) + + val property = signatures[2] + property.match( + "var ", A("a"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + + writerPlugin.writer.renderedContent("root/test/-a/index.html").let { javaClassContent -> + val signatures = javaClassContent.signature().toList() + assertEquals( + 2, signatures.size, + "Expected 2 signatures: class signature and property" + ) + + val property = signatures[1] + property.match( + "open var ", A("a"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } + + @Test + fun `should render as val if inherited java property has no setter`() { + val writerPlugin = TestOutputWriterPlugin() + 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(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + writerPlugin.writer.renderedContent("root/test/-b/index.html").let { kotlinClassContent -> + val signatures = kotlinClassContent.signature().toList() + assertEquals(3, signatures.size, "Expected 3 signatures: class signature, constructor and property") + + val property = signatures[2] + property.match( + "val ", A("a"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + + writerPlugin.writer.renderedContent("root/test/-a/index.html").let { javaClassContent -> + val signatures = javaClassContent.signature().toList() + assertEquals(2, signatures.size, "Expected 2 signatures: class signature and property") + + val property = signatures[1] + property.match( + "open val ", A("a"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } + + @Test + fun `should keep inherited java setter as a regular function due to inaccessible property`() { + val writerPlugin = TestOutputWriterPlugin() + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | private int a = 1; + | public void setA(int a) { this.a = a; } + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + writerPlugin.writer.renderedContent("root/test/-b/index.html").let { kotlinClassContent -> + val signatures = kotlinClassContent.signature().toList() + assertEquals(3, signatures.size, "Expected 3 signatures: class signature, constructor and setter") + + val setterFunction = signatures[2] + setterFunction.match( + "open fun ", A("setA"), "(", Parameters( + Parameter("a: ", A("Int")) + ), ")", Span(), + ignoreSpanWithTokenStyle = true + ) + } + + writerPlugin.writer.renderedContent("root/test/-a/index.html").let { javaClassContent -> + val signatures = javaClassContent.signature().toList() + assertEquals(2, signatures.size, "Expected 2 signatures: class signature and setter") + + val setterFunction = signatures[1] + setterFunction.match( + "open fun ", A("setA"), "(", Parameters( + Parameter("a: ", A("Int")) + ), ")", Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } + + @Test + fun `should keep inherited java accessor lookalikes if underlying function is public`() { + val writerPlugin = TestOutputWriterPlugin() + 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(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + val signatures = writerPlugin.writer.renderedContent("root/test/-b/index.html").signature().toList() + assertEquals( + 5, signatures.size, + "Expected 5 signatures: class signature, constructor, property and two accessor lookalikes" + ) + + val getterLookalikeFunction = signatures[2] + getterLookalikeFunction.match( + "open fun ", A("getA"), "():", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + + val setterLookalikeFunction = signatures[3] + setterLookalikeFunction.match( + "open fun ", A("setA"), "(", Parameters( + Parameter("a: ", A("Int")) + ), ")", Span(), + ignoreSpanWithTokenStyle = true + ) + + val property = signatures[4] + property.match( + "val ", A("a"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + + @Test + fun `should keep kotlin property with no accessors when java inherits kotlin a var`() { + val writerPlugin = TestOutputWriterPlugin() + testInline( + """ + |/src/test/JavaClass.java + |package test; + |public class JavaClass extends KotlinClass {} + | + |/src/test/KotlinClass.kt + |package test + |open class KotlinClass { + | var variable: String = "s" + |} + """.trimIndent(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + writerPlugin.writer.renderedContent("root/test/-java-class/index.html").let { kotlinClassContent -> + val signatures = kotlinClassContent.signature().toList() + assertEquals(2, signatures.size, "Expected to find two signatures: class and property") + + val property = signatures[1] + property.match( + "open var ", A("variable"), ": ", Span("String"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } + + @Test + fun `kotlin property with compute get and set`() { + val writerPlugin = TestOutputWriterPlugin() + testInline( + """ + |/src/test/JavaClass.java + |package test; + |public class JavaClass extends KotlinClass {} + | + |/src/test/KotlinClass.kt + |package test + |open class KotlinClass { + | var variable: String + | get() = "asd" + | set(value) {} + |} + """.trimIndent(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + writerPlugin.writer.renderedContent("root/test/-kotlin-class/index.html").let { kotlinClassContent -> + val signatures = kotlinClassContent.signature().toList() + assertEquals(3, signatures.size, "Expected to find 3 signatures: class, constructor and property") + + val property = signatures[2] + property.match( + "var ", A("variable"), ": ", A("String"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + + // it's actually unclear how it should react in this situation. It should most likely not + // break the abstraction and display it as a simple variable just like can be seen from Kotlin, + // test added to control changes + writerPlugin.writer.renderedContent("root/test/-java-class/index.html").let { javaClassContent -> + val signatures = javaClassContent.signature().toList() + assertEquals(3, signatures.size, "Expected to find 3 signatures: class and two accessors") + + val getter = signatures[1] + getter.match( + "fun ", A("getVariable"), "(): ", Span("String"), Span(), + ignoreSpanWithTokenStyle = true + ) + + val setter = signatures[2] + setter.match( + "fun ", A("setVariable"), "(", Parameters( + Parameter("value: ", Span("String")) + ), ")", Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } + @Test fun `actual property with a default value`() { val writerPlugin = TestOutputWriterPlugin() diff --git a/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt index a6dd43502e..f785c81556 100644 --- a/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt +++ b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt @@ -1,5 +1,6 @@ package superFields +import org.jetbrains.dokka.DokkaConfiguration import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest import org.jetbrains.dokka.links.DRI import org.jetbrains.dokka.model.InheritedMember @@ -53,6 +54,41 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() { } } + + @Test + fun `kotlin inheriting java should ignore setter lookalike for non accessible field`() { + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | private int a = 1; + | + | public void setA(int a) { this.a = a; } + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + commonTestConfiguration + ) { + documentablesMergingStage = { module -> + val testedClass = module.packages.single().classlikes.single { it.name == "B" } + + val property = testedClass.properties.firstOrNull { it.name == "a" } + assertNull(property, "Inherited property `a` should not be visible as it's not accessible") + + val setterLookalike = testedClass.functions.firstOrNull { it.name == "setA" } + assertNotNull(setterLookalike) { + "Expected setA to be a regular function because field `a` is neither var nor val from Kotlin's " + + "interop perspective, it's not accessible." + } + } + } + } + + @Test fun `kotlin inheriting java should append getter and setter`() { testInline( @@ -130,13 +166,27 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() { } @Test - fun `kotlin inheriting java should not append anything since field is public`() { + fun `kotlin inheriting java should not append anything since field is public api`() { + val configuration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + analysisPlatform = "jvm" + name = "jvm" + documentedVisibilities = setOf( + DokkaConfiguration.Visibility.PUBLIC, + DokkaConfiguration.Visibility.PROTECTED + ) + } + } + } + testInline( """ |/src/test/A.java |package test; |public class A { - | public int a = 1; + | protected int a = 1; | public int getA() { return a; } | public void setA(int a) { this.a = a; } |} @@ -145,14 +195,18 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() { |package test |class B : A {} """.trimIndent(), - commonTestConfiguration + configuration ) { documentablesMergingStage = { module -> - val kotlinProperties = module.packages.single().classlikes.single { it.name == "B" }.properties - val property = kotlinProperties.single { it.name == "a" } + val testedClass = module.packages.single().classlikes.single { it.name == "B" } + val property = testedClass.properties.single { it.name == "a" } assertNull(property.getter) assertNull(property.setter) + assertEquals(2, testedClass.functions.size) + + assertEquals("getA", testedClass.functions[0].name) + assertEquals("setA", testedClass.functions[1].name) val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) @@ -167,9 +221,11 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() { |/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 + | private var v: Int = 0 + | + | // not accessors because declared separately, just functions + | fun setV(new: Int) { v = new } + | fun getV(): Int = v |} """.trimIndent(), commonTestConfiguration diff --git a/plugins/kotlin-as-java/src/main/kotlin/signatures/JavaSignatureProvider.kt b/plugins/kotlin-as-java/src/main/kotlin/signatures/JavaSignatureProvider.kt index 50dbc1cb17..cff5b18299 100644 --- a/plugins/kotlin-as-java/src/main/kotlin/signatures/JavaSignatureProvider.kt +++ b/plugins/kotlin-as-java/src/main/kotlin/signatures/JavaSignatureProvider.kt @@ -114,7 +114,7 @@ class JavaSignatureProvider internal constructor(ctcc: CommentsToContentConverte ) { annotationsBlock(p) p.visibility[it]?.takeIf { it !in ignoredVisibilities }?.name?.let { keyword("$it ") } - p.modifier[it]?.name?.let { keyword("$it ") } + p.modifier[it]?.takeIf { it !in ignoredModifiers }?.name?.let { keyword("$it ") } p.modifiers()[it]?.toSignatureString()?.let { keyword(it) } signatureForProjection(p.type) text(nbsp.toString()) diff --git a/plugins/kotlin-as-java/src/main/kotlin/transformers/KotlinAsJavaDocumentableTransformer.kt b/plugins/kotlin-as-java/src/main/kotlin/transformers/KotlinAsJavaDocumentableTransformer.kt index 5916a11c94..8b07670f09 100644 --- a/plugins/kotlin-as-java/src/main/kotlin/transformers/KotlinAsJavaDocumentableTransformer.kt +++ b/plugins/kotlin-as-java/src/main/kotlin/transformers/KotlinAsJavaDocumentableTransformer.kt @@ -8,4 +8,4 @@ import org.jetbrains.dokka.transformers.documentation.DocumentableTransformer class KotlinAsJavaDocumentableTransformer : DocumentableTransformer { override fun invoke(original: DModule, context: DokkaContext): DModule = original.copy(packages = original.packages.map { it.asJava() }) -} \ No newline at end of file +}