KT-50292 - Implement vertical alignment of parameters (#2309)

* Implement vertical alignment (wrapping) of parameters for kt

* Add tests for params wrapping and extend matchers to check for classes

* Add distinguishable parameters block to kotlinAsJava, extract common logic

* Create a separate Kind for symbol function parameters

7 files changed, 276 insertions, 85 deletions

diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
index 8d258461..2906e8f2 100644
--- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
@@ -98,6 +98,21 @@ open class HtmlRenderer(
             }
             node.hasStyle(TextStyle.Span) -> span { childrenCallback() }
             node.dci.kind == ContentKind.Symbol -> div("symbol $additionalClasses") { childrenCallback() }
+            node.dci.kind == SymbolContentKind.Parameters -> {
+                span("parameters $additionalClasses") {
+                    childrenCallback()
+                }
+            }
+            node.dci.kind == SymbolContentKind.Parameter -> {
+                span("parameter $additionalClasses") {
+                    if (node.hasStyle(ContentStyle.Indented)) {
+                        // could've been done with CSS (padding-left, ::before, etc), 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.dci.kind == ContentKind.BriefComment -> div("brief $additionalClasses") { childrenCallback() }
             node.dci.kind == ContentKind.Cover -> div("cover $additionalClasses") { //TODO this can be removed
                 childrenCallback()
diff --git a/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt b/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
index 94af96e2..b6841323 100644
--- a/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
+++ b/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
@@ -174,6 +174,57 @@ interface JvmSignatureUtils {
                 parameters.flatMap { listOf(it.dri) + it.type.drisOfAllNestedBounds })
         return t.dri in allDris
     }
+
+    /**
+     * Builds a distinguishable [function] parameters block, so that it
+     * can be processed or custom rendered down the road.
+     *
+     * Resulting structure:
+     * ```
+     * SymbolContentKind.Parameters(style = wrapped) {
+     *     SymbolContentKind.Parameter(style = indented) { param, }
+     *     SymbolContentKind.Parameter(style = indented) { param, }
+     *     SymbolContentKind.Parameter(style = indented) { param }
+     * }
+     * ```
+     * Wrapping and indentation of parameters is applied conditionally, see [shouldWrapParams]
+     */
+    fun PageContentBuilder.DocumentableContentBuilder.parametersBlock(
+        function: DFunction, paramBuilder: PageContentBuilder.DocumentableContentBuilder.(DParameter) -> Unit
+    ) {
+        val shouldWrap = function.shouldWrapParams()
+        val parametersStyle = if (shouldWrap) setOf(ContentStyle.Wrapped) else emptySet()
+        val elementStyle = if (shouldWrap) setOf(ContentStyle.Indented) else emptySet()
+        group(kind = SymbolContentKind.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())
+            }
+        }
+    }
+
+    /**
+     * Determines whether parameters in a function (including constructor) should be wrapped
+     *
+     * Without wrapping:
+     * ```
+     * class SimpleClass(foo: String, bar: String) {}
+     * ```
+     * With wrapping:
+     * ```
+     * class SimpleClass(
+     *     foo: String,
+     *     bar: String,
+     *     baz: String
+     * )
+     * ```
+     */
+    private fun DFunction.shouldWrapParams() = this.parameters.size >= 3
 }
 
 sealed class AtStrategy
diff --git a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt
index 47e16e69..3f8c1703 100644
--- a/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt
+++ b/plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt
@@ -7,15 +7,13 @@ 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
 import org.jetbrains.dokka.model.TypeConstructor
 import org.jetbrains.dokka.model.properties.WithExtraProperties
-import org.jetbrains.dokka.pages.ContentKind
-import org.jetbrains.dokka.pages.ContentNode
-import org.jetbrains.dokka.pages.TextStyle
-import org.jetbrains.dokka.pages.TokenStyle
+import org.jetbrains.dokka.pages.*
 import org.jetbrains.dokka.plugability.DokkaContext
 import org.jetbrains.dokka.plugability.plugin
 import org.jetbrains.dokka.plugability.querySingle
@@ -179,19 +177,19 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
                         annotationsInline(pConstructor)
                         keyword("constructor")
                     }
