From 75f572b271c5959bd6fab0b51cef792fa403ea83 Mon Sep 17 00:00:00 2001
From: Andrzej Ratajczak <32793002+BarkingBad@users.noreply.github.com>
Date: Thu, 8 Oct 2020 20:25:06 +0200
Subject: Fix multiline links in javadoc and wrong linebreaking of <pre> bodies
 (#1518)

* Fix multiline links in javadoc

* Fix wrong linebreaking of <pre> bodies

* Use included static values for external links

Co-authored-by: Marcin Aman <marcin.aman@gmail.com>
---
 .../main/kotlin/translators/psi/JavadocParser.kt   | 73 +++++++++++++--------
 .../test/kotlin/translators/JavadocParserTest.kt   | 23 +++++++
 .../javadoc/AbstractJavadocTemplateMapTest.kt      |  7 +-
 .../dokka/javadoc/location/JavadocLinkingTest.kt   | 74 ++++++++++++++++++++++
 .../dokka/javadoc/location/JavadocLocationTest.kt  | 10 +--
 5 files changed, 148 insertions(+), 39 deletions(-)
 create mode 100644 plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLinkingTest.kt

(limited to 'plugins')

diff --git a/plugins/base/src/main/kotlin/translators/psi/JavadocParser.kt b/plugins/base/src/main/kotlin/translators/psi/JavadocParser.kt
index 905d1898..9ef5ea38 100644
--- a/plugins/base/src/main/kotlin/translators/psi/JavadocParser.kt
+++ b/plugins/base/src/main/kotlin/translators/psi/JavadocParser.kt
@@ -41,7 +41,7 @@ class JavadocParser(
                 )
                 "throws" -> Throws(wrapTagIfNecessary(convertJavadocElements(tag.contentElements())), tag.text)
                 "return" -> Return(wrapTagIfNecessary(convertJavadocElements(tag.contentElements())))
-                "author" -> Author(wrapTagIfNecessary(convertJavadocElements(tag.contentElements())))
+                "author" -> Author(wrapTagIfNecessary(convertJavadocElements(tag.authorContentElements()))) // Workaround: PSI returns first word after @author tag as a `DOC_TAG_VALUE_ELEMENT`, then the rest as a `DOC_COMMENT_DATA`, so for `Name Surname` we get them parted
                 "see" -> getSeeTagElementContent(tag).let {
                     See(
                         wrapTagIfNecessary(it.first),
@@ -140,8 +140,7 @@ class JavadocParser(
     }
 
     private fun PsiDocComment.getDescription(): Description? {
-        val nonEmptyDescriptionElements = descriptionElements.filter { it.text.isNotBlank() }
-        return convertJavadocElements(nonEmptyDescriptionElements).takeIf { it.isNotEmpty() }?.let {
+        return convertJavadocElements(descriptionElements.asIterable()).takeIf { it.isNotEmpty() }?.let {
             Description(wrapTagIfNecessary(it))
         }
     }
@@ -154,30 +153,29 @@ class JavadocParser(
             is PsiInlineDocTag -> convertInlineDocTag(this)
             is PsiDocParamRef -> toDocumentationLinkString()
             is PsiDocTagValue,
-            is LeafPsiElement -> (if ((prevSibling as? PsiDocToken)?.isLeadingAsterisk() == true) text?.drop(1) else text)
-                ?.takeUnless { it.isBlank() }
+            is LeafPsiElement -> text.let {
+                if ((prevSibling as? PsiDocToken)?.isLeadingAsterisk() == true) it?.drop(1) else it
+            }.let {
+                if ((nextSibling as? PsiDocToken)?.isLeadingAsterisk() == true) it?.dropLastWhile { it == ' ' } else it
+            }
             else -> null
         }
 
         private fun PsiElement.toDocumentationLinkString(
-            labelElement: PsiElement? = null
-        ): String? =
-            reference?.resolve()?.let {
-                if (it !is PsiParameter) {
-                    val dri = DRI.from(it)
-                    driMap[dri.toString()] = dri
-                    val label = labelElement ?: defaultLabel()
-                    """<a data-dri=$dri>${label.text}</a>"""
-                } else null
+            labelElement: List<PsiElement>? = null
+        ): String =
+            (reference?.resolve()?.takeIf { it !is PsiParameter }?.let {
+                val dri = DRI.from(it)
+                driMap[dri.toString()] = dri
+                Pair(labelElement ?: listOf(defaultLabel()), dri.toString())
+            } ?: Pair(listOf(defaultLabel()), UNRESOLVED_PSI_ELEMENT)).let { (label, dri) ->
+                """<a data-dri=$dri>${label.joinToString(" ") { it.text }}</a>"""
             }
 
         private fun convertInlineDocTag(tag: PsiInlineDocTag) = when (tag.name) {
-            "link", "linkplain" -> {
-                tag.referenceElement()?.toDocumentationLinkString(tag.dataElements.firstIsInstanceOrNull<PsiDocToken>())
-            }
-            "code", "literal" -> {
-                "<code data-inline>${tag.text}</code>"
-            }
+            "link", "linkplain" -> tag.referenceElement()
+                ?.toDocumentationLinkString(tag.dataElements.filterIsInstance<PsiDocToken>())
+            "code", "literal" -> "<code data-inline>${tag.text}</code>"
             "index" -> "<index>${tag.children.filterIsInstance<PsiDocTagValue>().joinToString { it.text }}</index>"
             else -> tag.text
         }
@@ -190,12 +188,14 @@ class JavadocParser(
                     A(children, params = mapOf("href" to element.attr("href")))
                 element.hasAttr("data-dri") && driMap.containsKey(element.attr("data-dri")) ->
                     DocumentationLink(driMap[element.attr("data-dri")]!!, children)
-                else -> Text(children = children)
+                else -> Text(body = children.filterIsInstance<Text>().joinToString { it.body })
             }
         }
 
         private fun createBlock(element: Element, insidePre: Boolean = false): DocTag? {
-            val children = element.childNodes().mapNotNull { convertHtmlNode(it, insidePre = insidePre || element.tagName() == "pre") }
+            val children = element.childNodes()
+                .mapNotNull { convertHtmlNode(it, insidePre = insidePre || element.tagName() == "pre") }
+
             fun ifChildrenPresent(operation: () -> DocTag): DocTag? {
                 return if (children.isNotEmpty()) operation() else null
             }
@@ -218,20 +218,33 @@ class JavadocParser(
         }
 
         private fun convertHtmlNode(node: Node, insidePre: Boolean = false): DocTag? = when (node) {
-            is TextNode -> (if (insidePre) node.wholeText else node.text().takeIf { it.isNotBlank() })?.let { Text(body = it) }
+            is TextNode -> (if (insidePre) node.wholeText else node.text()
+                .takeIf { it.isNotBlank() })?.let { Text(body = it) }
             is Element -> createBlock(node)
             else -> null
         }
 
         override fun invoke(elements: Iterable<PsiElement>, asParagraph: Boolean): List<DocTag> =
-            Jsoup.parseBodyFragment(elements.mapNotNull { it.stringify() }.joinToString("\n", prefix = if (asParagraph) "<p>" else ""))
-                .body().childNodes().mapNotNull { convertHtmlNode(it) }
+            Jsoup.parseBodyFragment(elements.mapNotNull { it.stringify() }.dropWhile { it.isBlank() }
+                .dropLastWhile { it.isBlank() }.joinToString(
+                    "",
+                    prefix = if (asParagraph) "<p>" else "",
+                    postfix = if (asParagraph) "</p>" else ""
+                )
+            ).body().childNodes().mapNotNull { convertHtmlNode(it) }
     }
 
     private fun PsiDocTag.contentElements(): List<PsiElement> =
-        dataElements.mapNotNull { it.takeIf { it.text.isNotBlank() } }
+        dataElements.mapNotNull { it.takeIf { it is PsiDocToken && it.text.isNotBlank() } }
+
+    private fun PsiDocTag.authorContentElements(): List<PsiElement> = listOf(
+        dataElements[0],
+        dataElements[0].nextSibling,
+        *dataElements.drop(1).toTypedArray()
+    )
 
-    private fun convertJavadocElements(elements: Iterable<PsiElement>, asParagraph: Boolean = true): List<DocTag> = Parse()(elements, asParagraph)
+    private fun convertJavadocElements(elements: Iterable<PsiElement>, asParagraph: Boolean = true): List<DocTag> =
+        Parse()(elements, asParagraph)
 
     private fun PsiDocToken.isSharpToken() = tokenType.toString() == "DOC_TAG_VALUE_SHARP_TOKEN"
 
@@ -255,8 +268,12 @@ class JavadocParser(
 
     private fun PsiElement.defaultLabel() = children.firstOrNull {
         it is PsiDocToken && it.text.isNotBlank() && !it.isSharpToken()
-    }  ?: this
+    } ?: this
 
     private fun PsiDocTag.linkElement(): PsiElement? =
         valueElement ?: dataElements.firstOrNull { it !is PsiWhiteSpace }
+
+    companion object {
+        private const val UNRESOLVED_PSI_ELEMENT = "UNRESOLVED_PSI_ELEMENT"
+    }
 }
diff --git a/plugins/base/src/test/kotlin/translators/JavadocParserTest.kt b/plugins/base/src/test/kotlin/translators/JavadocParserTest.kt
index 762c2e27..df5b6dae 100644
--- a/plugins/base/src/test/kotlin/translators/JavadocParserTest.kt
+++ b/plugins/base/src/test/kotlin/translators/JavadocParserTest.kt
@@ -114,6 +114,14 @@ class JavadocParserTest : AbstractCoreTest() {
             | * not fall within the indicated ranges; for example, a date may be
             | * specified as January 32 and is interpreted as meaning February 1.
             | *
+            | * <pre class="prettyprint">
+            | * class MyFragment extends Fragment {
+            | *   public MyFragment() {
+            | *     super(R.layout.fragment_main);
+            | *   }
+            | * }
+            | * </pre>
+            |
             | * @author  James Gosling
             | * @author  Arthur van Hoff
             | * @author  Alan Liu
@@ -178,4 +186,19 @@ class JavadocParserTest : AbstractCoreTest() {
             assertEquals(expectedText.trim(), preTagContent.body.trim())
         }
     }
+
+    @Test
+    fun `correctly parsed code block with curly braces (which PSI has problem with)`() {
+        performJavadocTest { module ->
+            val dateDescription = module.descriptionOf("Date2")!!
+            val preTagContent = dateDescription.childrenOfType<Pre>()[1].firstChildOfType<Text>()
+            val expectedText = """class MyFragment extends Fragment {
+  public MyFragment() {
+    super(R.layout.fragment_main);
+  }
+}""".trimIndent()
+            assertEquals(expectedText.trim(), preTagContent.body.trim())
+        }
+    }
+
 }
diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/AbstractJavadocTemplateMapTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/AbstractJavadocTemplateMapTest.kt
index 09feebe9..ec5c215e 100644
--- a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/AbstractJavadocTemplateMapTest.kt
+++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/AbstractJavadocTemplateMapTest.kt
@@ -1,7 +1,6 @@
 package org.jetbrains.dokka.javadoc
 
-import org.jetbrains.dokka.DokkaConfigurationImpl
-import org.jetbrains.dokka.ExternalDocumentationLink
+import org.jetbrains.dokka.*
 import org.jetbrains.dokka.javadoc.pages.JavadocPageNode
 import org.jetbrains.dokka.javadoc.renderer.JavadocContentToTemplateMapTranslator
 import org.jetbrains.dokka.javadoc.JavadocPlugin
@@ -19,8 +18,8 @@ internal abstract class AbstractJavadocTemplateMapTest : AbstractCoreTest() {
                 sourceRoots = listOf("src")
                 analysisPlatform = "jvm"
                 externalDocumentationLinks = listOf(
-                    ExternalDocumentationLink("https://docs.oracle.com/javase/8/docs/api/"),
-                    ExternalDocumentationLink("https://kotlinlang.org/api/latest/jvm/stdlib/")
+                    DokkaConfiguration.ExternalDocumentationLink.jdk(8),
+                    DokkaConfiguration.ExternalDocumentationLink.kotlinStdlib()
                 )
             }
         }
diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLinkingTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLinkingTest.kt
new file mode 100644
index 00000000..95dc6a2e
--- /dev/null
+++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLinkingTest.kt
@@ -0,0 +1,74 @@
+package org.jetbrains.dokka.javadoc.location
+
+import org.jetbrains.dokka.DokkaConfiguration
+import org.jetbrains.dokka.jdk
+import org.jetbrains.dokka.kotlinStdlib
+import org.jetbrains.dokka.model.doc.DocumentationLink
+import org.jetbrains.dokka.model.doc.Text
+import org.jetbrains.dokka.testApi.testRunner.AbstractCoreTest
+import org.jetbrains.dokka.utilities.cast
+import org.junit.jupiter.api.Test
+import utils.TestOutputWriterPlugin
+import kotlin.test.assertEquals
+
+class JavadocLinkingTest : AbstractCoreTest() {
+
+    @Test
+    fun lineBrokenLink() {
+        val config = dokkaConfiguration {
+            sourceSets {
+                sourceSet {
+                    sourceRoots = listOf("jvmSrc/")
+                    externalDocumentationLinks = listOf(
+                        DokkaConfiguration.ExternalDocumentationLink.jdk(8),
+                        DokkaConfiguration.ExternalDocumentationLink.kotlinStdlib(),
+                    )
+                    analysisPlatform = "jvm"
+                }
+            }
+        }
+        testInline(
+            """
+            |/jvmSrc/javadoc/test/SomeClass.kt
+            |
+            |package example
+            |
+            |class SomeClass {
+            |    fun someFun(x: Int): Int = 1
+            |}
+            |
+            |/jvmSrc/javadoc/test/SomeJavaDocExample.java
+            |
+            |package example;
+            |
+            |/**
+            | * Here comes some comment
+            | *
+            | * {@link example.SomeClass#someFun(int) someName(ads,
+            | * dsa)}
+            | *
+            | * longer comment
+            | */
+            |public class SomeJavaDocExample {
+            |    public void someFunc(int integer, Object object) {
+            |    }
+            |}
+        """.trimMargin(),
+            config,
+            pluginOverrides = listOf(TestOutputWriterPlugin())
+        ) {
+            documentablesMergingStage = {
+                it.packages.single()
+                    .classlikes.single { classlike -> classlike.name == "SomeJavaDocExample" }
+                    .documentation.values.single()
+                    .children.single()
+                    .children.single()
+                    .children.single {
+                        it is DocumentationLink
+                    }.children.filterIsInstance<Text>().single { it.body.contains("someName") }.cast<Text>().body.run {
+                        assertEquals("someName(ads, dsa)", this)
+                    }
+            }
+        }
+    }
+}
diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationTest.kt
index 87de4228..c8ae59e3 100644
--- a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationTest.kt
+++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationTest.kt
@@ -1,11 +1,9 @@
 package org.jetbrains.dokka.javadoc.location
 
