Support for providing sourceRoots as relative paths (#3362)

* Use `Path::toRealPath` to resolve symlinks and `..` in descriptors * Use `analysisContext.modulesWithFiles` in symbols
Kotlin · Dec 6, 2023 · b855a0b · b855a0b
1 parent bb036a3
commit b855a0b
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 25 deletions.
diff --git a/...ion-tests/cli/src/integrationTest/kotlin/org/jetbrains/dokka/it/cli/CliIntegrationTest.kt b/...ion-tests/cli/src/integrationTest/kotlin/org/jetbrains/dokka/it/cli/CliIntegrationTest.kt
@@ -374,4 +374,42 @@ class CliIntegrationTest : AbstractCliIntegrationTest() {
 
         assertTrue(dokkaOutputDir.isDirectory, "Missing dokka output directory")
     }
+
+    @Test
+    fun `relative paths in configuraiton should work`() {
+        val resourcePath =
+            javaClass.getResource("/my-file.json")?.toURI() ?: throw IllegalStateException("No JSON found!")
+        val jsonPath = File(resourcePath)
+
+        val dokkaOutputDir = File(projectDir, "output-relative")
+        assertTrue(dokkaOutputDir.mkdirs())
+        jsonPath.writeText(
+            jsonBuilder(
+                outputPath = dokkaOutputDir.invariantSeparatorsPath,
+                pluginsClasspath = basePluginJarFile.absoluteFile.invariantSeparatorsPath,
+                projectPath = "src", // relative path
+            )
+        )
+
+        ProcessBuilder(
+            "java", "-jar", cliJarFile.absolutePath, jsonPath.absolutePath
+        ).directory(projectDir).redirectErrorStream(true).start().also { process ->
+            val result = process.awaitProcessResult()
+            assertEquals(0, result.exitCode, "Expected exitCode 0 (Success)")
+        }
+
+        assertTrue(dokkaOutputDir.isDirectory, "Missing dokka output directory")
+
+        val htmlFiles = dokkaOutputDir.allHtmlFiles().map { it.relativeTo(dokkaOutputDir).path }.toList()
+
+        // check that both Kotlin and Java sources are processed
+
+        // kotlin:
+        assertContains(htmlFiles, "-dokka -example/it.basic/index.html")
+        assertContains(htmlFiles, "-dokka -example/it.basic/-public-class/public-documented-function.html")
+
+        // java:
+        assertContains(htmlFiles, "-dokka -example/it.basic.java/index.html")
+        assertContains(htmlFiles, "-dokka -example/it.basic.java/-sample-java-class/public-documented-function.html")
+    }
 }