-                    list(
-                        elements = pConstructor.parameters,
-                        prefix = "(",
-                        suffix = ")",
-                        separator = ", ",
-                        separatorStyles = mainStyles + TokenStyle.Punctuation,
-                        surroundingCharactersStyle = mainStyles + TokenStyle.Punctuation,
-                        sourceSets = pConstructor.sourceSets.toSet()
-                    ) {
-                        annotationsInline(it)
-                        text(it.name.orEmpty())
-                        operator(": ")
-                        signatureForProjection(it.type)
+
+                    // for primary constructor, opening and closing parentheses
+                    // should be present only if it has parameters. If there are
+                    // no parameters, it should result in `class Example`
+                    if (pConstructor.parameters.isNotEmpty()) {
+                        punctuation("(")
+                        parametersBlock(pConstructor) { param ->
+                            annotationsInline(param)
+                            text(param.name.orEmpty())
+                            operator(": ")
+                            signatureForProjection(param.type)
+                        }
+                        punctuation(")")
                     }
                 }
             }
@@ -283,17 +281,21 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
                     punctuation(".")
                 }
                 link(f.name, f.dri, styles = mainStyles + TokenStyle.Function)
+
+                // for a function, opening and closing parentheses must be present
+                // anyway, even if it has no parameters, resulting in `fun test(): R`
                 punctuation("(")
