Fix anchor linking when the linked type is on a different platform (#1568)

* Fix anchor linking when the linked type is on a different platform

* Refactor location provider to use types

Co-authored-by: Marcin Aman <marcin.aman@gmail.com>

4 files changed, 88 insertions, 44 deletions

diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
index f90a3ace..2a10bdd5 100644
--- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
@@ -9,12 +9,11 @@ import org.jetbrains.dokka.DokkaSourceSetID
 import org.jetbrains.dokka.base.DokkaBase
 import org.jetbrains.dokka.base.renderers.DefaultRenderer
 import org.jetbrains.dokka.base.renderers.TabSortingStrategy
-import org.jetbrains.dokka.base.resolvers.anchors.SymbolAnchorHint
-import org.jetbrains.dokka.base.transformers.pages.sourcelinks.hasTabbedContent
 import org.jetbrains.dokka.base.renderers.isImage
 import org.jetbrains.dokka.base.renderers.pageId
+import org.jetbrains.dokka.base.resolvers.anchors.SymbolAnchorHint
+import org.jetbrains.dokka.base.resolvers.local.DokkaBaseLocationProvider
 import org.jetbrains.dokka.links.DRI
-import org.jetbrains.dokka.model.CompositeSourceSetID
 import org.jetbrains.dokka.model.DisplaySourceSet
 import org.jetbrains.dokka.model.properties.PropertyContainer
 import org.jetbrains.dokka.model.sourceSetIDs
@@ -25,7 +24,6 @@ import org.jetbrains.dokka.plugability.plugin
 import org.jetbrains.dokka.plugability.query
 import org.jetbrains.dokka.plugability.querySingle
 import org.jetbrains.dokka.utilities.htmlEscape
-import org.jetbrains.dokka.utilities.urlEncoded
 import java.net.URI
 
 open class HtmlRenderer(
@@ -833,6 +831,18 @@ open class HtmlRenderer(
                 }
             }
         }
+
+    private val ContentNode.isAnchorable: Boolean
+        get() = anchorLabel != null
+
+    private val ContentNode.anchorLabel: String?
+        get() = extra[SymbolAnchorHint]?.anchorName
+
+    private val ContentNode.anchor: String?
+        get() = extra[SymbolAnchorHint]?.contentKind?.let { contentKind ->
+            (locationProvider as DokkaBaseLocationProvider).anchorForDCI(DCI(dci.dri, contentKind), sourceSets)
+        }
+
 }
 
 fun List<SimpleAttr>.joinAttr() = joinToString(" ") { it.extraKey + "=" + it.extraValue }
@@ -842,21 +852,7 @@ private fun String.stripDiv() = drop(5).dropLast(6) // TODO: Find a way to do it
 private val PageNode.isNavigable: Boolean
     get() = this !is RendererSpecificPage || strategy != RenderingStrategy.DoNothing
 
-fun PropertyContainer<ContentNode>.extraHtmlAttributes() = allOfType<SimpleAttr>()
-
-val ContentNode.isAnchorable: Boolean
-    get() = anchorLabel != null
-
-val ContentNode.anchorLabel: String?
-    get() = extra[SymbolAnchorHint]?.anchorName
-
-/**
- * Anchors should be unique and should contain sourcesets, dri and contentKind.
- * The idea is to make them as short as possible and just use a hashCode from sourcesets in order to match the
- * 2040 characters limit
- */
-val ContentNode.anchor: String?
-    get() = extra[SymbolAnchorHint]?.contentKind?.let { contentKind -> (dci.dri.first().toString() + "/" + contentKind + "/" + sourceSets.hashCode()).urlEncoded() }
+private fun PropertyContainer<ContentNode>.extraHtmlAttributes() = allOfType<SimpleAttr>()
 
-val ContentNode.sourceSetsFilters: String
+private val ContentNode.sourceSetsFilters: String
     get() = sourceSets.sourceSetIDs.joinToString(" ") { it.toString() }
