diff --git a/dokka-integration-tests/gradle/src/testExternalProjectKotlinxDatetime/kotlin/DatetimeGradleIntegrationTest.kt b/dokka-integration-tests/gradle/src/testExternalProjectKotlinxDatetime/kotlin/DatetimeGradleIntegrationTest.kt index edb259fee9..37f0a17a58 100644 --- a/dokka-integration-tests/gradle/src/testExternalProjectKotlinxDatetime/kotlin/DatetimeGradleIntegrationTest.kt +++ b/dokka-integration-tests/gradle/src/testExternalProjectKotlinxDatetime/kotlin/DatetimeGradleIntegrationTest.kt @@ -68,7 +68,7 @@ class DatetimeGradleIntegrationTest : AbstractGradleIntegrationTest(), TestOutpu projectOutputLocation.allHtmlFiles().forEach { file -> assertContainsNoErrorClass(file) assertNoUnresolvedLinks(file) - // assertNoHrefToMissingLocalFileOrDirectory(file) + assertNoHrefToMissingLocalFileOrDirectory(file) assertNoEmptyLinks(file) assertNoEmptySpans(file) } diff --git a/dokka-integration-tests/gradle/src/testExternalProjectKotlinxIo/kotlin/IoGradleIntegrationTest.kt b/dokka-integration-tests/gradle/src/testExternalProjectKotlinxIo/kotlin/IoGradleIntegrationTest.kt index 81bc0697ca..d04575014d 100644 --- a/dokka-integration-tests/gradle/src/testExternalProjectKotlinxIo/kotlin/IoGradleIntegrationTest.kt +++ b/dokka-integration-tests/gradle/src/testExternalProjectKotlinxIo/kotlin/IoGradleIntegrationTest.kt @@ -68,7 +68,7 @@ class IoGradleIntegrationTest : AbstractGradleIntegrationTest(), TestOutputCopie projectOutputLocation.allHtmlFiles().forEach { file -> assertContainsNoErrorClass(file) assertNoUnresolvedLinks(file) - // assertNoHrefToMissingLocalFileOrDirectory(file) + assertNoHrefToMissingLocalFileOrDirectory(file) assertNoEmptyLinks(file) assertNoEmptySpans(file) } diff --git a/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ExternalModuleLinkResolver.kt b/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ExternalModuleLinkResolver.kt index f64f6fa634..44c39b95ee 100644 --- a/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ExternalModuleLinkResolver.kt +++ b/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ExternalModuleLinkResolver.kt @@ -72,21 +72,14 @@ public class DefaultExternalModuleLinkResolver( val resolvedLinks = elps.mapNotNull { locProviderWithMdl -> locProviderWithMdl.locationProvider.resolve(dri)?.let { it to locProviderWithMdl.moduleDescription } } - val validLink = resolvedLinks.firstOrNull { - // this is a temporary hack for the first case (short-term solution) of #3368: - // a situation where 2 (or more) local modules have the same package name. - // TODO #3368 currently, it does not work for external modules with the same package name - if (resolvedLinks.size > 1) { - val moduleDescription = it.second - val resolvedLinkWithoutRelativePath = it.first.removePrefix( - "file:/" + moduleDescription.relativePathToOutputDirectory.toRelativeOutputDir() - .toString() - ) - - val partialModuleOutput = moduleDescription.sourceOutputDirectory - val absolutePath = File(partialModuleOutput.absolutePath + resolvedLinkWithoutRelativePath) - absolutePath.isFile - } else true + // A link is resolvable from a module's package-list as long as the package is documented, + // but the symbol itself might be excluded from the documentation (suppressed, internal, + // deprecated when `skipDeprecated` is on, a private constructor, etc.), so the target page + // is never generated. Verifying that the page actually exists prevents rendering broken + // links to non-existent pages (#4448). It also disambiguates between local modules that + // share a package name (#3368). + val validLink = resolvedLinks.firstOrNull { (link, moduleDescription) -> + pointsToExistingPage(link, moduleDescription) }?.first ?: return null // relativization [fileContext] path over output path (or `fileContext.relativeTo(outputPath)`) @@ -105,6 +98,22 @@ public class DefaultExternalModuleLinkResolver( .joinToString("/") + validLink.removePrefix("file:") } + /** + * Checks that the file backing a [resolvedLink] (produced by an [ExternalLocationProvider] + * from a module's package-list) actually exists in the module's partial output. + * + * The anchor (`#...`) is dropped first because member links resolve to `index.html#anchor`, + * and only the `.html` page on disk should be checked for existence. + */ + private fun pointsToExistingPage(resolvedLink: String, moduleDescription: DokkaModuleDescription): Boolean { + val relativePathPrefix = "file:/" + moduleDescription.relativePathToOutputDirectory.toRelativeOutputDir().toString() + val resolvedLinkWithoutRelativePath = resolvedLink + .substringBefore('#') + .removePrefix(relativePathPrefix) + val absolutePath = File(moduleDescription.sourceOutputDirectory.absolutePath + resolvedLinkWithoutRelativePath) + return absolutePath.isFile + } + override fun resolveLinkToModuleIndex(moduleName: String): String? = context.configuration.modules.firstOrNull { it.name == moduleName } ?.let { module -> diff --git a/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ResolveLinkCommandHandler.kt b/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ResolveLinkCommandHandler.kt index 31b57091b9..9bdc1fb885 100644 --- a/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ResolveLinkCommandHandler.kt +++ b/dokka-subprojects/plugin-all-modules-page/src/main/kotlin/org/jetbrains/dokka/allModulesPage/ResolveLinkCommandHandler.kt @@ -15,7 +15,7 @@ import org.jsoup.nodes.Element import org.jsoup.parser.Tag import java.io.File -public class ResolveLinkCommandHandler(context: DokkaContext) : CommandHandler { +public class ResolveLinkCommandHandler(private val context: DokkaContext) : CommandHandler { private val externalModuleLinkResolver = context.plugin().querySingle { externalModuleLinkResolver } @@ -24,6 +24,10 @@ public class ResolveLinkCommandHandler(context: DokkaContext) : CommandHandler { command as ResolveLinkCommand val link = externalModuleLinkResolver.resolve(command.dri, output) if (link == null) { + context.logger.warn( + "Couldn't resolve link to `${command.dri}`: the target is not part of the documentation " + + "(it may be suppressed, internal, deprecated, or an undocumented external symbol)" + ) val children = body.childNodes().toList() val attributes = Attributes().apply { put("data-unresolved-link", command.dri.toString()) diff --git a/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkCommandResolutionTest.kt b/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkCommandResolutionTest.kt index d13f263919..a3dc9fd345 100644 --- a/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkCommandResolutionTest.kt +++ b/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkCommandResolutionTest.kt @@ -41,6 +41,44 @@ class ResolveLinkCommandResolutionTest : MultiModuleAbstractTest() { } val contentFile = setup(outputDirectory, link) + // The page the link points to must actually exist, otherwise it is treated as a + // broken link to a symbol excluded from the documentation (see #4448). + outputDirectory.resolve("module2/package2/-sample/index.html") + .also { assertTrue(it.parentFile.mkdirs()) } + .writeText("") + val configuration = createConfiguration(outputDirectory) + + testFromData(configuration, useOutputLocationFromConfig = true) { + finishProcessingSubmodules = { + assertHtmlEqualsIgnoringWhitespace(expected, contentFile.readText()) + } + } + } + + @Test + fun `should not resolve link to a symbol excluded from the documentation`(@TempDir outputDirectory: File) { + // The package is documented (present in the package-list), but the symbol itself is + // excluded, so its page is never generated. The link must not be rendered as a broken + // link to a non-existent page (#4448). + val testedDri = DRI( + packageName = "package2", + classNames = "Excluded", + ) + val link = createHTML().templateCommand(ResolveLinkCommand(testedDri)) { + span { + +"Excluded" + } + } + + val expected = createHTML().span { + attributes["data-unresolved-link"] = testedDri.toString() + span { + +"Excluded" + } + } + + val contentFile = setup(outputDirectory, link) + // Note: no page file is created for `package2/-excluded`, mimicking an excluded symbol. val configuration = createConfiguration(outputDirectory) testFromData(configuration, useOutputLocationFromConfig = true) { diff --git a/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkGfmCommandResolutionTest.kt b/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkGfmCommandResolutionTest.kt index 23eb94eb6b..da3a5b110b 100644 --- a/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkGfmCommandResolutionTest.kt +++ b/dokka-subprojects/plugin-all-modules-page/src/test/kotlin/org/jetbrains/dokka/allModulesPage/templates/ResolveLinkGfmCommandResolutionTest.kt @@ -61,6 +61,11 @@ class ResolveLinkGfmCommandResolutionTest : MultiModuleAbstractTest() { indexMd.writeText(indexMdContent) packageList.writeText(mockedPackageListForPackages(RecognizedLinkFormat.DokkaGFM, "package2")) + // The page the link points to must actually exist, otherwise it is treated as a + // broken link to a symbol excluded from the documentation (see #4448). + innerModule2.resolve("package2/-sample/index.md") + .also { assertTrue(it.parentFile.mkdirs()) } + .writeText("") testFromData( configuration, diff --git a/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/renderers/html/HtmlRenderer.kt b/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/renderers/html/HtmlRenderer.kt index 02105ea37d..619d5a62b4 100644 --- a/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/renderers/html/HtmlRenderer.kt +++ b/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/renderers/html/HtmlRenderer.kt @@ -823,10 +823,16 @@ public open class HtmlRenderer( buildText(node.children, pageContext, sourceSetRestriction) } } ?: if (isPartial) { + // The link may still be resolved against another module during multi-module + // assembly, so defer the decision (and any warning) to ResolveLinkCommandHandler. templateCommand(ResolveLinkCommand(node.address)) { buildText(node.children, pageContext, sourceSetRestriction) } } else { + context.logger.warn( + "Couldn't resolve link to `${node.address}`: the target is not part of the documentation " + + "(it may be suppressed, internal, deprecated, or an undocumented external symbol)" + ) span { attributes["data-unresolved-link"] = node.address.toString().htmlEscape() buildText(node.children, pageContext, sourceSetRestriction) diff --git a/dokka-subprojects/plugin-base/src/test/kotlin/markdown/LinkToExcludedSymbolTest.kt b/dokka-subprojects/plugin-base/src/test/kotlin/markdown/LinkToExcludedSymbolTest.kt new file mode 100644 index 0000000000..71e0c7f47e --- /dev/null +++ b/dokka-subprojects/plugin-base/src/test/kotlin/markdown/LinkToExcludedSymbolTest.kt @@ -0,0 +1,79 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package markdown + +import org.jetbrains.dokka.DokkaException +import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest +import utils.TestOutputWriterPlugin +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * A KDoc link to a symbol that is excluded from the documentation (suppressed, `internal`, + * deprecated when `skipDeprecated` is on, etc.) resolves to a valid `DRI` during analysis, + * but the symbol has no page. Such a link must not be rendered as a working `` link to a + * non-existent page, and Dokka should warn the developer about it (#4448). + */ +class LinkToExcludedSymbolTest : BaseAbstractTest() { + + private val configuration = dokkaConfiguration { + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + } + } + } + + private val source = """ + |/src/main/kotlin/test/Test.kt + |package test + | + |internal class Hidden + | + |/** + | * See [Hidden] for details. + | */ + |public class Public + """.trimMargin() + + @Test + fun `link to excluded symbol is rendered as unresolved span and warns`() { + val writerPlugin = TestOutputWriterPlugin() + testInline(source, configuration, pluginOverrides = listOf(writerPlugin)) { + renderingStage = { _, _ -> + val content = writerPlugin.writer.contents.getValue("root/test/-public/index.html") + assertTrue( + content.contains("""data-unresolved-link="test/Hidden///PointingToDeclaration/""""), + "Expected an unresolved-link span for the excluded symbol, but got:\n$content" + ) + assertFalse( + content.contains(Regex("""]*href="[^"]*Hidden""")), + "A link to an excluded symbol must not be rendered as a working link:\n$content" + ) + assertTrue( + logger.warnMessages.any { it.contains("test/Hidden") && it.contains("Couldn't resolve link") }, + "Expected a warning about the unresolved link, but got: ${logger.warnMessages}" + ) + } + } + } + + @Test + fun `link to excluded symbol fails the build when failOnWarning is enabled`() { + val failOnWarningConfiguration = dokkaConfiguration { + failOnWarning = true + sourceSets { + sourceSet { + sourceRoots = listOf("src/") + } + } + } + assertFailsWith { + testInline(source, failOnWarningConfiguration) {} + } + } +}