Implement more reliable PackageListService

4 files changed, 51 insertions, 23 deletions

diff --git a/plugins/base/src/main/kotlin/renderers/PackageListService.kt b/plugins/base/src/main/kotlin/renderers/PackageListService.kt
index 3d631f5c..9b753cb1 100644
--- a/plugins/base/src/main/kotlin/renderers/PackageListService.kt
+++ b/plugins/base/src/main/kotlin/renderers/PackageListService.kt
@@ -2,25 +2,23 @@ package org.jetbrains.dokka.base.renderers
 
 import org.jetbrains.dokka.base.DokkaBase
 import org.jetbrains.dokka.links.DRI
-import org.jetbrains.dokka.links.parent
 import org.jetbrains.dokka.pages.*
 import org.jetbrains.dokka.plugability.DokkaContext
 import org.jetbrains.dokka.plugability.plugin
 import org.jetbrains.dokka.plugability.querySingle
 import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 
-class PackageListService(val context: DokkaContext) {
+class PackageListService(val context: DokkaContext, val rootPage: RootPageNode) {
 
-    fun formatPackageList(module: RootPageNode, format: String, linkExtension: String): String {
+    fun createPackageList(module: ModulePage, format: String, linkExtension: String): String {
 
         val packages = mutableSetOf<String>()
         val nonStandardLocations = mutableMapOf<String, String>()
 
         val locationProvider =
-            context.plugin<DokkaBase>().querySingle { locationProviderFactory }.getLocationProvider(module)
-
-        fun visit(node: PageNode, parentDris: Set<DRI>) {
+            context.plugin<DokkaBase>().querySingle { locationProviderFactory }.getLocationProvider(rootPage)
 
+        fun visit(node: PageNode) {
             if (node is PackagePage) {
                 node.name
                     .takeUnless { name -> name.startsWith("[") && name.endsWith("]") } // Do not include the package name for declarations without one
@@ -28,18 +26,19 @@ class PackageListService(val context: DokkaContext) {
             }
 
             val contentPage = node.safeAs<ContentPage>()
-            contentPage?.dri?.forEach {
-                if (parentDris.isNotEmpty() && it.parent !in parentDris) {
-                    locationProvider.resolve(node)
-                        ?.let { nodeLocation -> nonStandardLocations[it.toString()] = nodeLocation }
-                        ?: context.logger.error("Cannot resolve path for ${node.name}!")
+            contentPage?.dri?.forEach { dri ->
+                val nodeLocation = locationProvider.resolve(node, context = module, skipExtension = true)
+                    ?: run { context.logger.error("Cannot resolve path for ${node.name}!"); null }
+
+                if (dri != DRI.topLevel && locationProvider.expectedLocationForDri(dri) != nodeLocation) {
+                    nonStandardLocations[dri.toString()] = "$nodeLocation.$linkExtension"
                 }
             }
 
-            node.children.forEach { visit(it, contentPage?.dri ?: setOf()) }
+            node.children.forEach { visit(it) }
         }
 
-        visit(module, setOf())
+        visit(module)
 
         return buildString {
             appendLine("$DOKKA_PARAM_PREFIX.format:${format}")
diff --git a/plugins/base/src/main/kotlin/renderers/preprocessors.kt b/plugins/base/src/main/kotlin/renderers/preprocessors.kt
index 2640398c..b64d2e1f 100644
--- a/plugins/base/src/main/kotlin/renderers/preprocessors.kt
+++ b/plugins/base/src/main/kotlin/renderers/preprocessors.kt
@@ -10,20 +10,28 @@ object RootCreator : PageTransformer {
         RendererSpecificRootPage("", listOf(input), RenderingStrategy.DoNothing)
 }
 
-
-class PackageListCreator(val context: DokkaContext, val format: LinkFormat, val outputFilesNames: List<String> = listOf("package-list")) : PageTransformer {
+class PackageListCreator(
+    val context: DokkaContext,
+    val format: LinkFormat,
+    val outputFilesNames: List<String> = listOf("package-list"),
+    val removeModulePrefix: Boolean = true
+) : PageTransformer {
     override fun invoke(input: RootPageNode) =
         input.modified(children = input.children.map {
             it.takeUnless { it is ModulePage }
-                ?: it.modified(children = it.children + packageList(input)) // TODO packageList should take module as an input
+                ?: it.modified(children = it.children + packageList(input, it as ModulePage))
         })
 
 
-    private fun packageList(pageNode: RootPageNode): List<RendererSpecificPage> {
-        val content = PackageListService(context).formatPackageList(pageNode, format.formatName, format.linkExtension)
+    private fun packageList(rootPageNode: RootPageNode, module: ModulePage): List<RendererSpecificPage> {
+        val content = PackageListService(context, rootPageNode).createPackageList(
+            module,
+            format.formatName,
+            format.linkExtension
+        )
         return outputFilesNames.map { fileName ->
             RendererSpecificResourcePage(
-                "${pageNode.name}/${fileName}",
+                "${rootPageNode.name}/${fileName}",
                 emptyList(),
                 RenderingStrategy.Write(content)
             )
diff --git a/plugins/base/src/main/kotlin/resolvers/local/LocationProvider.kt b/plugins/base/src/main/kotlin/resolvers/local/LocationProvider.kt
index 0a095153..6d3ad215 100644
--- a/plugins/base/src/main/kotlin/resolvers/local/LocationProvider.kt
+++ b/plugins/base/src/main/kotlin/resolvers/local/LocationProvider.kt
@@ -1,6 +1,7 @@
 package org.jetbrains.dokka.base.resolvers.local
 
 import org.jetbrains.dokka.DokkaException
+import org.jetbrains.dokka.base.resolvers.local.DokkaLocationProvider.Companion.identifierToFilename
 import org.jetbrains.dokka.links.DRI
 import org.jetbrains.dokka.model.DisplaySourceSet
 import org.jetbrains.dokka.pages.PageNode
@@ -10,6 +11,18 @@ interface LocationProvider {
     fun resolve(node: PageNode, context: PageNode? = null, skipExtension: Boolean = false): String?
     fun pathToRoot(from: PageNode): String
     fun ancestors(node: PageNode): List<PageNode>
+
+    /**
+     * This method should return guessed filesystem location for a given [DRI]
+     * It is used to decide if a [DRI] should be present in the relocation list of the
+     * generated package-list so it is ok if the path differs from the one returned by [resolve]
+     * @return Path to a giver [DRI] or null if path should not be considered for relocations
+     */
+    fun expectedLocationForDri(dri: DRI): String =
+        (listOf(dri.packageName) +
+                dri.classNames?.split(".")?.map { identifierToFilename(it) }.orEmpty() +
+                listOf(dri.callable?.let { identifierToFilename(it.name) } ?: "index")
+                ).filterNotNull().joinToString("/")
 }
 
 fun LocationProvider.resolveOrThrow(dri: DRI, sourceSets: Set<DisplaySourceSet>, context: PageNode? = null): String =
diff --git a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationProvider.kt b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationProvider.kt
index 38258fc6..b9828e9b 100644
--- a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationProvider.kt
+++ b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/location/JavadocLocationProvider.kt
@@ -2,12 +2,11 @@ package org.jetbrains.dokka.javadoc.location
 
 import org.jetbrains.dokka.base.resolvers.local.DefaultLocationProvider
 import org.jetbrains.dokka.javadoc.pages.*
-import org.jetbrains.dokka.links.*
+import org.jetbrains.dokka.links.DRI
 import org.jetbrains.dokka.links.Nullable
-import org.jetbrains.dokka.model.*
-import org.jetbrains.dokka.pages.ContentPage
+import org.jetbrains.dokka.links.PointingToDeclaration
 import org.jetbrains.dokka.model.DisplaySourceSet
-import org.jetbrains.dokka.model.TypeConstructor
+import org.jetbrains.dokka.pages.ContentPage
 import org.jetbrains.dokka.pages.PageNode
 import org.jetbrains.dokka.pages.RootPageNode
 import org.jetbrains.dokka.plugability.DokkaContext
@@ -128,4 +127,13 @@ class JavadocLocationProvider(pageRoot: RootPageNode, dokkaContext: DokkaContext
     override fun ancestors(node: PageNode): List<PageNode> {
         TODO("Not yet implemented")
     }
+
+    override fun expectedLocationForDri(dri: DRI): String {
+        if (dri.packageName?.isNotEmpty() == true && dri.classNames == null)
+            return (dri.packageName?.split(".").orEmpty() + "package-summary").joinToString("/")
+
+        return (dri.packageName?.split(".").orEmpty() +
+                dri.classNames?.split(".").orEmpty() // Top-level methods will always be relocated which is fine
+                ).joinToString("/")
+    }
 }