Use canonicalPath instead of absolutePath for srcLink

This way a user can define "./" instead of an absolute path to the
root of the project dir (or a user can use ../ etc.). Thus:
- use canonicalPath in:
  - SourceLinkDefinitionImpl::parseSourceLinkDefinition
  - and DocumentationNode.appendSourceLink => here because if the
    config is deserialized we bypass parseSourceLinkDefinition
  - also use canonicalPath for the path of PsiElement

Moreover:
- make sure the comparison works for unix and windows paths
- fixes #289

5 files changed, 149 insertions, 13 deletions

diff --git a/core/src/main/kotlin/Generation/configurationImpl.kt b/core/src/main/kotlin/Generation/configurationImpl.kt
index 90e27b4b..eecf122e 100644
--- a/core/src/main/kotlin/Generation/configurationImpl.kt
+++ b/core/src/main/kotlin/Generation/configurationImpl.kt
@@ -11,9 +11,9 @@ data class SourceLinkDefinitionImpl(override val path: String,
     companion object {
         fun parseSourceLinkDefinition(srcLink: String): SourceLinkDefinition {
             val (path, urlAndLine) = srcLink.split('=')
-            return SourceLinkDefinitionImpl(File(path).absolutePath,
+            return SourceLinkDefinitionImpl(File(path).canonicalPath,
                     urlAndLine.substringBefore("#"),
-                    urlAndLine.substringAfter("#", "").let { if (it.isEmpty()) null else "#" + it })
+                    urlAndLine.substringAfter("#", "").let { if (it.isEmpty()) null else "#$it" })
         }
     }
 }