diff --git a/...lysis/kotlin/descriptors/compiler/translator/DefaultDescriptorToDocumentableTranslator.kt b/...lysis/kotlin/descriptors/compiler/translator/DefaultDescriptorToDocumentableTranslator.kt
@@ -168,9 +168,14 @@ private class DokkaDescriptorVisitor(
     private val syntheticDocProvider = SyntheticDescriptorDocumentationProvider(kDocFinder, sourceSet)
 
     private fun Collection<DeclarationDescriptor>.filterDescriptorsInSourceSet() = filter {
-        it.toSourceElement.containingFile.toString().let { path ->
-            path.isNotBlank() && sourceSet.sourceRoots.any { root ->
-                Paths.get(path).startsWith(root.toPath())
+        val pathString = it.toSourceElement.containingFile.toString()
+        when {
+            pathString.isBlank() -> false
+            else -> {
+                val absolutePath = Paths.get(pathString).toRealPath()
+                sourceSet.sourceRoots.any { root ->
+                    absolutePath.startsWith(root.toPath().toRealPath())
+                }
             }
         }
     }

diff --git a/...brains/dokka/analysis/kotlin/symbols/translators/DefaultSymbolToDocumentableTranslator.kt b/...brains/dokka/analysis/kotlin/symbols/translators/DefaultSymbolToDocumentableTranslator.kt
@@ -46,7 +46,6 @@ import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.hasActualModifier
 import org.jetbrains.kotlin.psi.psiUtil.hasExpectModifier
-import java.nio.file.Paths
 
 internal class DefaultSymbolToDocumentableTranslator(context: DokkaContext) : AsyncSourceToDocumentableTranslator {
     private val kotlinAnalysis = context.plugin<SymbolsAnalysisPlugin>().querySingle { kotlinAnalysis }
@@ -111,46 +110,46 @@ internal class DokkaSymbolVisitor(
     private val KtDeclarationSymbol.isExpect
         get() = (psi as? KtModifierListOwner)?.hasExpectModifier() == true
 
-    private fun <T : KtSymbol> Collection<T>.filterSymbolsInSourceSet() = filter {
-        it.psi?.containingFile?.virtualFile?.path?.let { path ->
-            path.isNotBlank() && sourceSet.sourceRoots.any { root ->
-                Paths.get(path).startsWith(root.toPath())
-            }
-        } == true
+    private fun <T : KtSymbol> List<T>.filterSymbolsInSourceSet(moduleFiles: Set<KtFile>): List<T> = filter {
+        when (val file = it.psi?.containingFile) {
+            is KtFile -> moduleFiles.contains(file)
+            else -> false
+        }
     }
 
     fun visitModule(): DModule {
         val sourceModule = analysisContext.getModule(sourceSet)
-        val ktFiles = analysisContext.modulesWithFiles[sourceModule]?.filterIsInstance<KtFile>() ?: throw IllegalStateException("No source files for a source module ${sourceModule.moduleName} of source set ${sourceSet.sourceSetID}")
+        val ktFiles = analysisContext.modulesWithFiles[sourceModule]?.filterIsInstance<KtFile>()?.toSet()
+            ?: throw IllegalStateException("No source files for a source module ${sourceModule.moduleName} of source set ${sourceSet.sourceSetID}")
         val processedPackages: MutableSet<FqName> = mutableSetOf()
         return analyze(sourceModule) {
-            val packageSymbols: List<DPackage> = ktFiles
-                .mapNotNull {
-                    if (processedPackages.contains(it.packageFqName))
-                        return@mapNotNull null
-                    processedPackages.add(it.packageFqName)
-                    getPackageSymbolIfPackageExists(it.packageFqName)?.let { it1 ->
-                        visitPackageSymbol(
-                            it1
-                        )
-                    }
+            val packages = ktFiles.mapNotNull { file ->
+                if (processedPackages.contains(file.packageFqName))
+                    return@mapNotNull null
+                processedPackages.add(file.packageFqName)
+                getPackageSymbolIfPackageExists(file.packageFqName)?.let { packageSymbol ->
+                    visitPackageSymbol(packageSymbol, ktFiles)
                 }
+            }
 
             DModule(
                 name = moduleName,
-                packages = packageSymbols,
+                packages = packages,
                 documentation = emptyMap(),
                 expectPresentInSet = null,
                 sourceSets = setOf(sourceSet)
             )
         }
     }
 
-    private fun KtAnalysisSession.visitPackageSymbol(packageSymbol: KtPackageSymbol): DPackage {
+    private fun KtAnalysisSession.visitPackageSymbol(
+        packageSymbol: KtPackageSymbol,
+        moduleFiles: Set<KtFile>
+    ): DPackage {
         val dri = getDRIFromPackage(packageSymbol)
         val scope = packageSymbol.getPackageScope()
-        val callables = scope.getCallableSymbols().toList().filterSymbolsInSourceSet()
-        val classifiers = scope.getClassifierSymbols().toList().filterSymbolsInSourceSet()
+        val callables = scope.getCallableSymbols().toList().filterSymbolsInSourceSet(moduleFiles)
+        val classifiers = scope.getClassifierSymbols().toList().filterSymbolsInSourceSet(moduleFiles)
 
         val functions = callables.filterIsInstance<KtFunctionSymbol>().map { visitFunctionSymbol(it, dri) }
         val properties = callables.filterIsInstance<KtPropertySymbol>().map { visitPropertySymbol(it, dri) }

diff --git a/dokka-subprojects/plugin-base/src/test/kotlin/content/inheritors/ContentForInheritorsTest.kt b/dokka-subprojects/plugin-base/src/test/kotlin/content/inheritors/ContentForInheritorsTest.kt
@@ -258,6 +258,11 @@ class ContentForInheritorsTest : BaseAbstractTest() {
                 |
                 |class Child : Parent()
                 |
+                |/src/linuxX64Main/kotlin/pageMerger/Test.kt
+                |package pageMerger
+                |
+                |class Child : Parent()
+                |
             """.trimMargin(),
             mppTestConfiguration
         ) {