From 0150acca7b8b40823a268af56b28ed591212d749 Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 13 Jan 2022 17:26:57 +0300 Subject: [PATCH 1/6] Implement vertical alignment (wrapping) of parameters for kt --- core/api/core.api | 1 + .../kotlin/renderers/html/HtmlRenderer.kt | 15 +++++++ .../signatures/KotlinSignatureProvider.kt | 40 +++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/core/api/core.api b/core/api/core.api index 53a0ab3f0b..e992f14d1f 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -3749,6 +3749,7 @@ public final class org/jetbrains/dokka/pages/ContentKind : java/lang/Enum, org/j public static final field Inheritors Lorg/jetbrains/dokka/pages/ContentKind; public static final field Main Lorg/jetbrains/dokka/pages/ContentKind; public static final field Packages Lorg/jetbrains/dokka/pages/ContentKind; + public static final field Parameter Lorg/jetbrains/dokka/pages/ContentKind; public static final field Parameters Lorg/jetbrains/dokka/pages/ContentKind; public static final field Properties Lorg/jetbrains/dokka/pages/ContentKind; public static final field Sample Lorg/jetbrains/dokka/pages/ContentKind; diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt index 2906e8f277..1671f2b205 100644 --- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt +++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt @@ -89,6 +89,21 @@ open class HtmlRenderer( childrenCallback() if (node.hasStyle(TextStyle.Monospace)) copyButton() } + node.dci.kind == ContentKind.Parameters -> { + span("parameters $additionalClasses") { + childrenCallback() + } + } + node.dci.kind == ContentKind.Parameter -> { + span("parameter $additionalClasses") { + if (node.hasStyle(ContentStyle.Indented)) { + // could've been done with CSS (padding-left), but the indent needs to + // consist of physical spaces, otherwise select and copy won't work properly + repeat(4) { consumer.onTagContentEntity(Entities.nbsp) } + } + childrenCallback() + } + } node.hasStyle(TextStyle.BreakableAfter) -> { span { childrenCallback() } wbr { } diff --git a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt index 3f8c1703a8..07f83bc3e6 100644 --- a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt +++ b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt @@ -306,6 +306,26 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog } } + private fun DocumentableContentBuilder.parametersBlock( + function: DFunction, paramBuilder: DocumentableContentBuilder.(DParameter) -> Unit + ) { + val parametersStyle = if (function.shouldWrapParams()) setOf(ContentStyle.Wrapped) else emptySet() + val elementStyle = if (function.shouldWrapParams()) setOf(ContentStyle.Indented) else emptySet() + group(kind = ContentKind.Parameters, styles = parametersStyle) { + function.parameters.dropLast(1).forEach { + group(kind = SymbolContentKind.Parameter, styles = elementStyle) { + paramBuilder(it) + keyword(", ") + } + } + group(kind = SymbolContentKind.Parameter, styles = elementStyle) { + paramBuilder(function.parameters.last()) + } + } + } + + private fun DFunction.shouldWrapParams() = this.parameters.size >= FUNCTION_PARAMETERS_WRAP_THRESHOLD + private fun DFunction.documentReturnType() = when { this.isConstructor -> false this.type is TypeConstructor && (this.type as TypeConstructor).dri == DriOfUnit -> false @@ -437,6 +457,26 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog operator(" -> ") signatureForProjection(args.last()) } + + private companion object { + /** + * Number of parameters in a function (including constructor) after + * which the parameters should be wrapped + * ``` + * class SimpleClass(foo: String, bar: String, baz: String) {} + * ``` + * After wrapping: + * ``` + * class SimpleClass( + * foo: String, + * bar: String, + * baz: String, + * qux: String + * ) + * ``` + */ + private const val FUNCTION_PARAMETERS_WRAP_THRESHOLD = 4 + } } private fun PrimitiveJavaType.translateToKotlin() = GenericTypeConstructor( From cd0f5dc960a5b458f241be4d0ad87ef948791333 Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 13 Jan 2022 21:37:45 +0300 Subject: [PATCH 2/6] Add tests for params wrapping and extend matchers to check for classes --- .../main/kotlin/signatures/KotlinSignatureProvider.kt | 9 ++++----- plugins/base/src/test/kotlin/signatures/SignatureTest.kt | 3 +++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt index 07f83bc3e6..f54ebc6f41 100644 --- a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt +++ b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt @@ -315,7 +315,7 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog function.parameters.dropLast(1).forEach { group(kind = SymbolContentKind.Parameter, styles = elementStyle) { paramBuilder(it) - keyword(", ") + punctuation(", ") } } group(kind = SymbolContentKind.Parameter, styles = elementStyle) { @@ -463,19 +463,18 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog * Number of parameters in a function (including constructor) after * which the parameters should be wrapped * ``` - * class SimpleClass(foo: String, bar: String, baz: String) {} + * class SimpleClass(foo: String, bar: String) {} * ``` * After wrapping: * ``` * class SimpleClass( * foo: String, * bar: String, - * baz: String, - * qux: String + * baz: String * ) * ``` */ - private const val FUNCTION_PARAMETERS_WRAP_THRESHOLD = 4 + private const val FUNCTION_PARAMETERS_WRAP_THRESHOLD = 3 } } diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt index 7ab0f66315..2d14c772a1 100644 --- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt +++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt @@ -831,4 +831,7 @@ class SignatureTest : BaseAbstractTest() { } } } + + private class Parameters(vararg matchers: Any) : Tag("span", *matchers, expectedClasses = listOf("parameters")) + private class Parameter(vararg matchers: Any) : Tag("span", *matchers, expectedClasses = listOf("parameter")) } From 8ac9bd35addddb9a90142689bb46f207367e5e68 Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 13 Jan 2022 23:51:15 +0300 Subject: [PATCH 3/6] Add distinguishable parameters block to kotlinAsJava, extract common logic --- plugins/base/src/test/kotlin/signatures/SignatureTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt index 2d14c772a1..7ab0f66315 100644 --- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt +++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt @@ -831,7 +831,4 @@ class SignatureTest : BaseAbstractTest() { } } } - - private class Parameters(vararg matchers: Any) : Tag("span", *matchers, expectedClasses = listOf("parameters")) - private class Parameter(vararg matchers: Any) : Tag("span", *matchers, expectedClasses = listOf("parameter")) } From 524688eadb536a554bab8cfaa108c0197f71481c Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Mon, 17 Jan 2022 17:59:47 +0300 Subject: [PATCH 4/6] Enhance generated primary constructor signature for html format --- .../signatures/KotlinSignatureProvider.kt | 22 +++++++++++++++- .../SkippingParenthesisForConstructorsTest.kt | 9 +++++-- .../test/kotlin/signatures/SignatureTest.kt | 26 +++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt index f54ebc6f41..21efa93b6a 100644 --- a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt +++ b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt @@ -2,12 +2,12 @@ package org.jetbrains.dokka.base.signatures import org.jetbrains.dokka.DokkaConfiguration.DokkaSourceSet import org.jetbrains.dokka.Platform +import org.jetbrains.dokka.analysis.DescriptorDocumentableSource import org.jetbrains.dokka.base.DokkaBase import org.jetbrains.dokka.base.signatures.KotlinSignatureUtils.dri import org.jetbrains.dokka.base.signatures.KotlinSignatureUtils.driOrNull import org.jetbrains.dokka.base.transformers.pages.comments.CommentsToContentConverter import org.jetbrains.dokka.base.translators.documentables.PageContentBuilder -import org.jetbrains.dokka.base.translators.documentables.PageContentBuilder.DocumentableContentBuilder import org.jetbrains.dokka.links.* import org.jetbrains.dokka.model.* import org.jetbrains.dokka.model.Nullable @@ -18,6 +18,8 @@ import org.jetbrains.dokka.plugability.DokkaContext import org.jetbrains.dokka.plugability.plugin import org.jetbrains.dokka.plugability.querySingle import org.jetbrains.dokka.utilities.DokkaLogger +import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi +import org.jetbrains.kotlin.psi.KtParameter import kotlin.text.Typography.nbsp class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLogger) @@ -182,12 +184,23 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog // should be present only if it has parameters. If there are // no parameters, it should result in `class Example` if (pConstructor.parameters.isNotEmpty()) { + val parameterPropertiesByName = c.properties + .filter { it.isAlsoParameter(sourceSet) } + .associateBy { it.name } + punctuation("(") parametersBlock(pConstructor) { param -> annotationsInline(param) + parameterPropertiesByName[param.name]?.let { property -> + property.setter?.let { keyword("var ") } ?: keyword("val ") + } text(param.name.orEmpty()) operator(": ") signatureForProjection(param.type) + param.extra[DefaultValue]?.let { + operator(" = ") + highlightValue(it.value) + } } punctuation(")") } @@ -207,6 +220,13 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog } } + /** + * An example would be a primary constructor `class A(val s: String)`, + * where `s` is both a function parameter and a property + */ + private fun DProperty.isAlsoParameter(sourceSet: DokkaSourceSet) = + (this.sources[sourceSet] as? DescriptorDocumentableSource)?.descriptor?.findPsi() is KtParameter + private fun propertySignature(p: DProperty) = p.sourceSets.map { contentBuilder.contentFor( diff --git a/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt b/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt index 508a0a3693..b6fc4e6ba1 100644 --- a/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt +++ b/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt @@ -122,7 +122,7 @@ class ConstructorsSignaturesTest : BaseAbstractTest() { |/src/main/kotlin/test/source.kt |package test | - |class SomeClass(val a: String) + |class SomeClass(val a: String, var i: Int) | """.trimIndent(), testConfiguration ) { @@ -139,8 +139,13 @@ class ConstructorsSignaturesTest : BaseAbstractTest() { +"(" group { group { - +"a: " // TODO: Make sure if we still do not want to have "val" here + +"val a: " group { link { +"String" } } + +", " + } + group { + +"var i: " + group { link { +"Int" } } } } +")" diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt index 7ab0f66315..37595defdd 100644 --- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt +++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt @@ -744,6 +744,32 @@ class SignatureTest : BaseAbstractTest() { } } + @Test + fun `complete primary constructor with properties`() { + val writerPlugin = TestOutputWriterPlugin() + + testInline( + """ + |/src/main/kotlin/common/Test.kt + |package example + | + |class PrimaryConstructorClass(val x: Int, var s: String) { } + """.trimMargin(), + configuration, + pluginOverrides = listOf(writerPlugin) + ) { + renderingStage = { _, _ -> + writerPlugin.writer.renderedContent("root/example/-primary-constructor-class/index.html").firstSignature().match( + // In `` expression, an empty `` is present for some reason + Span("class "), A("PrimaryConstructorClass"), Span("<"), Span(), A("T"), Span(">"), Span("("), Parameters( + Parameter(Span("val "), "x", Span(": "), A("Int"), Span(",")), + Parameter(Span("var "), "s", Span(": "), A("String")) + ), Span(")"), Span(), + ) + } + } + } + @Test fun `fun with default values`() { val source = source("fun simpleFun(int: Int = 1, string: String = \"string\"): String = \"\"") From c579379a43eed166c58eed4d214782a1caca417d Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Mon, 17 Jan 2022 18:11:02 +0300 Subject: [PATCH 5/6] Correct test naming to show intention more --- plugins/base/src/test/kotlin/signatures/SignatureTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt index 37595defdd..ddf1c89294 100644 --- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt +++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt @@ -745,7 +745,7 @@ class SignatureTest : BaseAbstractTest() { } @Test - fun `complete primary constructor with properties`() { + fun `primary constructor with properties check for all tokens`() { val writerPlugin = TestOutputWriterPlugin() testInline( From e48a9afe0924b8cff34c4e9301631eb3fc09715e Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 27 Jan 2022 13:13:24 +0300 Subject: [PATCH 6/6] Resolve rebase conflicts after squash --- core/api/core.api | 1 - .../kotlin/renderers/html/HtmlRenderer.kt | 15 ------- .../signatures/KotlinSignatureProvider.kt | 39 ------------------- 3 files changed, 55 deletions(-) diff --git a/core/api/core.api b/core/api/core.api index e992f14d1f..53a0ab3f0b 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -3749,7 +3749,6 @@ public final class org/jetbrains/dokka/pages/ContentKind : java/lang/Enum, org/j public static final field Inheritors Lorg/jetbrains/dokka/pages/ContentKind; public static final field Main Lorg/jetbrains/dokka/pages/ContentKind; public static final field Packages Lorg/jetbrains/dokka/pages/ContentKind; - public static final field Parameter Lorg/jetbrains/dokka/pages/ContentKind; public static final field Parameters Lorg/jetbrains/dokka/pages/ContentKind; public static final field Properties Lorg/jetbrains/dokka/pages/ContentKind; public static final field Sample Lorg/jetbrains/dokka/pages/ContentKind; diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt index 1671f2b205..2906e8f277 100644 --- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt +++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt @@ -89,21 +89,6 @@ open class HtmlRenderer( childrenCallback() if (node.hasStyle(TextStyle.Monospace)) copyButton() } - node.dci.kind == ContentKind.Parameters -> { - span("parameters $additionalClasses") { - childrenCallback() - } - } - node.dci.kind == ContentKind.Parameter -> { - span("parameter $additionalClasses") { - if (node.hasStyle(ContentStyle.Indented)) { - // could've been done with CSS (padding-left), but the indent needs to - // consist of physical spaces, otherwise select and copy won't work properly - repeat(4) { consumer.onTagContentEntity(Entities.nbsp) } - } - childrenCallback() - } - } node.hasStyle(TextStyle.BreakableAfter) -> { span { childrenCallback() } wbr { } diff --git a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt index 21efa93b6a..8d97701d6f 100644 --- a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt +++ b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt @@ -326,26 +326,6 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog } } - private fun DocumentableContentBuilder.parametersBlock( - function: DFunction, paramBuilder: DocumentableContentBuilder.(DParameter) -> Unit - ) { - val parametersStyle = if (function.shouldWrapParams()) setOf(ContentStyle.Wrapped) else emptySet() - val elementStyle = if (function.shouldWrapParams()) setOf(ContentStyle.Indented) else emptySet() - group(kind = ContentKind.Parameters, styles = parametersStyle) { - function.parameters.dropLast(1).forEach { - group(kind = SymbolContentKind.Parameter, styles = elementStyle) { - paramBuilder(it) - punctuation(", ") - } - } - group(kind = SymbolContentKind.Parameter, styles = elementStyle) { - paramBuilder(function.parameters.last()) - } - } - } - - private fun DFunction.shouldWrapParams() = this.parameters.size >= FUNCTION_PARAMETERS_WRAP_THRESHOLD - private fun DFunction.documentReturnType() = when { this.isConstructor -> false this.type is TypeConstructor && (this.type as TypeConstructor).dri == DriOfUnit -> false @@ -477,25 +457,6 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog operator(" -> ") signatureForProjection(args.last()) } - - private companion object { - /** - * Number of parameters in a function (including constructor) after - * which the parameters should be wrapped - * ``` - * class SimpleClass(foo: String, bar: String) {} - * ``` - * After wrapping: - * ``` - * class SimpleClass( - * foo: String, - * bar: String, - * baz: String - * ) - * ``` - */ - private const val FUNCTION_PARAMETERS_WRAP_THRESHOLD = 3 - } } private fun PrimitiveJavaType.translateToKotlin() = GenericTypeConstructor(