diff --git a/core/src/main/kotlin/Model/SourceLinks.kt b/core/src/main/kotlin/Model/SourceLinks.kt
index 2c75cfda..99ee362e 100644
--- a/core/src/main/kotlin/Model/SourceLinks.kt
+++ b/core/src/main/kotlin/Model/SourceLinks.kt
@@ -10,20 +10,20 @@ import java.io.File
 
 fun DocumentationNode.appendSourceLink(psi: PsiElement?, sourceLinks: List<SourceLinkDefinition>) {
     val path = psi?.containingFile?.virtualFile?.path ?: return
+    val canonicalPath = File(path).canonicalPath
 
     val target = if (psi is PsiNameIdentifierOwner) psi.nameIdentifier else psi
-    val absPath = File(path).absolutePath
-    val linkDef = sourceLinks.firstOrNull { absPath.startsWith(it.path) }
-    if (linkDef != null) {
-        var url = linkDef.url + path.substring(linkDef.path.length)
-        if (linkDef.lineSuffix != null) {
+    val pair = determineSourceLinkDefinition(canonicalPath, sourceLinks)
+    if (pair != null) {
+        val (sourceLinkDefinition, sourceLinkCanonicalPath) = pair
+        var url = determineUrl(canonicalPath, sourceLinkDefinition, sourceLinkCanonicalPath)
+        if (sourceLinkDefinition.lineSuffix != null) {
             val line = target?.lineNumber()
             if (line != null) {
-                url += linkDef.lineSuffix + line.toString()
+                url += sourceLinkDefinition.lineSuffix + line.toString()
             }
         }
-        append(DocumentationNode(url, Content.Empty, NodeKind.SourceUrl),
-                RefKind.Detail);
+        append(DocumentationNode(url, Content.Empty, NodeKind.SourceUrl), RefKind.Detail)
     }
 
     if (target != null) {
@@ -31,6 +31,28 @@ fun DocumentationNode.appendSourceLink(psi: PsiElement?, sourceLinks: List<Sourc
     }
 }
 
+private fun determineSourceLinkDefinition(
+    canonicalPath: String,
+    sourceLinks: List<SourceLinkDefinition>
+): Pair<SourceLinkDefinition, String>? {
+    return sourceLinks
+        .asSequence()
+        .map { it to File(it.path).canonicalPath }
+        .firstOrNull { (_, sourceLinkCanonicalPath) ->
+            canonicalPath.startsWith(sourceLinkCanonicalPath)
+        }
+}
+
+private fun determineUrl(
+    canonicalPath: String,
+    sourceLinkDefinition: SourceLinkDefinition,
+    sourceLinkCanonicalPath: String
+): String {
+    val relativePath = canonicalPath.substring(sourceLinkCanonicalPath.length)
+    val relativeUrl = relativePath.replace('\\', '/').removePrefix("/")
+    return "${sourceLinkDefinition.url.removeSuffix("/")}/$relativeUrl"
+}
+
 private fun PsiElement.sourcePosition(): String {
     val path = containingFile.virtualFile.path
     val lineNumber = lineNumber()
diff --git a/core/src/test/kotlin/TestAPI.kt b/core/src/test/kotlin/TestAPI.kt
index f72373d9..4f77a2f6 100644
--- a/core/src/test/kotlin/TestAPI.kt
+++ b/core/src/test/kotlin/TestAPI.kt
@@ -6,6 +6,7 @@ import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.util.io.FileUtil
 import com.intellij.rt.execution.junit.FileComparisonFailure
 import org.jetbrains.dokka.*
+import org.jetbrains.dokka.DokkaConfiguration.SourceLinkDefinition
 import org.jetbrains.dokka.Utilities.DokkaAnalysisModule
 import org.jetbrains.kotlin.cli.common.config.ContentRoot
 import org.jetbrains.kotlin.cli.common.config.KotlinSourceRoot
@@ -26,6 +27,7 @@ fun verifyModel(vararg roots: ContentRoot,
                 perPackageOptions: List<DokkaConfiguration.PackageOptions> = emptyList(),
                 noStdlibLink: Boolean = true,
                 collectInheritedExtensionsFromLibraries: Boolean = false,
+                sourceLinks: List<SourceLinkDefinition> = emptyList(),
                 verifier: (DocumentationModule) -> Unit) {
     val documentation = DocumentationModule("test")
 
@@ -35,7 +37,7 @@ fun verifyModel(vararg roots: ContentRoot,
         includeNonPublic = includeNonPublic,
         skipEmptyPackages = false,
         includeRootPackage = true,
-        sourceLinks = listOf(),
+        sourceLinks = sourceLinks,
         perPackageOptions = perPackageOptions,
         generateIndexPages = false,
         noStdlibLink = noStdlibLink,
@@ -113,15 +115,17 @@ fun verifyModel(source: String,
                 withKotlinRuntime: Boolean = false,
                 format: String = "html",
                 includeNonPublic: Boolean = true,
+                sourceLinks: List<SourceLinkDefinition> = emptyList(),
                 verifier: (DocumentationModule) -> Unit) {
-    if (!File(source).exists()) {
-        throw IllegalArgumentException("Can't find test data file $source")
+    require (File(source).exists()) {
+        "Cannot find test data file $source"
     }
     verifyModel(contentRootFromPath(source),
             withJdk = withJdk,
             withKotlinRuntime = withKotlinRuntime,
             format = format,
             includeNonPublic = includeNonPublic,
+            sourceLinks = sourceLinks,
             verifier = verifier)
 }
 
diff --git a/core/src/test/kotlin/model/SourceLinksErrorTest.kt b/core/src/test/kotlin/model/SourceLinksErrorTest.kt
new file mode 100644
index 00000000..a72bd62a
--- /dev/null
+++ b/core/src/test/kotlin/model/SourceLinksErrorTest.kt
@@ -0,0 +1,34 @@
+package org.jetbrains.dokka.tests.model
+
+import org.jetbrains.dokka.NodeKind
+import org.jetbrains.dokka.SourceLinkDefinitionImpl
+import org.jetbrains.dokka.tests.verifyModel
+import org.junit.Assert
+import org.junit.Test
+import java.io.File
+
+class SourceLinksErrorTest {
+
+    @Test
+    fun absolutePath_notMatching() {
+        val sourceLink = SourceLinkDefinitionImpl(File("testdata/nonExisting").absolutePath, "http://...", null)
+        verifyNoSourceUrl(sourceLink)
+    }
+
+    @Test
+    fun relativePath_notMatching() {
+        val sourceLink = SourceLinkDefinitionImpl("testdata/nonExisting", "http://...", null)
+        verifyNoSourceUrl(sourceLink)
+    }
+
+    private fun verifyNoSourceUrl(sourceLink: SourceLinkDefinitionImpl) {
+        verifyModel("testdata/sourceLinks/dummy.kt", sourceLinks = listOf(sourceLink)) { model ->
+            with(model.members.single().members.single()) {
+                Assert.assertEquals("foo", name)
+                Assert.assertEquals(NodeKind.Function, kind)
+                Assert.assertTrue("should not have source urls", details(NodeKind.SourceUrl).isEmpty())
+            }
+        }
+    }
+}
+
diff --git a/core/src/test/kotlin/model/SourceLinksTest.kt b/core/src/test/kotlin/model/SourceLinksTest.kt
new file mode 100644
index 00000000..1a74506f
--- /dev/null
+++ b/core/src/test/kotlin/model/SourceLinksTest.kt
@@ -0,0 +1,76 @@
+package org.jetbrains.dokka.tests.model
+
+import org.jetbrains.dokka.NodeKind
+import org.jetbrains.dokka.SourceLinkDefinitionImpl
+import org.jetbrains.dokka.tests.verifyModel
+import org.junit.Assert
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import java.io.File
+
+@RunWith(Parameterized::class)
+class SourceLinksTest(
+    private val srcLink: String,
+    private val url: String,
+    private val lineSuffix: String?,
+    private val expectedUrl: String
+) {
+
+    @Test
+    fun test() {
+        val link = if(srcLink.contains(sourceLinks)){
+            srcLink.substringBeforeLast(sourceLinks) + sourceLinks
+        } else {
+            srcLink.substringBeforeLast(testdata) + testdata
+        }
+        val sourceLink = SourceLinkDefinitionImpl(link, url, lineSuffix)
+
+        verifyModel(filePath, sourceLinks = listOf(sourceLink)) { model ->
+            with(model.members.single().members.single()) {
+                Assert.assertEquals("foo", name)
+                Assert.assertEquals(NodeKind.Function, kind)
+                Assert.assertEquals(expectedUrl, details(NodeKind.SourceUrl).single().name)
+            }
+        }
+    }
+
+    companion object {
+        private const val testdata = "testdata"
+        private const val sourceLinks = "sourceLinks"
+        private const val dummy = "dummy.kt"
+        private const val pathSuffix = "$sourceLinks/$dummy"
+        private const val filePath = "$testdata/$pathSuffix/../dummy.kt"
+        private const val url = "https://example.com"
+
+        @Parameterized.Parameters(name = "{index}: {0}, {1}, {2} = {3}")
+        @JvmStatic
+        fun data(): Collection<Array<String?>> {
+            val longestPath = File(testdata).absolutePath.removeSuffix("/") + "/../$testdata/"
+            val maxLength = longestPath.length
+            val list = listOf(
+                arrayOf(File(testdata).absolutePath.removeSuffix("/"), "$url/$pathSuffix"),
+                arrayOf(File("$testdata/$sourceLinks").absolutePath.removeSuffix("/") + "/", "$url/$dummy"),
+                arrayOf(longestPath, "$url/$pathSuffix"),
+
+                arrayOf(testdata, "$url/$pathSuffix"),
+                arrayOf("./$testdata", "$url/$pathSuffix"),
+                arrayOf("../core/$testdata", "$url/$pathSuffix"),
+                arrayOf("$testdata/$sourceLinks", "$url/$dummy"),
+                arrayOf("./$testdata/../$testdata/$sourceLinks", "$url/$dummy")
+            )
+            val allPaths = list +
+                    // we want to be sure Windows paths work as well
+                    list.map { arrayOf(it[0].replace('/', '\\'), it[1]) }
+            return allPaths.map { arrayOf(it[0].padEnd(maxLength, '_'), url, null, it[1]) } +
+                    listOf(
+                        // check that it also works if url ends with /
+                        arrayOf((File(testdata).absolutePath.removeSuffix("/") + "/").padEnd(maxLength, '_'), "$url/", null, "$url/$pathSuffix"),
+                        // check if line suffix work
+                        arrayOf<String?>("../core/../core/./$testdata/$sourceLinks/".padEnd(maxLength, '_'), "$url/", "#L", "$url/$dummy#L4")
+                    )
+        }
+    }
+
+}
+