Skip to content

Commit a76b0ec

Browse files
vmishenevclaude
andcommitted
Don't render broken links to symbols excluded from documentation (#4448)
When a KDoc/signature link references a symbol that is not part of the rendered documentation (suppressed via `perPackageOption`/`suppress`, hidden by `skipDeprecated`, `internal`, a private constructor, or an undocumented external symbol), the link resolves to a valid DRI during analysis but the target page is never generated. Dokka silently produced a link to a non-existent page and emitted no warning. `DefaultExternalModuleLinkResolver.resolve()` resolved links purely from a module's package-list, and only verified the target page existed when several modules shared a package name (the #3368 hack). It now always verifies the page exists (dropping the `#anchor` first so member links are not falsely rejected); a missing page yields an unresolved span instead of a dead `<a href>`. In addition, both the multi-module assembly (`ResolveLinkCommandHandler`) and single-module rendering (`HtmlRenderer.buildDRILink`) now warn when a documentation link cannot be resolved to a page. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9531f10 commit a76b0ec

7 files changed

Lines changed: 158 additions & 17 deletions

File tree

dokka-integration-tests/gradle/src/testExternalProjectKotlinxIo/kotlin/IoGradleIntegrationTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class IoGradleIntegrationTest : AbstractGradleIntegrationTest(), TestOutputCopie
6868
projectOutputLocation.allHtmlFiles().forEach { file ->
6969
assertContainsNoErrorClass(file)
7070
assertNoUnresolvedLinks(file)
71-
// assertNoHrefToMissingLocalFileOrDirectory(file)
71+
assertNoHrefToMissingLocalFileOrDirectory(file)
7272
assertNoEmptyLinks(file)
7373
assertNoEmptySpans(file)
7474
}

dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ExternalModuleLinkResolver.kt

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,14 @@ public class DefaultExternalModuleLinkResolver(
7272
val resolvedLinks = elps.mapNotNull { locProviderWithMdl ->
7373
locProviderWithMdl.locationProvider.resolve(dri)?.let { it to locProviderWithMdl.moduleDescription }
7474
}
75-
val validLink = resolvedLinks.firstOrNull {
76-
// this is a temporary hack for the first case (short-term solution) of #3368:
77-
// a situation where 2 (or more) local modules have the same package name.
78-
// TODO #3368 currently, it does not work for external modules with the same package name
79-
if (resolvedLinks.size > 1) {
80-
val moduleDescription = it.second
81-
val resolvedLinkWithoutRelativePath = it.first.removePrefix(
82-
"file:/" + moduleDescription.relativePathToOutputDirectory.toRelativeOutputDir()
83-
.toString()
84-
)
85-
86-
val partialModuleOutput = moduleDescription.sourceOutputDirectory
87-
val absolutePath = File(partialModuleOutput.absolutePath + resolvedLinkWithoutRelativePath)
88-
absolutePath.isFile
89-
} else true
75+
// A link is resolvable from a module's package-list as long as the package is documented,
76+
// but the symbol itself might be excluded from the documentation (suppressed, internal,
77+
// deprecated when `skipDeprecated` is on, a private constructor, etc.), so the target page
78+
// is never generated. Verifying that the page actually exists prevents rendering broken
79+
// links to non-existent pages (#4448). It also disambiguates between local modules that
80+
// share a package name (#3368).
81+
val validLink = resolvedLinks.firstOrNull { (link, moduleDescription) ->
82+
pointsToExistingPage(link, moduleDescription)
9083
}?.first ?: return null
9184

9285
// relativization [fileContext] path over output path (or `fileContext.relativeTo(outputPath)`)
@@ -105,6 +98,22 @@ public class DefaultExternalModuleLinkResolver(
10598
.joinToString("/") + validLink.removePrefix("file:")
10699
}
107100

101+
/**
102+
* Checks that the file backing a [resolvedLink] (produced by an [ExternalLocationProvider]
103+
* from a module's package-list) actually exists in the module's partial output.
104+
*
105+
* The anchor (`#...`) is dropped first because member links resolve to `index.html#anchor`,
106+
* and only the `.html` page on disk should be checked for existence.
107+
*/
108+
private fun pointsToExistingPage(resolvedLink: String, moduleDescription: DokkaModuleDescription): Boolean {
109+
val relativePathPrefix = "file:/" + moduleDescription.relativePathToOutputDirectory.toRelativeOutputDir().toString()
110+
val resolvedLinkWithoutRelativePath = resolvedLink
111+
.substringBefore('#')
112+
.removePrefix(relativePathPrefix)
113+
val absolutePath = File(moduleDescription.sourceOutputDirectory.absolutePath + resolvedLinkWithoutRelativePath)
114+
return absolutePath.isFile
115+
}
116+
108117
override fun resolveLinkToModuleIndex(moduleName: String): String? =
109118
context.configuration.modules.firstOrNull { it.name == moduleName }
110119
?.let { module ->

dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ResolveLinkCommandHandler.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import org.jsoup.nodes.Element
1515
import org.jsoup.parser.Tag
1616
import java.io.File
1717

18-
public class ResolveLinkCommandHandler(context: DokkaContext) : CommandHandler {
18+
public class ResolveLinkCommandHandler(private val context: DokkaContext) : CommandHandler {
1919

2020
private val externalModuleLinkResolver =
2121
context.plugin<AllModulesPagePlugin>().querySingle { externalModuleLinkResolver }
@@ -24,6 +24,10 @@ public class ResolveLinkCommandHandler(context: DokkaContext) : CommandHandler {
2424
command as ResolveLinkCommand
2525
val link = externalModuleLinkResolver.resolve(command.dri, output)
2626
if (link == null) {
27+
context.logger.warn(
28+
"Couldn't resolve link to `${command.dri}`: the target is not part of the documentation " +
29+
"(it may be suppressed, internal, deprecated, or an undocumented external symbol)"
30+
)
2731
val children = body.childNodes().toList()
2832
val attributes = Attributes().apply {
2933
put("data-unresolved-link", command.dri.toString())

dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkCommandResolutionTest.kt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,44 @@ class ResolveLinkCommandResolutionTest : MultiModuleAbstractTest() {
4141
}
4242

4343
val contentFile = setup(outputDirectory, link)
44+
// The page the link points to must actually exist, otherwise it is treated as a
45+
// broken link to a symbol excluded from the documentation (see #4448).
46+
outputDirectory.resolve("module2/package2/-sample/index.html")
47+
.also { assertTrue(it.parentFile.mkdirs()) }
48+
.writeText("<html></html>")
49+
val configuration = createConfiguration(outputDirectory)
50+
51+
testFromData(configuration, useOutputLocationFromConfig = true) {
52+
finishProcessingSubmodules = {
53+
assertHtmlEqualsIgnoringWhitespace(expected, contentFile.readText())
54+
}
55+
}
56+
}
57+
58+
@Test
59+
fun `should not resolve link to a symbol excluded from the documentation`(@TempDir outputDirectory: File) {
60+
// The package is documented (present in the package-list), but the symbol itself is
61+
// excluded, so its page is never generated. The link must not be rendered as a broken
62+
// link to a non-existent page (#4448).
63+
val testedDri = DRI(
64+
packageName = "package2",
65+
classNames = "Excluded",
66+
)
67+
val link = createHTML().templateCommand(ResolveLinkCommand(testedDri)) {
68+
span {
69+
+"Excluded"
70+
}
71+
}
72+
73+
val expected = createHTML().span {
74+
attributes["data-unresolved-link"] = testedDri.toString()
75+
span {
76+
+"Excluded"
77+
}
78+
}
79+
80+
val contentFile = setup(outputDirectory, link)
81+
// Note: no page file is created for `package2/-excluded`, mimicking an excluded symbol.
4482
val configuration = createConfiguration(outputDirectory)
4583

4684
testFromData(configuration, useOutputLocationFromConfig = true) {

dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkGfmCommandResolutionTest.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ class ResolveLinkGfmCommandResolutionTest : MultiModuleAbstractTest() {
6161

6262
indexMd.writeText(indexMdContent)
6363
packageList.writeText(mockedPackageListForPackages(RecognizedLinkFormat.DokkaGFM, "package2"))
64+
// The page the link points to must actually exist, otherwise it is treated as a
65+
// broken link to a symbol excluded from the documentation (see #4448).
66+
innerModule2.resolve("package2/-sample/index.md")
67+
.also { assertTrue(it.parentFile.mkdirs()) }
68+
.writeText("")
6469

6570
testFromData(
6671
configuration,

dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/renderers/html/HtmlRenderer.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,10 +823,16 @@ public open class HtmlRenderer(
823823
buildText(node.children, pageContext, sourceSetRestriction)
824824
}
825825
} ?: if (isPartial) {
826+
// The link may still be resolved against another module during multi-module
827+
// assembly, so defer the decision (and any warning) to ResolveLinkCommandHandler.
826828
templateCommand(ResolveLinkCommand(node.address)) {
827829
buildText(node.children, pageContext, sourceSetRestriction)
828830
}
829831
} else {
832+
context.logger.warn(
833+
"Couldn't resolve link to `${node.address}`: the target is not part of the documentation " +
834+
"(it may be suppressed, internal, deprecated, or an undocumented external symbol)"
835+
)
830836
span {
831837
attributes["data-unresolved-link"] = node.address.toString().htmlEscape()
832838
buildText(node.children, pageContext, sourceSetRestriction)
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package markdown
6+
7+
import org.jetbrains.dokka.DokkaException
8+
import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
9+
import utils.TestOutputWriterPlugin
10+
import kotlin.test.Test
11+
import kotlin.test.assertFailsWith
12+
import kotlin.test.assertFalse
13+
import kotlin.test.assertTrue
14+
15+
/**
16+
* A KDoc link to a symbol that is excluded from the documentation (suppressed, `internal`,
17+
* deprecated when `skipDeprecated` is on, etc.) resolves to a valid `DRI` during analysis,
18+
* but the symbol has no page. Such a link must not be rendered as a working `<a>` link to a
19+
* non-existent page, and Dokka should warn the developer about it (#4448).
20+
*/
21+
class LinkToExcludedSymbolTest : BaseAbstractTest() {
22+
23+
private val configuration = dokkaConfiguration {
24+
sourceSets {
25+
sourceSet {
26+
sourceRoots = listOf("src/")
27+
}
28+
}
29+
}
30+
31+
private val source = """
32+
|/src/main/kotlin/test/Test.kt
33+
|package test
34+
|
35+
|internal class Hidden
36+
|
37+
|/**
38+
| * See [Hidden] for details.
39+
| */
40+
|public class Public
41+
""".trimMargin()
42+
43+
@Test
44+
fun `link to excluded symbol is rendered as unresolved span and warns`() {
45+
val writerPlugin = TestOutputWriterPlugin()
46+
testInline(source, configuration, pluginOverrides = listOf(writerPlugin)) {
47+
renderingStage = { _, _ ->
48+
val content = writerPlugin.writer.contents.getValue("root/test/-public/index.html")
49+
assertTrue(
50+
content.contains("""data-unresolved-link="test/Hidden///PointingToDeclaration/""""),
51+
"Expected an unresolved-link span for the excluded symbol, but got:\n$content"
52+
)
53+
assertFalse(
54+
content.contains(Regex("""<a [^>]*href="[^"]*Hidden""")),
55+
"A link to an excluded symbol must not be rendered as a working <a> link:\n$content"
56+
)
57+
assertTrue(
58+
logger.warnMessages.any { it.contains("test/Hidden") && it.contains("Couldn't resolve link") },
59+
"Expected a warning about the unresolved link, but got: ${logger.warnMessages}"
60+
)
61+
}
62+
}
63+
}
64+
65+
@Test
66+
fun `link to excluded symbol fails the build when failOnWarning is enabled`() {
67+
val failOnWarningConfiguration = dokkaConfiguration {
68+
failOnWarning = true
69+
sourceSets {
70+
sourceSet {
71+
sourceRoots = listOf("src/")
72+
}
73+
}
74+
}
75+
assertFailsWith<DokkaException> {
76+
testInline(source, failOnWarningConfiguration) {}
77+
}
78+
}
79+
}

0 commit comments

Comments
 (0)