From b1acc1f9c4efa36a027581c870007ad9b34c767c Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 16 Jun 2022 15:25:35 +0200 Subject: [PATCH] Make java properties inherit visibility from getters --- ...faultDescriptorToDocumentableTranslator.kt | 27 +- .../psi/DefaultPsiToDocumentableTranslator.kt | 34 +- .../psi/PsiAccessorConventionUtil.kt | 15 +- .../InheritedAccessorsSignatureTest.kt | 431 ++++++++++++++++++ .../test/kotlin/signatures/SignatureTest.kt | 287 ------------ .../DescriptorSuperPropertiesTest.kt | 110 ++++- .../kotlin/superFields/PsiSuperFieldsTest.kt | 31 +- ...tDescriptorToDocumentableTranslatorTest.kt | 106 +++-- .../DefaultPsiToDocumentableTranslatorTest.kt | 72 +++ .../dokka/javadoc/JavadocIndexTest.kt | 4 +- 10 files changed, 709 insertions(+), 408 deletions(-) create mode 100644 plugins/base/src/test/kotlin/signatures/InheritedAccessorsSignatureTest.kt diff --git a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt index 27cdcf66e1..38992ba009 100644 --- a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt +++ b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt @@ -41,6 +41,7 @@ import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.annotations.Annotated import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor +import org.jetbrains.kotlin.descriptors.java.JavaVisibilities import org.jetbrains.kotlin.idea.kdoc.findKDoc import org.jetbrains.kotlin.idea.kdoc.resolveKDocLink import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi @@ -501,16 +502,19 @@ private class DokkaDescriptorVisitor( } private fun PropertyDescriptor.getVisibility(implicitAccessors: DescriptorAccessorHolder?): Visibility { - val isPrivateJavaPropertyWithPublicGetter = - this is JavaPropertyDescriptor - && !this.visibility.isPublicAPI - && implicitAccessors?.getter?.visibility?.isPublicAPI == true + val isNonPublicJavaProperty = this is JavaPropertyDescriptor && !this.visibility.isPublicAPI + val visibility = + if (isNonPublicJavaProperty) { + // only try to take implicit getter's visibility if it's a java property + // because it's not guaranteed that implicit accessor will be used + // for the kotlin property, as it may have an explicit accessor of its own, + // i.e in data classes or with get() and set() are overridden + (implicitAccessors?.getter?.visibility ?: this.visibility) + } else { + this.visibility + } - return if (isPrivateJavaPropertyWithPublicGetter) { - KotlinVisibility.Public - } else { - return this.visibility.toDokkaVisibility() - } + return visibility.toDokkaVisibility() } private fun CallableMemberDescriptor.createDRI(wasOverridenBy: DRI? = null): Pair = @@ -1161,11 +1165,14 @@ private class DokkaDescriptorVisitor( }) + ancestry.interfaces.map { TypeConstructorWithKind(it.typeConstructor, KotlinClassKindTypes.INTERFACE) } } - private fun DescriptorVisibility.toDokkaVisibility(): org.jetbrains.dokka.model.Visibility = when (this.delegate) { + private fun DescriptorVisibility.toDokkaVisibility(): Visibility = when (this.delegate) { Visibilities.Public -> KotlinVisibility.Public Visibilities.Protected -> KotlinVisibility.Protected Visibilities.Internal -> KotlinVisibility.Internal Visibilities.Private -> KotlinVisibility.Private + JavaVisibilities.ProtectedAndPackage -> KotlinVisibility.Protected + JavaVisibilities.ProtectedStaticVisibility -> KotlinVisibility.Protected + JavaVisibilities.PackageVisibility -> JavaVisibility.Default else -> KotlinVisibility.Public } diff --git a/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt index a716da34f0..bef86144fc 100644 --- a/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt +++ b/plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt @@ -103,16 +103,6 @@ class DefaultPsiToDocumentableTranslator( private val cachedBounds = hashMapOf() - private fun PsiModifierListOwner.getVisibility() = modifierList?.let { - val ml = it.children.toList() - when { - ml.any { it.text == PsiKeyword.PUBLIC } || it.hasModifierProperty("public") -> JavaVisibility.Public - ml.any { it.text == PsiKeyword.PROTECTED } || it.hasModifierProperty("protected") -> JavaVisibility.Protected - ml.any { it.text == PsiKeyword.PRIVATE } || it.hasModifierProperty("private") -> JavaVisibility.Private - else -> JavaVisibility.Default - } - } ?: JavaVisibility.Default - private val PsiMethod.hash: Int get() = "$returnType $name$parameterList".hashCode() @@ -662,19 +652,7 @@ 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 + return getter?.visibility?.get(sourceSetData) ?: this.getVisibility() } private fun Collection.toListOfAnnotations() = @@ -751,3 +729,13 @@ class DefaultPsiToDocumentableTranslator( get() = getChildOfType()?.resolve() } } + +internal fun PsiModifierListOwner.getVisibility() = modifierList?.let { + val ml = it.children.toList() + when { + ml.any { it.text == PsiKeyword.PUBLIC } || it.hasModifierProperty("public") -> JavaVisibility.Public + ml.any { it.text == PsiKeyword.PROTECTED } || it.hasModifierProperty("protected") -> JavaVisibility.Protected + ml.any { it.text == PsiKeyword.PRIVATE } || it.hasModifierProperty("private") -> JavaVisibility.Private + else -> JavaVisibility.Default + } +} ?: JavaVisibility.Default diff --git a/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt index 7ca02107c9..2ab7084386 100644 --- a/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt +++ b/plugins/base/src/main/kotlin/translators/psi/PsiAccessorConventionUtil.kt @@ -3,6 +3,9 @@ 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.dokka.model.JavaVisibility +import org.jetbrains.dokka.model.KotlinVisibility +import org.jetbrains.dokka.model.Visibility import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.load.java.propertyNameByGetMethodName import org.jetbrains.kotlin.load.java.propertyNamesBySetMethodName @@ -69,7 +72,9 @@ internal fun PsiMethod.getPossiblePropertyNamesForFunction(): List { } internal fun PsiMethod.isAccessorFor(field: PsiField): Boolean { - return this.isGetterFor(field) || this.isSetterFor(field) + return (this.isGetterFor(field) || this.isSetterFor(field)) + && !field.getVisibility().isPublicAPI() + && this.getVisibility().isPublicAPI() } internal fun PsiMethod.isGetterFor(field: PsiField): Boolean { @@ -79,3 +84,11 @@ internal fun PsiMethod.isGetterFor(field: PsiField): Boolean { internal fun PsiMethod.isSetterFor(field: PsiField): Boolean { return parameterList.getParameter(0)?.type == field.type } + +internal fun Visibility.isPublicAPI() = when(this) { + KotlinVisibility.Public, + KotlinVisibility.Protected, + JavaVisibility.Public, + JavaVisibility.Protected -> true + else -> false +} diff --git a/plugins/base/src/test/kotlin/signatures/InheritedAccessorsSignatureTest.kt b/plugins/base/src/test/kotlin/signatures/InheritedAccessorsSignatureTest.kt new file mode 100644 index 0000000000..d3e0366680 --- /dev/null +++ b/plugins/base/src/test/kotlin/signatures/InheritedAccessorsSignatureTest.kt @@ -0,0 +1,431 @@ +package signatures + +import org.jetbrains.dokka.DokkaConfiguration +import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest +import org.junit.jupiter.api.Test +import utils.A +import utils.Span +import utils.TestOutputWriterPlugin +import utils.match +import kotlin.test.assertEquals + +class InheritedAccessorsSignatureTest : BaseAbstractTest() { + + private val configuration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + classpath = listOf( + commonStdlibPath ?: throw IllegalStateException("Common stdlib is not found"), + jvmStdlibPath ?: throw IllegalStateException("JVM stdlib is not found") + ) + externalDocumentationLinks = listOf(stdlibExternalDocumentationLink) + } + } + } + + @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 `inherited property should inherit getter's visibility`() { + val configWithProtectedVisibility = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + classpath = listOf( + commonStdlibPath ?: throw IllegalStateException("Common stdlib is not found"), + jvmStdlibPath ?: throw IllegalStateException("JVM stdlib is not found") + ) + externalDocumentationLinks = listOf(stdlibExternalDocumentationLink) + documentedVisibilities = setOf( + DokkaConfiguration.Visibility.PUBLIC, + DokkaConfiguration.Visibility.PROTECTED + ) + } + } + } + + val writerPlugin = TestOutputWriterPlugin() + testInline( + """ + |/src/test/JavaClass.java + |package test; + |public class JavaClass { + | private int protectedGetterAndProtectedSetter = 0; + | + | protected int getProtectedGetterAndProtectedSetter() { + | return protectedGetterAndProtectedSetter; + | } + | + | protected void setProtectedGetterAndProtectedSetter(int protectedGetterAndProtectedSetter) { + | this.protectedGetterAndProtectedSetter = protectedGetterAndProtectedSetter; + | } + |} + | + |/src/test/KotlinClass.kt + |package test + |open class KotlinClass : JavaClass() { } + """.trimIndent(), + configWithProtectedVisibility, + 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 3 signatures: class signature, constructor and property") + + val property = signatures[2] + property.match( + "protected var ", A("protectedGetterAndProtectedSetter"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + + writerPlugin.writer.renderedContent("root/test/-java-class/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( + "protected open var ", A("protectedGetterAndProtectedSetter"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } + + @Test + fun `should resolve protected java property as protected`() { + val configWithProtectedVisibility = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + classpath = listOf( + commonStdlibPath ?: throw IllegalStateException("Common stdlib is not found"), + jvmStdlibPath ?: throw IllegalStateException("JVM stdlib is not found") + ) + externalDocumentationLinks = listOf(stdlibExternalDocumentationLink) + documentedVisibilities = setOf( + DokkaConfiguration.Visibility.PUBLIC, + DokkaConfiguration.Visibility.PROTECTED + ) + } + } + } + + val writerPlugin = TestOutputWriterPlugin() + testInline( + """ + |/src/test/JavaClass.java + |package test; + |public class JavaClass { + | protected int protectedProperty = 0; + |} + | + |/src/test/KotlinClass.kt + |package test + |open class KotlinClass : JavaClass() { } + """.trimIndent(), + configWithProtectedVisibility, + 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 2 signatures: class signature, constructor and property") + + val property = signatures[2] + property.match( + "protected val ", A("protectedProperty"), ":", A("Int"), Span(), + ignoreSpanWithTokenStyle = true + ) + } + } + } + } +} diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt index ddb90bff57..b021fae147 100644 --- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt +++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt @@ -513,293 +513,6 @@ 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 f785c81556..06ced8c9dd 100644 --- a/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt +++ b/plugins/base/src/test/kotlin/superFields/DescriptorSuperPropertiesTest.kt @@ -4,6 +4,7 @@ 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 +import org.jetbrains.dokka.model.KotlinVisibility import org.junit.jupiter.api.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -215,33 +216,106 @@ class DescriptorSuperPropertiesTest : BaseAbstractTest() { } @Test - fun `should preserve regular functions that look like accessors, but are not accessors`() { + fun `should inherit property visibility from getter`() { + val configuration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + analysisPlatform = "jvm" + name = "jvm" + documentedVisibilities = setOf( + DokkaConfiguration.Visibility.PUBLIC, + DokkaConfiguration.Visibility.PROTECTED + ) + } + } + } + testInline( """ - |/src/test/A.kt - |package test - |class A { - | private var v: Int = 0 - | - | // not accessors because declared separately, just functions - | fun setV(new: Int) { v = new } - | fun getV(): Int = v + |/src/test/A.java + |package test; + |public class A { + | private int a = 1; + | protected int getA() { return a; } + | protected void setA(int a) { this.a = a; } |} + | + |/src/test/B.kt + |package test + |class B : A {} """.trimIndent(), - commonTestConfiguration + configuration ) { 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 testedClass = module.packages.single().classlikes.single { it.name == "B" } + assertEquals(0, testedClass.functions.size) + + val property = testedClass.properties.single { it.name == "a" } - val getterLookalike = testClass.functions.firstOrNull { it.name == "getV" } - assertNotNull(getterLookalike) { - "Expected regular function not found, wrongly categorized as getter?" + assertNotNull(property.getter) + assertNotNull(property.setter) + + val propertyVisibility = property.visibility.values.single() + assertEquals(KotlinVisibility.Protected, propertyVisibility) + + val getterVisibility = property.getter?.visibility?.values?.single() + assertEquals(KotlinVisibility.Protected, getterVisibility) + + val setterVisibility = property.setter?.visibility?.values?.single() + assertEquals(KotlinVisibility.Protected, setterVisibility) + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } + } + + @Test // checking for mapping between kotlin and java visibility + fun `should resolve inherited java protected field as protected`() { + 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 { + | protected int protectedProperty = 0; + |} + | + |/src/test/B.kt + |package test + |class B : A {} + """.trimIndent(), + configuration + ) { + documentablesMergingStage = { module -> + val testedClass = module.packages.single().classlikes.single { it.name == "B" } + assertEquals(0, testedClass.functions.size) + + val property = testedClass.properties.single { it.name == "protectedProperty" } + + assertNull(property.getter) + assertNull(property.setter) + + val propertyVisibility = property.visibility.values.single() + assertEquals(KotlinVisibility.Protected, propertyVisibility) + + val inheritedFrom = property.extra[InheritedMember]?.inheritedFrom?.values?.single() + assertEquals(DRI(packageName = "test", classNames = "A"), inheritedFrom) + } + } } } diff --git a/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt b/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt index 025c9b06af..8dd74ef261 100644 --- a/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.kt +++ b/plugins/base/src/test/kotlin/superFields/PsiSuperFieldsTest.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.Annotations @@ -153,34 +154,4 @@ class PsiSuperFieldsTest : BaseAbstractTest() { } } } - - @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/base/src/test/kotlin/translators/DefaultDescriptorToDocumentableTranslatorTest.kt b/plugins/base/src/test/kotlin/translators/DefaultDescriptorToDocumentableTranslatorTest.kt index 79e9f54867..a9466f2992 100644 --- a/plugins/base/src/test/kotlin/translators/DefaultDescriptorToDocumentableTranslatorTest.kt +++ b/plugins/base/src/test/kotlin/translators/DefaultDescriptorToDocumentableTranslatorTest.kt @@ -11,6 +11,7 @@ import org.junit.Assert import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test +import kotlin.test.assertNotNull class DefaultDescriptorToDocumentableTranslatorTest : BaseAbstractTest() { val configuration = dokkaConfiguration { @@ -22,6 +23,16 @@ class DefaultDescriptorToDocumentableTranslatorTest : BaseAbstractTest() { } } + @Suppress("DEPRECATION") // for includeNonPublic + val javaConfiguration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/main/java") + includeNonPublic = true + } + } + } + @Test fun `data class kdocs over generated methods`() { testInline( @@ -154,33 +165,6 @@ class DefaultDescriptorToDocumentableTranslatorTest : BaseAbstractTest() { } } - private sealed class TestSuite { - abstract val propertyName: String - - data class PropertyDoesntExist( - override val propertyName: String - ) : TestSuite() - - - data class PropertyExists( - override val propertyName: String, - val modifier: KotlinModifier, - val visibility: KotlinVisibility, - val additionalModifiers: Set - ) : TestSuite() - - data class FunctionDoesntExist( - override val propertyName: String, - ) : TestSuite() - - data class FunctionExists( - override val propertyName: String, - val modifier: KotlinModifier, - val visibility: KotlinVisibility, - val additionalModifiers: Set - ) : TestSuite() - } - private fun runTestSuitesAgainstGivenClasses(classlikes: List, testSuites: List>) { classlikes.zip(testSuites).forEach { (classlike, testSuites) -> testSuites.forEach { testSuite -> @@ -669,16 +653,6 @@ class DefaultDescriptorToDocumentableTranslatorTest : BaseAbstractTest() { } } - @Suppress("DEPRECATION") // for includeNonPublic - val javaConfiguration = dokkaConfiguration { - sourceSets { - sourceSet { - sourceRoots = listOf("src/main/java") - includeNonPublic = true - } - } - } - @Disabled // The compiler throws away annotations on unresolved types upstream @Test fun `Can annotate unresolved type`() { @@ -730,4 +704,62 @@ class DefaultDescriptorToDocumentableTranslatorTest : BaseAbstractTest() { } } } + + @Test + fun `should preserve regular functions that look like accessors, but are not accessors`() { + testInline( + """ + |/src/main/kotlin/A.kt + |package test + |class A { + | private var v: Int = 0 + | + | // not accessors because declared separately, just functions + | fun setV(new: Int) { v = new } + | fun getV(): Int = v + |} + """.trimIndent(), + configuration + ) { + 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?" + } + } + } + } +} + +private sealed class TestSuite { + abstract val propertyName: String + + data class PropertyDoesntExist( + override val propertyName: String + ) : TestSuite() + + + data class PropertyExists( + override val propertyName: String, + val modifier: KotlinModifier, + val visibility: KotlinVisibility, + val additionalModifiers: Set + ) : TestSuite() + + data class FunctionDoesntExist( + override val propertyName: String, + ) : TestSuite() + + data class FunctionExists( + override val propertyName: String, + val modifier: KotlinModifier, + val visibility: KotlinVisibility, + val additionalModifiers: Set + ) : TestSuite() } diff --git a/plugins/base/src/test/kotlin/translators/DefaultPsiToDocumentableTranslatorTest.kt b/plugins/base/src/test/kotlin/translators/DefaultPsiToDocumentableTranslatorTest.kt index 1213727d0b..b7873dfc17 100644 --- a/plugins/base/src/test/kotlin/translators/DefaultPsiToDocumentableTranslatorTest.kt +++ b/plugins/base/src/test/kotlin/translators/DefaultPsiToDocumentableTranslatorTest.kt @@ -1,5 +1,6 @@ package translators +import org.jetbrains.dokka.DokkaConfiguration import org.jetbrains.dokka.base.DokkaBase import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest import org.jetbrains.dokka.links.DRI @@ -8,6 +9,7 @@ import org.jetbrains.dokka.model.TypeConstructor import org.jetbrains.dokka.model.doc.Text import org.jetbrains.dokka.model.firstMemberOfType import org.jetbrains.dokka.plugability.DokkaPlugin +import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -259,4 +261,74 @@ class DefaultPsiToDocumentableTranslatorTest : BaseAbstractTest() { } } } + + @Test + fun `should preserve regular functions that look like accessors, but are not accessors`() { + testInline( + """ + |/src/main/java/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(), + configuration + ) { + documentablesMergingStage = { module -> + val testClass = module.packages.single().classlikes.single { it.name == "A" } + + val setterLookalike = testClass.functions.firstOrNull { it.name == "setA" } + Assertions.assertNotNull(setterLookalike) { + "Expected regular function not found, wrongly categorized as setter?" + } + + val getterLookalike = testClass.functions.firstOrNull { it.name == "getA" } + Assertions.assertNotNull(getterLookalike) { + "Expected regular function not found, wrongly categorized as getter?" + } + } + } + } + + @Test + fun `should not associate accessors with field because field is public api`() { + val configuration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + documentedVisibilities = setOf( + DokkaConfiguration.Visibility.PUBLIC, + DokkaConfiguration.Visibility.PROTECTED + ) + } + } + } + + testInline( + """ + |/src/test/A.java + |package test; + |public class A { + | protected int a = 1; + | public int getA() { return a; } + | public void setA(int a) { this.a = a; } + |} + """.trimIndent(), + configuration + ) { + documentablesMergingStage = { module -> + val testedClass = module.packages.single().classlikes.single { it.name == "A" } + val property = testedClass.properties.single { it.name == "a" } + + Assertions.assertNull(property.getter) + Assertions.assertNull(property.setter) + kotlin.test.assertEquals(2, testedClass.functions.size) + + kotlin.test.assertEquals("getA", testedClass.functions[0].name) + kotlin.test.assertEquals("setA", testedClass.functions[1].name) + } + } + } } 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 abb963a6e7..c7a4db35e4 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()) 32 else 31, indexPages.sumBy { it.elements.size }) + assertEquals(if (hasAdditionalFunction()) 31 else 30, indexPages.sumBy { it.elements.size }) } } @@ -163,4 +163,4 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { testTemplateMapInline(query) { operation(allPagesOfType().map { it.templateMap }) } -} \ No newline at end of file +}