diff --git a/plugins/base/src/main/kotlin/resolvers/local/DokkaBaseLocationProvider.kt b/plugins/base/src/main/kotlin/resolvers/local/DokkaBaseLocationProvider.kt
new file mode 100644
index 00000000..b1213b19
--- /dev/null
+++ b/plugins/base/src/main/kotlin/resolvers/local/DokkaBaseLocationProvider.kt
@@ -0,0 +1,23 @@
+package org.jetbrains.dokka.base.resolvers.local
+
+import org.jetbrains.dokka.model.DisplaySourceSet
+import org.jetbrains.dokka.pages.DCI
+import org.jetbrains.dokka.pages.RootPageNode
+import org.jetbrains.dokka.plugability.DokkaContext
+import org.jetbrains.dokka.utilities.urlEncoded
+
+abstract class DokkaBaseLocationProvider(
+    pageGraphRoot: RootPageNode,
+    dokkaContext: DokkaContext,
+    extension: String
+) : DefaultLocationProvider(pageGraphRoot, dokkaContext, extension) {
+
+    /**
+     * Anchors should be unique and should contain sourcesets, dri and contentKind.
+     * The idea is to make them as short as possible and just use a hashCode from sourcesets in order to match the
+     * 2040 characters limit
+     */
+    open fun anchorForDCI(dci: DCI, sourceSets: Set<DisplaySourceSet>): String =
+        (dci.dri.toString() + "/" + dci.kind + "/" + sourceSets.hashCode()).urlEncoded()
+
+}
diff --git a/plugins/base/src/main/kotlin/resolvers/local/DokkaLocationProvider.kt b/plugins/base/src/main/kotlin/resolvers/local/DokkaLocationProvider.kt
index fe8c8f84..3ef8bcce 100644
--- a/plugins/base/src/main/kotlin/resolvers/local/DokkaLocationProvider.kt
+++ b/plugins/base/src/main/kotlin/resolvers/local/DokkaLocationProvider.kt
@@ -5,17 +5,17 @@ import org.jetbrains.dokka.base.resolvers.anchors.SymbolAnchorHint
 import org.jetbrains.dokka.links.DRI
 import org.jetbrains.dokka.links.PointingToDeclaration
 import org.jetbrains.dokka.model.DisplaySourceSet
+import org.jetbrains.dokka.model.toDisplaySourceSet
 import org.jetbrains.dokka.model.withDescendants
 import org.jetbrains.dokka.pages.*
 import org.jetbrains.dokka.plugability.DokkaContext