+import org.jetbrains.dokka.*
 import org.jetbrains.dokka.javadoc.pages.JavadocClasslikePageNode
 import org.jetbrains.dokka.javadoc.pages.JavadocPackagePageNode
 import org.jetbrains.dokka.javadoc.renderer.JavadocContentToHtmlTranslator
-import org.jetbrains.dokka.DokkaConfiguration
-import org.jetbrains.dokka.ExternalDocumentationLink
-import org.jetbrains.dokka.ExternalDocumentationLinkImpl
 import org.jetbrains.dokka.javadoc.JavadocPlugin
 import org.jetbrains.dokka.model.firstChildOfType
 import org.jetbrains.dokka.pages.RootPageNode
@@ -19,16 +17,14 @@ import org.junit.jupiter.api.Assertions.assertEquals
 class JavadocLocationTest : AbstractCoreTest() {
 
     private fun locationTestInline(testHandler: (RootPageNode, DokkaContext) -> Unit) {
-        fun externalLink(link: String) = ExternalDocumentationLink(link)
-
         val config = dokkaConfiguration {
             format = "javadoc"
             sourceSets {
                 sourceSet {
                     sourceRoots = listOf("jvmSrc/")
                     externalDocumentationLinks = listOf(
-                        externalLink("https://docs.oracle.com/javase/8/docs/api/"),
-                        externalLink("https://kotlinlang.org/api/latest/jvm/stdlib/")
+                        DokkaConfiguration.ExternalDocumentationLink.jdk(8),
+                        DokkaConfiguration.ExternalDocumentationLink.kotlinStdlib()
                     )
                     analysisPlatform = "jvm"
                 }
-- 
cgit