Skip to content

Commit

Permalink
Fix incorrect handling of static members used within @see tag (#2627)
Browse files Browse the repository at this point in the history
  • Loading branch information
@IgnatBeresnev
IgnatBeresnev committed Aug 24, 2022
1 parent ee51388 commit 97628db
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 78 deletions.
23 changes: 18 additions & 5 deletions plugins/base/src/main/kotlin/parsers/MarkdownParser.kt
View file
Expand Up @@ -12,6 +12,7 @@ import org.intellij.markdown.flavours.gfm.GFMFlavourDescriptor
import org.intellij.markdown.flavours.gfm.GFMTokenTypes
import org.jetbrains.dokka.base.parsers.factories.DocTagsFromIElementFactory
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.links.PointingToDeclaration
import org.jetbrains.dokka.model.doc.*
import org.jetbrains.dokka.model.doc.Suppress
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
Expand Down Expand Up @@ -51,15 +52,15 @@ open class MarkdownParser(
val dri = externalDri(referencedName)
See(
parseStringToDocNode(content.substringAfter(' ')),
dri?.fqName() ?: referencedName,
dri?.fqDeclarationName() ?: referencedName,
dri
)
}
"throws", "exception" -> {
val dri = externalDri(content.substringBefore(' '))
Throws(
parseStringToDocNode(content.substringAfter(' ')),
dri?.fqName() ?: content.substringBefore(' '),
dri?.fqDeclarationName() ?: content.substringBefore(' '),
dri
)
}
Expand Down Expand Up @@ -523,15 +524,15 @@ open class MarkdownParser(
val dri = pointedLink(it)
Throws(
parseStringToDocNode(it.getContent()),
dri?.fqName() ?: it.getSubjectName().orEmpty(),
dri?.fqDeclarationName() ?: it.getSubjectName().orEmpty(),
dri,
)
}
KDocKnownTag.EXCEPTION -> {
val dri = pointedLink(it)
Throws(
parseStringToDocNode(it.getContent()),
dri?.fqName() ?: it.getSubjectName().orEmpty(),
dri?.fqDeclarationName() ?: it.getSubjectName().orEmpty(),
dri
)
}
Expand All @@ -545,7 +546,7 @@ open class MarkdownParser(
val dri = pointedLink(it)
See(
parseStringToDocNode(it.getContent()),
dri?.fqName() ?: it.getSubjectName().orEmpty(),
dri?.fqDeclarationName() ?: it.getSubjectName().orEmpty(),
dri,
)
}
Expand All @@ -567,8 +568,20 @@ open class MarkdownParser(
}

//Horrible hack but since link resolution is passed as a function i am not able to resolve them otherwise
@kotlin.Suppress("DeprecatedCallableAddReplaceWith")
@Deprecated("This function makes wrong assumptions and is missing a lot of corner cases related to generics, " +
"parameters and static members. This is not supposed to be public API and will not be supported in the future")
fun DRI.fqName(): String? = "$packageName.$classNames".takeIf { packageName != null && classNames != null }

private fun DRI.fqDeclarationName(): String? {
if (this.target !is PointingToDeclaration) {
return null
}
return listOfNotNull(this.packageName, this.classNames, this.callable?.name)
.joinToString(separator = ".")
.takeIf { it.isNotBlank() }
}

private fun findParent(kDoc: PsiElement): PsiElement =
if (kDoc.canHaveParent()) findParent(kDoc.parent) else kDoc

Expand Down
18 changes: 10 additions & 8 deletions plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt
View file
Expand Up @@ -11,6 +11,7 @@ import org.jetbrains.dokka.base.transformers.pages.comments.CommentsToContentCon
import org.jetbrains.dokka.base.transformers.pages.tags.CustomTagContentProvider
import org.jetbrains.dokka.base.translators.documentables.PageContentBuilder.DocumentableContentBuilder
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.links.PointingToDeclaration
import org.jetbrains.dokka.model.*
import org.jetbrains.dokka.model.doc.*
import org.jetbrains.dokka.model.properties.PropertyContainer
Expand Down Expand Up @@ -592,26 +593,26 @@ open class DefaultPageCreator(
availablePlatforms.forEach { platform ->
val possibleFallbacks = sourceSets.getPossibleFallbackSourcesets(platform)
seeAlsoTags.forEach { (_, see) ->
(see[platform] ?: see.fallback(possibleFallbacks))?.let {
(see[platform] ?: see.fallback(possibleFallbacks))?.let { seeTag ->
row(
sourceSets = setOf(platform),
kind = ContentKind.Comment,
styles = this@group.mainStyles,
) {
it.address?.let { dri ->
seeTag.address?.let { dri ->
link(
dri.classNames ?: it.name,
dri,
text = seeTag.name.removePrefix("${dri.packageName}."),
address = dri,
kind = ContentKind.Comment,
styles = mainStyles + ContentStyle.RowTitle
)
} ?: text(
it.name,
text = seeTag.name,
kind = ContentKind.Comment,
styles = mainStyles + ContentStyle.RowTitle
)
if (it.isNotEmpty()) {
comment(it.root)
if (seeTag.isNotEmpty()) {
comment(seeTag.root)
}
}
}
Expand Down Expand Up @@ -641,7 +642,8 @@ open class DefaultPageCreator(
row(sourceSets = setOf(sourceset)) {
group(styles = mainStyles + ContentStyle.RowTitle) {
throws.exceptionAddress?.let {
link(text = it.classNames ?: entry.key, address = it)
val className = it.takeIf { it.target is PointingToDeclaration }?.classNames
link(text = className ?: entry.key, address = it)
} ?: text(entry.key)
}
if (throws.isNotEmpty()) {
Expand Down
121 changes: 59 additions & 62 deletions plugins/base/src/test/kotlin/content/seealso/ContentForSeeAlsoTest.kt
View file
Expand Up @@ -517,76 +517,73 @@ class ContentForSeeAlsoTest : BaseAbstractTest() {
}

@Test
fun `undocumented see also from java`(){
fun `should prefix static function and property links with class name`() {
testInline(
"""
|/src/main/java/example/Source.java
|package example;
|/src/main/kotlin/com/example/package/CollectionExtensions.kt
|package com.example.util
|
|public interface Source {
| String getProperty(String k, String v);
|
| /**
| * @see #getProperty(String, String)
| */
| String getProperty(String k);
|object CollectionExtensions {
| val property = "Hi"
| fun emptyList() {}
|}
""".trimIndent(), testConfiguration
) {
documentablesTransformationStage = { module ->
val functionWithSeeTag = module.packages.flatMap { it.classlikes }.flatMap { it.functions }.find { it.name == "getProperty" && it.parameters.count() == 1 }
val seeTag = functionWithSeeTag?.docs()?.firstIsInstanceOrNull<See>()
val expectedLinkDestinationDRI = DRI(
packageName = "example",
classNames = "Source",
callable = Callable(
name = "getProperty",
params = listOf(JavaClassReference("java.lang.String"), JavaClassReference("java.lang.String"))
)
)

kotlin.test.assertNotNull(seeTag)
assertEquals("getProperty(String, String)", seeTag.name)
assertEquals(expectedLinkDestinationDRI, seeTag.address)
assertEquals(emptyList(), seeTag.children)
}
}
}

@Test
fun `documented see also from java`(){
testInline(
"""
|/src/main/java/example/Source.java
|package example;
|
|public interface Source {
| String getProperty(String k, String v);
|/src/main/kotlin/com/example/foo.kt
|package com.example
|
| /**
| * @see #getProperty(String, String) this is a reference to a method that is present on the same class.
| */
| String getProperty(String k);
|}
""".trimIndent(), testConfiguration
|import com.example.util.CollectionExtensions.property
|import com.example.util.CollectionExtensions.emptyList
|
|/**
| * @see [property] static property
| * @see [emptyList] static emptyList
| */
|fun function() {}
""".trimIndent(),
testConfiguration
) {
documentablesTransformationStage = { module ->
val functionWithSeeTag = module.packages.flatMap { it.classlikes }.flatMap { it.functions }.find { it.name == "getProperty" && it.parameters.size == 1 }
val seeTag = functionWithSeeTag?.docs()?.firstIsInstanceOrNull<See>()
val expectedLinkDestinationDRI = DRI(
packageName = "example",
classNames = "Source",
callable = Callable(
name = "getProperty",
params = listOf(JavaClassReference("java.lang.String"), JavaClassReference("java.lang.String"))
)
)
pagesTransformationStage = { module ->
val page = module.children.single { it.name == "com.example" }
.children.single { it.name == "function" } as ContentPage

kotlin.test.assertNotNull(seeTag)
assertEquals("getProperty(String, String)", seeTag.name)
assertEquals(expectedLinkDestinationDRI, seeTag.address)
assertEquals("this is a reference to a method that is present on the same class.", seeTag.children.first().text().trim())
assertEquals(1, seeTag.children.size)
page.content.assertNode {
group {
header(1) { +"function" }
}
divergentGroup {
divergentInstance {
divergent {
bareSignature(
annotations = emptyMap(),
visibility = "",
modifier = "",
keywords = emptySet(),
name = "function",
returnType = null,
)
}
after {
header(4) { +"See also" }
group {
table {
group {
link { +"CollectionExtensions.property" }
group {
group { +"static property" }
}
}
group {
link { +"CollectionExtensions.emptyList" }
group {
group { +"static emptyList" }
}
}
}
}
}
}
}
}
}
}
}
Expand Down
84 changes: 84 additions & 0 deletions plugins/base/src/test/kotlin/parsers/JavadocParserTest.kt
View file
Expand Up @@ -2,12 +2,18 @@ package parsers

import com.jetbrains.rd.util.first
import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.jetbrains.dokka.links.Callable
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.links.JavaClassReference
import org.jetbrains.dokka.model.DEnum
import org.jetbrains.dokka.model.DModule
import org.jetbrains.dokka.model.doc.*
import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import utils.docs
import utils.text
import kotlin.test.assertNotNull

class JavadocParserTest : BaseAbstractTest() {

Expand Down Expand Up @@ -428,4 +434,82 @@ class JavadocParserTest : BaseAbstractTest() {
}
}
}

@Test
fun `undocumented see also from java`(){
testInline(
"""
|/src/main/java/example/Source.java
|package example;
|
|public interface Source {
| String getProperty(String k, String v);
|
| /**
| * @see #getProperty(String, String)
| */
| String getProperty(String k);
|}
""".trimIndent(), configuration
) {
documentablesTransformationStage = { module ->
val functionWithSeeTag = module.packages.flatMap { it.classlikes }.flatMap { it.functions }.find { it.name == "getProperty" && it.parameters.count() == 1 }
val seeTag = functionWithSeeTag?.docs()?.firstIsInstanceOrNull<See>()
val expectedLinkDestinationDRI = DRI(
packageName = "example",
classNames = "Source",
callable = Callable(
name = "getProperty",
params = listOf(JavaClassReference("java.lang.String"), JavaClassReference("java.lang.String"))
)
)

assertNotNull(seeTag)
assertEquals("getProperty(String, String)", seeTag.name)
assertEquals(expectedLinkDestinationDRI, seeTag.address)
assertEquals(emptyList<DocTag>(), seeTag.children)
}
}
}

@Test
fun `documented see also from java`(){
testInline(
"""
|/src/main/java/example/Source.java
|package example;
|
|public interface Source {
| String getProperty(String k, String v);
|
| /**
| * @see #getProperty(String, String) this is a reference to a method that is present on the same class.
| */
| String getProperty(String k);
|}
""".trimIndent(), configuration
) {
documentablesTransformationStage = { module ->
val functionWithSeeTag = module.packages.flatMap { it.classlikes }.flatMap { it.functions }.find { it.name == "getProperty" && it.parameters.size == 1 }
val seeTag = functionWithSeeTag?.docs()?.firstIsInstanceOrNull<See>()
val expectedLinkDestinationDRI = DRI(
packageName = "example",
classNames = "Source",
callable = Callable(
name = "getProperty",
params = listOf(JavaClassReference("java.lang.String"), JavaClassReference("java.lang.String"))
)
)

assertNotNull(seeTag)
assertEquals("getProperty(String, String)", seeTag.name)
assertEquals(expectedLinkDestinationDRI, seeTag.address)
assertEquals(
"this is a reference to a method that is present on the same class.",
seeTag.children.first().text().trim()
)
assertEquals(1, seeTag.children.size)
}
}
}
}

0 comments on commit 97628db

Please sign in to comment.