Add explicit exceptions in markdown and allow for empty link destinations (#1757)

6 files changed, 180 insertions, 21 deletions

diff --git a/plugins/base/src/main/kotlin/parsers/MarkdownParser.kt b/plugins/base/src/main/kotlin/parsers/MarkdownParser.kt
index 9a4aa39f..37b1db47 100644
--- a/plugins/base/src/main/kotlin/parsers/MarkdownParser.kt
+++ b/plugins/base/src/main/kotlin/parsers/MarkdownParser.kt
@@ -23,7 +23,8 @@ import java.net.URL
 import org.intellij.markdown.parser.MarkdownParser as IntellijMarkdownParser
 
 open class MarkdownParser(
-    private val externalDri: (String) -> DRI?
+    private val externalDri: (String) -> DRI?,
+    private val kdocLocation: String?
 ) : Parser() {
 
     private lateinit var destinationLinksMap: Map<String, String>
@@ -65,7 +66,8 @@ open class MarkdownParser(
         DocTagsFromIElementFactory.getInstance(
             node.type,
             visitNode(node.children.find { it.type == MarkdownTokenTypes.ATX_CONTENT }
-                ?: throw IllegalStateException("Wrong AST Tree. ATX Header does not contain expected content")).children
+                ?: throw detailedException("Wrong AST Tree. Header does not contain expected content", node)
+            ).children
         )
 
     private fun horizontalRulesHandler(node: ASTNode) =
@@ -163,7 +165,7 @@ open class MarkdownParser(
 
     private fun referenceLinksHandler(node: ASTNode): DocTag {
         val linkLabel = node.children.find { it.type == MarkdownElementTypes.LINK_LABEL }
-            ?: throw IllegalStateException("Wrong AST Tree. Reference link does not contain expected content")
+            ?: throw detailedException("Wrong AST Tree. Reference link does not contain link label", node)
         val linkText = node.children.findLast { it.type == MarkdownElementTypes.LINK_TEXT } ?: linkLabel
 
         val linkKey = text.substring(linkLabel.startOffset, linkLabel.endOffset)
@@ -175,12 +177,12 @@ open class MarkdownParser(
 
     private fun inlineLinksHandler(node: ASTNode): DocTag {
         val linkText = node.children.find { it.type == MarkdownElementTypes.LINK_TEXT }
-            ?: throw IllegalStateException("Wrong AST Tree. Inline link does not contain expected content")
+            ?: throw detailedException("Wrong AST Tree. Inline link does not contain link text", node)
         val linkDestination = node.children.find { it.type == MarkdownElementTypes.LINK_DESTINATION }
-            ?: throw IllegalStateException("Wrong AST Tree. Inline link does not contain expected content")
         val linkTitle = node.children.find { it.type == MarkdownElementTypes.LINK_TITLE }
 
-        val link = text.substring(linkDestination.startOffset, linkDestination.endOffset)
+        // Link destination may be ommited: https://github.github.com/gfm/#example-495
+        val link = linkDestination?.let { text.substring(it.startOffset, it.endOffset) }
 
         return linksHandler(linkText, link, linkTitle)
     }
@@ -197,18 +199,18 @@ open class MarkdownParser(
         return linksHandler(node, link)
     }
 
-    private fun linksHandler(linkText: ASTNode, link: String, linkTitle: ASTNode? = null): DocTag {
-        val dri: DRI? = resolveDRI(link)
+    private fun linksHandler(linkText: ASTNode, link: String?, linkTitle: ASTNode? = null): DocTag {
+        val dri: DRI? = link?.let { resolveDRI(it) }
+        val linkOrEmpty = link ?: ""
         val linkTextString =
-            if (linkTitle == null) link else text.substring(linkTitle.startOffset + 1, linkTitle.endOffset - 1)
+            if (linkTitle == null) linkOrEmpty else text.substring(linkTitle.startOffset + 1, linkTitle.endOffset - 1)
 
         val params = if (linkTitle == null)
-            mapOf("href" to link)
+            mapOf("href" to linkOrEmpty)
         else
-            mapOf("href" to link, "title" to linkTextString)
+            mapOf("href" to linkOrEmpty, "title" to linkTextString)
 
-
-        return if (dri == null && !link.isRemoteLink()) {
+        return if (link != null && dri == null && !linkOrEmpty.isRemoteLink()) {
             DocTagsFromIElementFactory.getInstance(
                 MarkdownTokenTypes.TEXT,
                 params = params,
@@ -451,14 +453,28 @@ open class MarkdownParser(
         .replace(Regex("\n"), " ")
         .replace(Regex(" >+ +"), " ")      // Replacement used in blockquotes, get rid of garbage
 
+    private fun detailedException(baseMessage: String, node: ASTNode) =
+        IllegalStateException(
+            baseMessage + " in ${kdocLocation ?: "unspecified location"}, element starts from offset ${node.startOffset} and ends ${node.endOffset}: ${
+                text.substring(
+                    node.startOffset,
+                    node.endOffset
+                )
+            }"
+        )
 
-    companion object {
 
-        fun parseFromKDocTag(kDocTag: KDocTag?, externalDri: (String) -> DRI?): DocumentationNode {
+    companion object {
+        fun parseFromKDocTag(
+            kDocTag: KDocTag?,
+            externalDri: (String) -> DRI?,
+            kdocLocation: String?
+        ): DocumentationNode {
             return if (kDocTag == null) {
                 DocumentationNode(emptyList())
             } else {
-                fun parseStringToDocNode(text: String) = MarkdownParser(externalDri).parseStringToDocNode(text)
+                fun parseStringToDocNode(text: String) =
+                    MarkdownParser(externalDri, kdocLocation).parseStringToDocNode(text)
 
                 fun pointedLink(tag: KDocTag): DRI? = (parseStringToDocNode("[${tag.getSubjectName()}]")).let {
                     val link = it.children[0].children[0]
diff --git a/plugins/base/src/main/kotlin/parsers/moduleAndPackage/ModuleAndPackageDocumentationParsingContext.kt b/plugins/base/src/main/kotlin/parsers/moduleAndPackage/ModuleAndPackageDocumentationParsingContext.kt
index afdcc43f..9122d8ee 100644
--- a/plugins/base/src/main/kotlin/parsers/moduleAndPackage/ModuleAndPackageDocumentationParsingContext.kt
+++ b/plugins/base/src/main/kotlin/parsers/moduleAndPackage/ModuleAndPackageDocumentationParsingContext.kt
@@ -18,19 +18,19 @@ import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.Name
 
 fun interface ModuleAndPackageDocumentationParsingContext {
-    fun markdownParserFor(fragment: ModuleAndPackageDocumentationFragment): MarkdownParser
+    fun markdownParserFor(fragment: ModuleAndPackageDocumentationFragment, location: String): MarkdownParser
 }
 
 internal fun ModuleAndPackageDocumentationParsingContext.parse(
     fragment: ModuleAndPackageDocumentationFragment
 ): DocumentationNode {
-    return markdownParserFor(fragment).parse(fragment.documentation)
+    return markdownParserFor(fragment, fragment.source.sourceDescription).parse(fragment.documentation)
 }
 
 fun ModuleAndPackageDocumentationParsingContext(
     logger: DokkaLogger,
     facade: DokkaResolutionFacade? = null
-) = ModuleAndPackageDocumentationParsingContext { fragment ->
+) = ModuleAndPackageDocumentationParsingContext { fragment, sourceLocation ->
     val descriptor = when (fragment.classifier) {
         Module -> facade?.moduleDescriptor?.getPackage(FqName.topLevel(Name.identifier("")))
         Package -> facade?.moduleDescriptor?.getPackage(FqName(fragment.name))
@@ -53,7 +53,7 @@ fun ModuleAndPackageDocumentationParsingContext(
         }
     }
 
-    MarkdownParser(externalDri = externalDri)
+    MarkdownParser(externalDri = externalDri, sourceLocation)
 }
 
 private fun Collection<DeclarationDescriptor>.sorted() = sortedWith(
diff --git a/plugins/base/src/main/kotlin/parsers/moduleAndPackage/parseModuleAndPackageDocumentationFragments.kt b/plugins/base/src/main/kotlin/parsers/moduleAndPackage/parseModuleAndPackageDocumentationFragments.kt
index ea88e6e5..a21a5acb 100644
--- a/plugins/base/src/main/kotlin/parsers/moduleAndPackage/parseModuleAndPackageDocumentationFragments.kt
+++ b/plugins/base/src/main/kotlin/parsers/moduleAndPackage/parseModuleAndPackageDocumentationFragments.kt
@@ -30,7 +30,7 @@ private fun parseModuleAndPackageDocFragment(
         "Module" -> Module
         "Package" -> Package
         else -> throw IllegalStateException(
-            """Unexpected classifier ${classifierAndName[0]}, expected either "Module" or "Package". 
+            """Unexpected classifier: "${classifierAndName[0]}", expected either "Module" or "Package". 
             |For more information consult the specification: https://kotlinlang.org/docs/reference/kotlin-doc.html#module-and-package-documentation""".trimMargin()
         )
     }
diff --git a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt
index 51389f37..b08c11b8 100644
--- a/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt
+++ b/plugins/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt
@@ -47,6 +47,7 @@ import org.jetbrains.kotlin.resolve.constants.ConstantValue
 import org.jetbrains.kotlin.resolve.constants.KClassValue.Value.LocalClass
 import org.jetbrains.kotlin.resolve.constants.KClassValue.Value.NormalClass
 import org.jetbrains.kotlin.resolve.descriptorUtil.annotationClass
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
 import org.jetbrains.kotlin.resolve.scopes.MemberScope
 import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 import org.jetbrains.kotlin.resolve.source.PsiSourceElement
@@ -860,6 +861,11 @@ private class DokkaDescriptorVisitor(
                     logger.warn("Couldn't resolve link for $link")
                     null
                 }
+            },
+            kdocLocation = toSourceElement?.containingFile?.name?.let {
+                val fqName = fqNameOrNull()?.asString()
+                if (fqName != null) "$it/$fqName"
+                else it
             }
         )
     }.takeIf { it.children.isNotEmpty() }
diff --git a/plugins/base/src/test/kotlin/markdown/ParserTest.kt b/plugins/base/src/test/kotlin/markdown/ParserTest.kt
index 0fdb10c1..4ac4a43f 100644
--- a/plugins/base/src/test/kotlin/markdown/ParserTest.kt
+++ b/plugins/base/src/test/kotlin/markdown/ParserTest.kt
@@ -5,6 +5,9 @@ import org.intellij.markdown.MarkdownElementTypes
 import org.jetbrains.dokka.model.doc.*
 import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 
 
 class ParserTest : KDocTest() {
@@ -1437,5 +1440,46 @@ class ParserTest : KDocTest() {
         )
         executeTest(kdoc, expectedDocumentationNode)
     }
+
+    @Test
+    fun `short link without destination`() {
+        val kdoc = """
+        | This is [link]()
+        """.trimMargin()
+        val expectedDocumentationNode = DocumentationNode(
+            listOf(
+                Description(
+                    CustomDocTag(
+                        listOf(
+                            P(
+                                listOf(
+                                    Text("This is "),
+                                    A(
+                                        listOf(Text("link")),
+                                        mapOf("href" to "")
+                                    )
+                                )
+                            )
+                        ), name = MarkdownElementTypes.MARKDOWN_FILE.name
+                    )
+                )
+            )
+        )
+        executeTest(kdoc, expectedDocumentationNode)
+    }
+
+    @Test
+    fun `exception thrown by empty header should point to location of a file`() {
+        val kdoc = """
+        | ###
+        """.trimMargin()
+        val expectedDocumentationNode = DocumentationNode(emptyList())
+        val exception = runCatching { executeTest(kdoc, expectedDocumentationNode) }.exceptionOrNull()
+
+        assertEquals(
+            "Wrong AST Tree. Header does not contain expected content in Test.kt/example.Test, element starts from offset 0 and ends 3: ###",
+            exception?.message
+        )
+    }
 }
 
diff --git a/plugins/base/src/test/kotlin/transformers/InvalidContentModuleAndPackageDocumentationReaderTest.kt b/plugins/base/src/test/kotlin/transformers/InvalidContentModuleAndPackageDocumentationReaderTest.kt
new file mode 100644
index 00000000..b9837750
--- /dev/null
+++ b/plugins/base/src/test/kotlin/transformers/InvalidContentModuleAndPackageDocumentationReaderTest.kt
@@ -0,0 +1,93 @@
+package transformers
+
+import org.jetbrains.dokka.base.transformers.documentables.ModuleAndPackageDocumentationReader
+import org.jetbrains.dokka.plugability.DokkaContext
+import org.jetbrains.dokka.utilities.DokkaConsoleLogger
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import testApi.testRunner.TestDokkaConfigurationBuilder
+import testApi.testRunner.dModule
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+
+class InvalidContentModuleAndPackageDocumentationReaderTest : AbstractContextModuleAndPackageDocumentationReaderTest() {
+
+    private val includeA by lazy { temporaryDirectory.resolve("includeA.md").toFile() }
+    private val includeB by lazy { temporaryDirectory.resolve("includeB.md").toFile() }
+
+    @BeforeEach
+    fun materializeInclude() {
+        includeA.writeText(
+            """
+            Invalid random stuff 
+            
+            # Module moduleA
+            Simple stuff
+            """.trimIndent()
+        )
+        includeB.writeText(
+            """
+            # Module moduleB
+            ###
+            """.trimIndent()
+        )
+    }
+
+    private val configurationBuilderA = TestDokkaConfigurationBuilder().apply {
+        moduleName = "moduleA"
+    }
+    private val configurationBuilderB = TestDokkaConfigurationBuilder().apply {
+        moduleName = "moduleB"
+    }
+
+    private val sourceSetA by configurationBuilderA.sourceSet {
+        includes = listOf(includeA.canonicalPath)
+    }
+
+    private val sourceSetB by configurationBuilderB.sourceSet {
+        includes = listOf(includeB.canonicalPath)
+    }
+
+    private val contextA by lazy {
+        DokkaContext.create(
+            configuration = configurationBuilderA.build(),
+            logger = DokkaConsoleLogger,
+            pluginOverrides = emptyList()
+        )
+    }
+    private val contextB by lazy {
+        DokkaContext.create(
+            configuration = configurationBuilderB.build(),
+            logger = DokkaConsoleLogger,
+            pluginOverrides = emptyList()
+        )
+    }
+
+    private val readerA by lazy { ModuleAndPackageDocumentationReader(contextA) }
+    private val readerB by lazy { ModuleAndPackageDocumentationReader(contextB) }
+
+
+    @Test
+    fun `parsing should fail with a message when documentation is in not proper format`() {
+        val exception =
+            runCatching { readerA[dModule(name = "moduleA", sourceSets = setOf(sourceSetA))] }.exceptionOrNull()
+        assertEquals(
+            "Unexpected classifier: \"Invalid\", expected either \"Module\" or \"Package\". \n" +
+                    "For more information consult the specification: https://kotlinlang.org/docs/reference/kotlin-doc.html#module-and-package-documentation",
+            exception?.message
+        )
+    }
+
+    @Test
+    fun `parsing should fail with a message where it encountered error and why`() {
+        val exception =
+            runCatching { readerB[dModule(name = "moduleB", sourceSets = setOf(sourceSetB))] }.exceptionOrNull()?.message!!
+
+        //I don't want to assert whole message since it contains a path to a temporary folder
+        assertTrue(exception.contains("Wrong AST Tree. Header does not contain expected content in "))
+        assertTrue(exception.contains("includeB.md"))
+        assertTrue(exception.contains("element starts from offset 0 and ends 3: ###"))
+    }
+}
+