-import org.jetbrains.dokka.utilities.urlEncoded
 import java.util.*
 
 open class DokkaLocationProvider(
     pageGraphRoot: RootPageNode,
     dokkaContext: DokkaContext,
     extension: String = ".html"
-) : DefaultLocationProvider(pageGraphRoot, dokkaContext, extension) {
+) : DokkaBaseLocationProvider(pageGraphRoot, dokkaContext, extension) {
     protected open val PAGE_WITH_CHILDREN_SUFFIX = "index"
 
     protected open val pathsIndex: Map<PageNode, List<String>> = IdentityHashMap<PageNode, List<String>>().apply {
@@ -28,28 +28,33 @@ open class DokkaLocationProvider(
         pageGraphRoot.children.forEach { registerPath(it, emptyList()) }
     }
 
-    protected val pagesIndex: Map<Pair<DRI, DisplaySourceSet?>, ContentPage> =
+    protected val pagesIndex: Map<DRIWithSourceSet, ContentPage> =
         pageGraphRoot.withDescendants().filterIsInstance<ContentPage>()
             .flatMap { page ->
                 page.dri.flatMap { dri ->
-                    page.sourceSets().ifEmpty { setOf(null) }.map { sourceSet -> (dri to sourceSet) to page }
+                    page.sourceSets().ifEmpty { setOf(null) }.map { sourceSet -> DRIWithSourceSet(dri,sourceSet) to page }
                 }
             }
             .groupingBy { it.first }
             .aggregate { key, _, (_, page), first ->
-                if (first) page else throw AssertionError("Multiple pages associated with key: ${key.first}/${key.second}")
+                if (first) page else throw AssertionError("Multiple pages associated with key: ${key.dri}/${key.sourceSet}")
             }
 
-    protected val anchorsIndex: Map<Pair<DRI, DisplaySourceSet?>, ContentPage> =
+    protected val anchorsIndex: Map<DRIWithSourceSet, PageWithKind> =
         pageGraphRoot.withDescendants().filterIsInstance<ContentPage>()
             .flatMap { page ->
                 page.content.withDescendants()
-                    .filter { it.extra[SymbolAnchorHint] != null }
-                    .mapNotNull { it.dci.dri.singleOrNull() }
+                    .filter { it.extra[SymbolAnchorHint] != null && it.dci.dri.any() }
+                    .flatMap { content ->
+                        content.dci.dri.map { dri ->
+                            (dri to content.sourceSets) to content.extra[SymbolAnchorHint]?.contentKind!!
+                        }
+                    }
                     .distinct()
-                    .flatMap { dri ->
-                        page.sourceSets().ifEmpty { setOf(null) }.map { sourceSet ->
-                            (dri to sourceSet) to page
+                    .flatMap { (pair, kind) ->
+                        val (dri, sourceSets) = pair
+                        sourceSets.ifEmpty { setOf(null) }.map { sourceSet ->
+                            DRIWithSourceSet(dri, sourceSet) to PageWithKind(page, kind)
                         }
                     }
             }.toMap()
@@ -59,18 +64,33 @@ open class DokkaLocationProvider(
 
     override fun resolve(dri: DRI, sourceSets: Set<DisplaySourceSet>, context: PageNode?): String? =
         sourceSets.ifEmpty { setOf(null) }.mapNotNull { sourceSet ->
-            val driWithSourceSet = Pair(dri, sourceSet)
+            val driWithSourceSet = DRIWithSourceSet(dri, sourceSet)
             getLocalLocation(driWithSourceSet, context)
-                ?: getLocalLocation(driWithSourceSet.copy(first = dri.copy(target = PointingToDeclaration)), context)
+                ?: getLocalLocation(driWithSourceSet.copy(dri = dri.copy(target = PointingToDeclaration)), context)
                 // Not found in PageGraph, that means it's an external link
                 ?: getExternalLocation(dri, sourceSets)
                 ?: getExternalLocation(dri.copy(target = PointingToDeclaration), sourceSets)
         }.distinct().singleOrNull()
 
-    private fun getLocalLocation(dri: Pair<DRI, DisplaySourceSet?>, context: PageNode?): String? =
-        pagesIndex[dri]?.let { resolve(it, context) }
-            ?: anchorsIndex[dri]?.let { resolve(it, context) + "#${dri.first.toString().urlEncoded()}" }
+    private fun getLocalLocation(driWithSourceSet: DRIWithSourceSet, context: PageNode?): String? {
+        val (dri, originalSourceSet) = driWithSourceSet
+        val allSourceSets =
+            listOf(originalSourceSet) + originalSourceSet?.let { oss ->
+                dokkaContext.configuration.sourceSets.filter { it.sourceSetID in oss.sourceSetIDs }
+                    .flatMap { it.dependentSourceSets }
+                    .mapNotNull { ssid ->
+                        dokkaContext.configuration.sourceSets.find { it.sourceSetID == ssid }?.toDisplaySourceSet()
+                    }
+            }.orEmpty()
 
+        return allSourceSets.asSequence().mapNotNull { displaySourceSet ->
+            pagesIndex[DRIWithSourceSet(dri, displaySourceSet)]?.let { page -> resolve(page, context) }
+                ?: anchorsIndex[driWithSourceSet]?.let { (page, kind) ->
+                    val dci = DCI(setOf(dri), kind)
+                    resolve(page, context) + "#" + anchorForDCI(dci, setOfNotNull(displaySourceSet))
+                }
+        }.firstOrNull()
+    }
 
     override fun pathToRoot(from: PageNode): String =
         pathTo(pageGraphRoot, from).removeSuffix(PAGE_WITH_CHILDREN_SUFFIX)
@@ -104,8 +124,13 @@ open class DokkaLocationProvider(
     private val PageNode.pathName: String
         get() = if (this is PackagePageNode) name else identifierToFilename(name)
 
+    protected data class DRIWithSourceSet(val dri: DRI, val sourceSet: DisplaySourceSet?)
+
+    protected data class PageWithKind(val page: ContentPage, val kind: Kind)
+
     companion object {
         internal val reservedFilenames = setOf("index", "con", "aux", "lst", "prn", "nul", "eof", "inp", "out")
+
         //Taken from: https://stackoverflow.com/questions/1976007/what-characters-are-forbidden-in-windows-and-linux-directory-names
         internal val reservedCharacters = setOf('|', '>', '<', '*', ':', '"', '?', '%')
 
@@ -119,10 +144,9 @@ open class DokkaLocationProvider(
 internal fun sanitizeFileName(name: String, reservedFileNames: Set<String>, reservedCharacters: Set<Char>): String {
     val lowercase = name.replace("[A-Z]".toRegex()) { matchResult -> "-" + matchResult.value.toLowerCase() }
     val withoutReservedFileNames = if (lowercase in reservedFileNames) "--$lowercase--" else lowercase
-    return reservedCharacters.fold(withoutReservedFileNames){
-        acc, character ->
-            if(character in acc) acc.replace(character.toString(), "[${character.toInt()}]")
-            else acc
+    return reservedCharacters.fold(withoutReservedFileNames) { acc, character ->
+        if (character in acc) acc.replace(character.toString(), "[${character.toInt()}]")
+        else acc
     }
 }
 
diff --git a/plugins/base/src/main/kotlin/resolvers/local/MultimoduleLocationProvider.kt b/plugins/base/src/main/kotlin/resolvers/local/MultimoduleLocationProvider.kt
index b5891dbf..60e72319 100644
--- a/plugins/base/src/main/kotlin/resolvers/local/MultimoduleLocationProvider.kt
+++ b/plugins/base/src/main/kotlin/resolvers/local/MultimoduleLocationProvider.kt
@@ -7,11 +7,12 @@ import org.jetbrains.dokka.pages.PageNode
 import org.jetbrains.dokka.pages.RootPageNode
 import org.jetbrains.dokka.plugability.DokkaContext
 
-class MultimoduleLocationProvider(private val root: RootPageNode, context: DokkaContext) : LocationProvider {
+class MultimoduleLocationProvider(pageGraphRoot: RootPageNode, dokkaContext: DokkaContext) :
+    DokkaBaseLocationProvider(pageGraphRoot, dokkaContext, ".html") {
 
-    private val defaultLocationProvider = DokkaLocationProvider(root, context)
+    private val defaultLocationProvider = DokkaLocationProvider(pageGraphRoot, dokkaContext)
 
-    val paths = context.configuration.modules.map {
+    val paths = dokkaContext.configuration.modules.map {
         it.name to it.relativePathToOutputDirectory
     }.toMap()
 
@@ -25,7 +26,7 @@ class MultimoduleLocationProvider(private val root: RootPageNode, context: Dokka
 
     override fun pathToRoot(from: PageNode): String = defaultLocationProvider.pathToRoot(from)
 
-    override fun ancestors(node: PageNode): List<PageNode> = listOf(root)
+    override fun ancestors(node: PageNode): List<PageNode> = listOf(pageGraphRoot)
 
     companion object {
         const val MULTIMODULE_PACKAGE_PLACEHOLDER = ".ext"