-                list(f.parameters,
-                    separatorStyles = mainStyles + TokenStyle.Punctuation) {
-                    annotationsInline(it)
-                    processExtraModifiers(it)
-                    text(it.name!!)
-                    operator(": ")
-                    signatureForProjection(it.type)
-                    it.extra[DefaultValue]?.run {
-                        operator(" = ")
-                        highlightValue(value)
+                if (f.parameters.isNotEmpty()) {
+                    parametersBlock(f) { param ->
+                        annotationsInline(param)
+                        processExtraModifiers(param)
+                        text(param.name!!)
+                        operator(": ")
+                        signatureForProjection(param.type)
+                        param.extra[DefaultValue]?.run {
+                            operator(" = ")
+                            highlightValue(value)
+                        }
                     }
                 }
                 punctuation(")")
diff --git a/plugins/base/src/main/resources/dokka/styles/style.css b/plugins/base/src/main/resources/dokka/styles/style.css
index ad28e895..cc8b6823 100644
--- a/plugins/base/src/main/resources/dokka/styles/style.css
+++ b/plugins/base/src/main/resources/dokka/styles/style.css
@@ -886,6 +886,10 @@ td.content {
     padding-bottom: 0;
 }
 
+.parameters.wrapped > .parameter {
+    display: block;
+}
+
 .table-row .with-platform-tabs .sourceset-depenent-content .brief {
     padding: 8px;
 }
diff --git a/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt b/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt
index 12160db8..508a0a36 100644
--- a/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt
+++ b/plugins/base/src/test/kotlin/content/signatures/SkippingParenthesisForConstructorsTest.kt
@@ -98,8 +98,13 @@ class ConstructorsSignaturesTest : BaseAbstractTest() {
                             group {
                                 +"class "
                                 link { +"SomeClass" }
-                                +"(a: "
-                                group { link { +"String" } }
+                                +"("
+                                group {
+                                    group {
+                                        +"a: "
+                                        group { link { +"String" } }
+                                    }
+                                }
                                 +")"
                             }
                         }
@@ -131,8 +136,13 @@ class ConstructorsSignaturesTest : BaseAbstractTest() {
                             group {
                                 +"class "
                                 link { +"SomeClass" }
-                                +"(a: " // TODO: Make sure if we still do not want to have "val" here
-                                group { link { +"String" } }
+                                +"("
+                                group {
+                                    group {
+                                        +"a: " // TODO: Make sure if we still do not want to have "val" here
+                                        group { link { +"String" } }
+                                    }
+                                }
                                 +")"
                             }
                         }
@@ -165,8 +175,13 @@ class ConstructorsSignaturesTest : BaseAbstractTest() {
                             group {
                                 +"class "
                                 link { +"SomeClass" }
-                                +"(a: "
-                                group { link { +"String" } }
+                                +"("
+                                group {
+                                    group {
+                                        +"a: "
+                                        group { link { +"String" } }
+                                    }
+                                }
                                 +")"
                             }
                         }
@@ -227,8 +242,13 @@ class ConstructorsSignaturesTest : BaseAbstractTest() {
                             group {
                                 +"class "
                                 link { +"SomeClass" }
-                                +"(a: "
-                                group { link { +"String" } }
+                                +"("
+                                group {
+                                    group {
+                                        +"a: "
+                                        group { link { +"String" } }
+                                    }
+                                }
                                 +")"
                             }
                             skipAllNotMatching()
@@ -243,9 +263,14 @@ class ConstructorsSignaturesTest : BaseAbstractTest() {
                                     group {
                                         +"fun "
                                         link { +"SomeClass" }
-                                        +"(a: "
+                                        +"("
                                         group {
-                                            link { +"String" }
+                                            group {
+                                                +"a: "
+                                                group {
+                                                    link { +"String" }
+                                                }
+                                            }
                                         }
                                         +")"
                                     }
diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
index 9ca6a5db..7ab0f663 100644
--- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
+++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
@@ -4,6 +4,7 @@ import org.jetbrains.dokka.DokkaSourceSetID
 import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
 import org.junit.jupiter.api.Test
 import utils.*
+import kotlin.test.assertFalse
 
 class SignatureTest : BaseAbstractTest() {
     private val configuration = dokkaConfiguration {
@@ -93,10 +94,12 @@ class SignatureTest : BaseAbstractTest() {
         ) {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/simple-fun.html").firstSignature().match(
-                    "fun ", A("simpleFun"), "(a: ", A("Int"),
-                    ", b: ", A("Boolean"), ", c: ", A("Any"),
-                    "): ", A("String"), Span(),
-                        ignoreSpanWithTokenStyle = true
+                    "fun ", A("simpleFun"), "(", Parameters(
+                        Parameter("a: ", A("Int"), ","),
+                        Parameter("b: ", A("Boolean"), ","),
+                        Parameter("c: ", A("Any")),
+                    ), "): ", A("String"), Span(),
+                    ignoreSpanWithTokenStyle = true
                 )
             }
         }
@@ -114,9 +117,10 @@ class SignatureTest : BaseAbstractTest() {
         ) {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/simple-fun.html").firstSignature().match(
-                    "fun ", A("simpleFun"), "(a: (", A("Int"),
-                    ") -> ", A("String"), "): ", A("String"), Span(),
-                        ignoreSpanWithTokenStyle = true
+                    "fun ", A("simpleFun"), "(", Parameters(
+                        Parameter("a: (", A("Int"), ") -> ", A("String")),
+                    ),"): ", A("String"), Span(),
+                    ignoreSpanWithTokenStyle = true
                 )
             }
         }
@@ -174,9 +178,11 @@ class SignatureTest : BaseAbstractTest() {
         ) {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/simple-fun.html").firstSignature().match(
-                    "inline suspend fun <", A("T"), " : ", A("String"), "> ", A("simpleFun"),
-                    "(a: ", A("Int"), ", b: ", A("String"), "): ", A("T"), Span(),
-                        ignoreSpanWithTokenStyle = true
+                    "inline suspend fun <", A("T"), " : ", A("String"), "> ", A("simpleFun"), "(", Parameters(
+                        Parameter("a: ", A("Int"), ","),
+                        Parameter("b: ", A("String")),
+                    ), "): ", A("T"), Span(),
+                    ignoreSpanWithTokenStyle = true
                 )
             }
         }
@@ -194,8 +200,10 @@ class SignatureTest : BaseAbstractTest() {
         ) {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/simple-fun.html").firstSignature().match(
-                    "fun ", A("simpleFun"), "(vararg params: ", A("Int"), ")", Span(),
-                        ignoreSpanWithTokenStyle = true
+                    "fun ", A("simpleFun"), "(", Parameters(
+                        Parameter("vararg params: ", A("Int")),
+                    ), ")", Span(),
+                    ignoreSpanWithTokenStyle = true
                 )
             }
         }
