Improve annotations rendering (#2045)

omit parentheses if there are no parameters
add quotes for string literals into annotations

4 files changed, 45 insertions, 35 deletions

diff --git a/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt b/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
index f83eb4d8..a2624da0 100644
--- a/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
+++ b/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
@@ -69,20 +69,19 @@ interface JvmSignatureUtils {
             is Never -> Unit
         }
         link(a.dri.classNames!!, a.dri)
-        text("(")
-        a.params.entries.forEachIndexed { i, it ->
-            group(styles = setOf(TextStyle.BreakableAfter)) {
-                text(it.key + " = ")
-                when (renderAtStrategy) {
-                    is All -> All
-                    is Never, is OnlyOnce -> Never
-                }.let { strategy ->
-                    valueToSignature(it.value, strategy, listBrackets, classExtension)
-                }
-                if (i != a.params.entries.size - 1) text(", ")
+        val isNoWrappedBrackets = a.params.entries.isEmpty() && renderAtStrategy is OnlyOnce
+        listParams(
+            a.params.entries,
+            if (isNoWrappedBrackets) null else Pair('(', ')')
+        ) {
+            text(it.key + " = ")
+            when (renderAtStrategy) {
+                is All -> All
+                is Never, is OnlyOnce -> Never
+            }.let { strategy ->
+                valueToSignature(it.value, strategy, listBrackets, classExtension)
             }
         }
-        text(")")
     }
 
     private fun PageContentBuilder.DocumentableContentBuilder.valueToSignature(
@@ -93,20 +92,29 @@ interface JvmSignatureUtils {
     ): Unit = when (a) {
         is AnnotationValue -> toSignatureString(a.annotation, renderAtStrategy, listBrackets, classExtension)
         is ArrayValue -> {
-            text(listBrackets.first.toString())
-            a.value.forEachIndexed { i, it ->
-                group(styles = setOf(TextStyle.BreakableAfter)) {
-                    valueToSignature(it, renderAtStrategy, listBrackets, classExtension)
-                    if (i != a.value.size - 1) text(", ")
-                }
-            }
-            text(listBrackets.second.toString())
+            listParams(a.value, listBrackets) { valueToSignature(it, renderAtStrategy, listBrackets, classExtension) }
         }
         is EnumValue -> link(a.enumName, a.enumDri)
         is ClassValue -> link(a.className + classExtension, a.classDRI)
+        is StringValue -> group(styles = setOf(TextStyle.Breakable)) { text( "\"${a.text()}\"") }
         is LiteralValue -> group(styles = setOf(TextStyle.Breakable)) { text(a.text()) }
     }
 
+    private fun<T> PageContentBuilder.DocumentableContentBuilder.listParams(
+        params: Collection<T>,
+        listBrackets: Pair<Char, Char>?,
+        outFn: PageContentBuilder.DocumentableContentBuilder.(T) -> Unit
+    ) {
+        listBrackets?.let{ text(it.first.toString()) }
+        params.forEachIndexed { i, it ->
+            group(styles = setOf(TextStyle.BreakableAfter)) {
+                this.outFn(it)
+                if (i != params.size - 1) text(", ")
+            }
+        }
+        listBrackets?.let{ text(it.second.toString()) }
+    }
+
     fun PageContentBuilder.DocumentableContentBuilder.annotationsBlockWithIgnored(
         d: AnnotationTarget,
         ignored: Set<Annotations.Annotation>,
diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
index 5605d867..35590d52 100644
--- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
+++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
@@ -270,7 +270,7 @@ class SignatureTest : BaseAbstractTest() {
             renderingStage = { _, _ ->
                 writerPlugin.writer.renderedContent("root/example/simple-fun.html").firstSignature().match(
                     Div(
-                        Div("@", A("Marking"), "()")
+                        Div("@", A("Marking"))
                     ),
                     "fun ", A("simpleFun"),
                     "(): ", A("String"), Span()
@@ -309,8 +309,8 @@ class SignatureTest : BaseAbstractTest() {
                     .firstSignature()
                     .match(
                         Div(
-                            Div("@", A("Marking"), "(", Span("msg = ", Span("Nenya")), Wbr, ")"),
-                            Div("@", A("Marking2"), "(", Span("int = ", Span("1")), Wbr, ")")
+                           Div("@", A("Marking"), "(", Span("msg = ", Span("\"Nenya\"")), Wbr, ")"),
+                           Div("@", A("Marking2"), "(", Span("int = ", Span("1")), Wbr, ")")
                         ),
                         "fun ", A("simpleFun"),
                         "(): ", A("String"), Span()
@@ -346,9 +346,9 @@ class SignatureTest : BaseAbstractTest() {
                         Div(
                             "@", A("Marking"), "(", Span(
                                 "msg = [",
-                                Span(Span("Nenya"), ", "), Wbr,
-                                Span(Span("Vilya"), ", "), Wbr,
-                                Span(Span("Narya")), Wbr, "]"
+                                Span(Span("\"Nenya\""), ", "), Wbr,
+                                Span(Span("\"Vilya\""), ", "), Wbr,
+                                Span(Span("\"Narya\"")), Wbr, "]"
                             ), Wbr, ")"
                         )
                     ),
@@ -457,7 +457,7 @@ class SignatureTest : BaseAbstractTest() {
                 writerPlugin.writer.renderedContent("root/example/index.html").signature().first().match(
                     Div(
                         Div(
-                            "@", A("SomeAnnotation"), "()"
+                            "@", A("SomeAnnotation")
                         )
                     ),
                     "typealias ", A("PlainTypealias"), " = ", A("Int"), Span()
diff --git a/plugins/base/src/test/kotlin/utils/contentUtils.kt b/plugins/base/src/test/kotlin/utils/contentUtils.kt
index 5d8673d0..be796c89 100644
--- a/plugins/base/src/test/kotlin/utils/contentUtils.kt
+++ b/plugins/base/src/test/kotlin/utils/contentUtils.kt
@@ -242,14 +242,16 @@ fun ContentMatcherBuilder<*>.unwrapAnnotation(elem: Map.Entry<String, Set<String
     group {
         +"@"
         link { +elem.key }
-        +"("
-        elem.value.forEach {
-            group {
-                +("$it = ")
-                skipAllNotMatching()
+        if(elem.value.isNotEmpty()) {
+            +"("
+            elem.value.forEach {
+                group {
+                    +("$it = ")
+                    skipAllNotMatching()
+                }
             }
+            +")"
         }
-        +")"
     }
 }
 
diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocClasslikeTemplateMapTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocClasslikeTemplateMapTest.kt
index 0b1eab7f..19ef6ae7 100644
--- a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocClasslikeTemplateMapTest.kt
+++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocClasslikeTemplateMapTest.kt
@@ -149,13 +149,13 @@ internal class JavadocClasslikeTemplateMapTest : AbstractJavadocTemplateMapTest(
             val map = allPagesOfType<JavadocClasslikePageNode>().first { it.name == "TestClass" }.templateMap
             assertEquals("TestClass", map["name"])
             val signature = assertIsInstance<Map<String, Any?>>(map["signature"])
-            assertEquals("@<a href=Author.html>Author</a>(name = Benjamin Franklin)", signature["annotations"])
+            assertEquals("@<a href=Author.html>Author</a>(name = \"Benjamin Franklin\")", signature["annotations"])
 
             val methods = assertIsInstance<Map<Any, Any?>>(map["methods"])
             val ownMethods = assertIsInstance<List<*>>(methods["own"])
             val method = assertIsInstance<Map<String, Any?>>(ownMethods.single())
             val methodSignature = assertIsInstance<Map<String, Any?>>(method["signature"])
-            assertEquals("@<a href=Author.html>Author</a>(name = Franklin D. Roosevelt)", methodSignature["annotations"])
+            assertEquals("@<a href=Author.html>Author</a>(name = \"Franklin D. Roosevelt\")", methodSignature["annotations"])
         }
     }