@@ -655,9 +663,11 @@ class SignatureTest : BaseAbstractTest() {
             pluginOverrides = listOf(writerPlugin)
         ) {
             renderingStage = { _, _ ->
-                writerPlugin.writer.renderedContent("root/kotlinAsJavaPlugin/-a-b-c/some-fun.html").signature().first().match(
-                    "fun ", A("someFun"), "(xd: ", A("XD"), "<", A("Int"),
-                    ", ", A("String"), ">):", A("Int"), Span(),
+                writerPlugin.writer.renderedContent("root/kotlinAsJavaPlugin/-a-b-c/some-fun.html").signature().first()
+                    .match(
+                        "fun ", A("someFun"), "(", Parameters(
+                            Parameter("xd: ", A("XD"), "<", A("Int"), ", ", A("String"), ">"),
+                        ), "):", A("Int"), Span(),
                         ignoreSpanWithTokenStyle = true
                 )
             }
@@ -666,8 +676,6 @@ class SignatureTest : BaseAbstractTest() {
 
     @Test
     fun `generic constructor params`() {
-
-
         val writerPlugin = TestOutputWriterPlugin()
 
         testInline(
@@ -694,13 +702,40 @@ class SignatureTest : BaseAbstractTest() {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/-generic-class/-generic-class.html").signature().zip(
                     listOf(
-                        arrayOf("fun <", A("T"), "> ", A("GenericClass"), "(x: ", A("T"), ")", Span()),
-                        arrayOf("fun ", A("GenericClass"), "(x: ", A("Int"), ", y: ", A("String"), ")", Span()),
-                        arrayOf("fun <", A("T"), "> ", A("GenericClass"), "(x: ", A("Int"), ", y: ", A("List"), "<", A("T"), ">)", Span()),
-                        arrayOf("fun ", A("GenericClass"), "(x: ", A("Boolean"), ", y: ", A("Int"), ", z:", A("String"), ")", Span()),
-                        arrayOf("fun <", A("T"), "> ", A("GenericClass"), "(x: ", A("List"), "<", A("Comparable"),
-                            "<", A("Lazy"), "<", A("T"), ">>>?)", Span()),
-                        arrayOf("fun ", A("GenericClass"), "(x: ", A("Int"), ")", Span()),
+                        arrayOf(
+                            "fun <", A("T"), "> ", A("GenericClass"), "(", Parameters(
+                                Parameter("x: ", A("T"))
+                            ), ")", Span()
+                        ),
+                        arrayOf(
+                            "fun ", A("GenericClass"), "(", Parameters(
+                                Parameter("x: ", A("Int"), ", "),
+                                Parameter("y: ", A("String"))
+                            ), ")", Span()
+                        ),
+                        arrayOf(
+                            "fun <", A("T"), "> ", A("GenericClass"), "(", Parameters(
+                                Parameter("x: ", A("Int"), ", "),
+                                Parameter("y: ", A("List"), "<", A("T"), ">")
+                            ), ")", Span()
+                        ),
+                        arrayOf(
+                            "fun ", A("GenericClass"), "(", Parameters(
+                                Parameter("x: ", A("Boolean"), ", "),
+                                Parameter("y: ", A("Int"), ", "),
+                                Parameter("z:", A("String"))
+                            ), ")", Span()
+                        ),
+                        arrayOf(
+                            "fun <", A("T"), "> ", A("GenericClass"), "(", Parameters(
+                                Parameter("x: ", A("List"), "<", A("Comparable"), "<", A("Lazy"), "<", A("T"), ">>>?")
+                            ), ")", Span()
+                        ),
+                        arrayOf(
+                            "fun ", A("GenericClass"), "(", Parameters(
+                                Parameter("x: ", A("Int"))
+                            ), ")", Span()
+                        ),
                     )
                 ).forEach {
                     it.first.match(*it.second, ignoreSpanWithTokenStyle = true)
@@ -721,8 +756,10 @@ class SignatureTest : BaseAbstractTest() {
         ) {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/simple-fun.html").firstSignature().match(
-                    "fun", A("simpleFun"), "(int: ", A("Int"), " = 1, string: ", A("String"),
-                            " = \"string\"): ", A("String"), Span(),
+                    "fun", A("simpleFun"), "(", Parameters(
+                        Parameter("int: ", A("Int"), " = 1,"),
+                        Parameter("string: ", A("String"), " = \"string\"")
+                    ), "): ", A("String"), Span(),
                         ignoreSpanWithTokenStyle = true
                 )
             }
@@ -730,6 +767,53 @@ class SignatureTest : BaseAbstractTest() {
     }
 
     @Test
+    fun `fun with single param should NOT have any wrapped or indented parameters`() {
+        val source = source("fun assertNoIndent(int: Int): String = \"\"")
+        val writerPlugin = TestOutputWriterPlugin()
+
+        testInline(
+            source,
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
+        ) {
+            renderingStage = { _, _ ->
+                val signature = writerPlugin.writer.renderedContent("root/example/assert-no-indent.html").firstSignature()
+                signature.match(
+                    "fun", A("assertNoIndent"), "(", Parameters(
+                        Parameter("int: ", A("Int")),
+                    ), "): ", A("String"), Span(),
+                    ignoreSpanWithTokenStyle = true
+                )
+                assertFalse { signature.select("span.parameters").single().hasClass("wrapped") }
+                assertFalse { signature.select("span.parameters > span.parameter").single().hasClass("indented") }
+            }
+        }
+    }
+
+    @Test
+    fun `fun with many params should have wrapped and indented parameters`() {
+        val source = source("fun assertParamsIndent(int: Int, string: String, long: Long): String = \"\"")
+        val writerPlugin = TestOutputWriterPlugin()
+
+        testInline(
+            source,
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
+        ) {
+            renderingStage = { _, _ ->
+                writerPlugin.writer.renderedContent("root/example/assert-params-indent.html").firstSignature().match(
+                    "fun", A("assertParamsIndent"), "(", Parameters(
+                        Parameter("int: ", A("Int"), ",").withClasses("indented"),
+                        Parameter("string: ", A("String"), ",").withClasses("indented"),
+                        Parameter("long: ", A("Long")).withClasses("indented")
+                    ).withClasses("wrapped"), "): ", A("String"), Span(),
+                    ignoreSpanWithTokenStyle = true
+                )
+            }
+        }
+    }
+
+    @Test
     fun `const val with default values`() {
         val source = source("const val simpleVal = 1")
         val writerPlugin = TestOutputWriterPlugin()
diff --git a/plugins/base/src/test/kotlin/utils/contentUtils.kt b/plugins/base/src/test/kotlin/utils/contentUtils.kt
index be796c89..66e5ef53 100644
--- a/plugins/base/src/test/kotlin/utils/contentUtils.kt
+++ b/plugins/base/src/test/kotlin/utils/contentUtils.kt
@@ -38,19 +38,24 @@ fun ContentMatcherBuilder<*>.bareSignature(
     +("${keywords.joinToString("") { "$it " }}fun ")
     link { +name }
     +"("
-    params.forEachIndexed { id, (n, t) ->
+    if (params.isNotEmpty()) {
+        group {
+            params.forEachIndexed { id, (n, t) ->
+                group {
+                    t.annotations.forEach {
+                        unwrapAnnotation(it)
+                    }
+                    t.keywords.forEach {
+                        +it
+                    }
 
-        t.annotations.forEach {
-            unwrapAnnotation(it)
-        }
-        t.keywords.forEach {
-            +it
+                    +"$n: "
+                    group { link { +(t.type) } }
+                    if (id != params.lastIndex)
+                        +", "
+                }
+            }
         }
-
-        +"$n: "
-        group { link { +(t.type) } }
-        if (id != params.lastIndex)
-            +", "
     }
     +")"
     if (returnType != null) {
@@ -101,19 +106,24 @@ fun ContentMatcherBuilder<*>.bareSignatureWithReceiver(
     +"."
     link { +name }
     +"("
-    params.forEachIndexed { id, (n, t) ->
+    if (params.isNotEmpty()) {
+        group {
+            params.forEachIndexed { id, (n, t) ->
+                group {
+                    t.annotations.forEach {
+                        unwrapAnnotation(it)
+                    }
+                    t.keywords.forEach {
+                        +it
+                    }
 
-        t.annotations.forEach {
-            unwrapAnnotation(it)
-        }
-        t.keywords.forEach {
-            +it
+                    +"$n: "
+                    group { link { +(t.type) } }
+                    if (id != params.lastIndex)
+                        +", "
+                }
+            }
         }
-
-        +"$n: "
-        group { link { +(t.type) } }
-        if (id != params.lastIndex)
-            +", "
     }
     +")"
     if (returnType != null) {