Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation for synthetic Enum values() and valueOf() functions #2650

Merged
merged 8 commits into from
Sep 26, 2022
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.jetbrains.dokka.it

import java.net.URL
import kotlin.test.Test

class StdLibDocumentationIntegrationTest {

/**
* Documentation for Enum's synthetic values() and valueOf() functions is only present in source code,
* but not present in the descriptors. However, Dokka needs to generate documentation for these functions,
* so it ships with hardcoded kdoc templates.
*
* This test exists to make sure documentation for these hardcoded synthetic functions does not change,
* and fails if it does, indicating that it needs to be updated.
*/
@Test
fun shouldAssertEnumDocumentationHasNotChanged() {
val sourcesLink = "https://raw.githubusercontent.com/JetBrains/kotlin/master/core/builtins/native/kotlin/Enum.kt"
val sources = URL(sourcesLink).readText()

val expectedValuesDoc =
" /**\n" +
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
" * Returns an array containing the constants of this enum type, in the order they're declared.\n" +
" * This method may be used to iterate over the constants.\n" +
" * @values\n" +
" */"
check(sources.contains(expectedValuesDoc))

val expectedValueOfDoc =
" /**\n" +
" * Returns the enum constant of this type with the specified name. The string must match exactly " +
"an identifier used to declare an enum constant in this type. (Extraneous whitespace characters are not permitted.)\n" +
" * @throws IllegalArgumentException if this enum type has no constant with the specified name\n" +
" * @valueOf\n" +
" */"
check(sources.contains(expectedValueOfDoc))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.jetbrains.dokka.it.gradle.AbstractGradleIntegrationTest
import org.jetbrains.dokka.it.gradle.BuildVersions
import org.junit.runners.Parameterized
import java.io.File
import java.net.URL
import kotlin.test.*

class StdlibGradleIntegrationTest(override val versions: BuildVersions) : AbstractGradleIntegrationTest(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ private class DokkaDescriptorVisitor(
private val logger: DokkaLogger
) {
private val javadocParser = JavadocParser(logger, resolutionFacade)
private val syntheticDocProvider = SyntheticDescriptorDocumentationProvider(resolutionFacade)

private fun Collection<DeclarationDescriptor>.filterDescriptorsInSourceSet() = filter {
it.toSourceElement.containingFile.toString().let { path ->
Expand Down Expand Up @@ -577,8 +578,7 @@ private class DokkaDescriptorVisitor(
sources = actual,
visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(),
generics = generics.await(),
documentation = descriptor.takeIf { it.kind != CallableMemberDescriptor.Kind.SYNTHESIZED }
?.resolveDescriptorData() ?: emptyMap(),
documentation = descriptor.getDocumentation(),
modifier = descriptor.modifier().toSourceSetDependent(),
type = descriptor.returnType!!.toBound(),
sourceSets = setOf(sourceSet),
Expand All @@ -594,6 +594,15 @@ private class DokkaDescriptorVisitor(
}
}

private fun FunctionDescriptor.getDocumentation(): SourceSetDependent<DocumentationNode> {
val isSynthesized = this.kind == CallableMemberDescriptor.Kind.SYNTHESIZED
return if (isSynthesized) {
syntheticDocProvider.getDocumentation(this)?.toSourceSetDependent() ?: emptyMap()
} else {
this.resolveDescriptorData()
}
}

/**
* `createDRI` returns the DRI of the exact element and potential DRI of an element that is overriding it
* (It can be also FAKE_OVERRIDE which is in fact just inheritance of the symbol)
Expand All @@ -609,7 +618,7 @@ private class DokkaDescriptorVisitor(

private fun FunctionDescriptor.isObvious(): Boolean {
return kind == CallableMemberDescriptor.Kind.FAKE_OVERRIDE
|| kind == CallableMemberDescriptor.Kind.SYNTHESIZED
|| (kind == CallableMemberDescriptor.Kind.SYNTHESIZED && !syntheticDocProvider.isDocumented(this))
|| containingDeclaration.fqNameOrNull()?.isObvious() == true
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package org.jetbrains.dokka.base.translators.descriptors

import org.jetbrains.dokka.analysis.DokkaResolutionFacade
import org.jetbrains.dokka.analysis.from
import org.jetbrains.dokka.base.parsers.MarkdownParser
import org.jetbrains.dokka.links.DRI
import org.jetbrains.dokka.model.doc.DocumentationNode
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.idea.kdoc.resolveKDocLink
import org.jetbrains.kotlin.resolve.DescriptorFactory

private const val ENUM_VALUEOF_TEMPLATE_PATH = "/dokka/docs/kdoc/EnumValueOf.kt.template"
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
private const val ENUM_VALUES_TEMPLATE_PATH = "/dokka/docs/kdoc/EnumValues.kt.template"

internal class SyntheticDescriptorDocumentationProvider(
private val resolutionFacade: DokkaResolutionFacade
) {
fun isDocumented(descriptor: DeclarationDescriptor): Boolean = descriptor is FunctionDescriptor
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
&& (DescriptorFactory.isEnumValuesMethod(descriptor) || DescriptorFactory.isEnumValueOfMethod(descriptor))

fun getDocumentation(descriptor: DeclarationDescriptor): DocumentationNode? {
val function = descriptor as? FunctionDescriptor ?: return null
return when {
DescriptorFactory.isEnumValuesMethod(function) -> loadTemplate(descriptor, ENUM_VALUES_TEMPLATE_PATH)
DescriptorFactory.isEnumValueOfMethod(function) -> loadTemplate(descriptor, ENUM_VALUEOF_TEMPLATE_PATH)
else -> null
}
}

private fun loadTemplate(descriptor: DeclarationDescriptor, filePath: String): DocumentationNode? {
val kdoc = loadContent(filePath) ?: return null
val parser = MarkdownParser({ link -> resolveLink(descriptor, link)}, filePath)
return parser.parse(kdoc)
}

private fun loadContent(filePath: String): String? = javaClass.getResource(filePath)?.readText()

private fun resolveLink(descriptor: DeclarationDescriptor, link: String): DRI? =
resolveKDocLink(
resolutionFacade.resolveSession.bindingContext,
resolutionFacade,
descriptor,
null,
link.split('.')
).firstOrNull()?.let { DRI.from(it) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import com.intellij.lang.jvm.annotation.JvmAnnotationEnumFieldValue
import com.intellij.lang.jvm.types.JvmReferenceType
import com.intellij.openapi.vfs.VirtualFileManager
import com.intellij.psi.*
import com.intellij.psi.impl.source.PsiClassReferenceType
import com.intellij.psi.impl.source.PsiImmediateClassType
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import org.jetbrains.dokka.DokkaConfiguration.DokkaSourceSet
Expand Down Expand Up @@ -99,8 +97,8 @@ class DefaultPsiToDocumentableTranslator(
facade: DokkaResolutionFacade,
private val logger: DokkaLogger
) {

private val javadocParser: JavaDocumentationParser = JavadocParser(logger, facade)
private val javadocParser = JavadocParser(logger, facade)
private val syntheticDocProvider = SyntheticElementDocumentationProvider(javadocParser, facade)

private val cachedBounds = hashMapOf<String, Bound>()

Expand Down Expand Up @@ -404,7 +402,7 @@ class DefaultPsiToDocumentableTranslator(
val dri = parentDRI?.let { dri ->
DRI.from(psi).copy(packageName = dri.packageName, classNames = dri.classNames)
} ?: DRI.from(psi)
val docs = javadocParser.parseDocumentation(psi)
val docs = psi.getDocumentation()
return DFunction(
dri = dri,
name = psi.name,
Expand Down Expand Up @@ -452,8 +450,13 @@ class DefaultPsiToDocumentableTranslator(
)
}

private fun PsiMethod.getDocumentation(): DocumentationNode =
this.takeIf { it is SyntheticElement }?.let { syntheticDocProvider.getDocumentation(it) }
?: javadocParser.parseDocumentation(this)

private fun PsiMethod.isObvious(inheritedFrom: DRI? = null): Boolean {
return this is SyntheticElement || inheritedFrom?.isObvious() == true
return (this is SyntheticElement && !syntheticDocProvider.isDocumented(this))
|| inheritedFrom?.isObvious() == true
}

private fun DRI.isObvious(): Boolean {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package org.jetbrains.dokka.base.translators.psi

import com.intellij.psi.*
import com.intellij.psi.javadoc.PsiDocComment
import org.jetbrains.dokka.analysis.DokkaResolutionFacade
import org.jetbrains.dokka.base.translators.psi.parsers.JavadocParser
import org.jetbrains.dokka.model.doc.DocumentationNode

private const val ENUM_VALUEOF_TEMPLATE_PATH = "/dokka/docs/javadoc/EnumValueOf.java.template"
private const val ENUM_VALUES_TEMPLATE_PATH = "/dokka/docs/javadoc/EnumValues.java.template"

internal class SyntheticElementDocumentationProvider(
private val javadocParser: JavadocParser,
private val resolutionFacade: DokkaResolutionFacade
) {
fun isDocumented(psiElement: PsiElement): Boolean = psiElement is PsiMethod
&& (psiElement.isSyntheticEnumValuesMethod() || psiElement.isSyntheticEnumValueOfMethod())

fun getDocumentation(psiElement: PsiElement): DocumentationNode? {
val psiMethod = psiElement as? PsiMethod ?: return null
val templatePath = when {
psiMethod.isSyntheticEnumValuesMethod() -> ENUM_VALUES_TEMPLATE_PATH
psiMethod.isSyntheticEnumValueOfMethod() -> ENUM_VALUEOF_TEMPLATE_PATH
else -> return null
}
val docComment = loadSyntheticDoc(templatePath) ?: return null
return javadocParser.parseDocComment(docComment, psiElement)
}

private fun loadSyntheticDoc(path: String): PsiDocComment? {
val text = javaClass.getResource(path)?.readText() ?: return null
return JavaPsiFacade.getElementFactory(resolutionFacade.project).createDocCommentFromText(text)
}
}

private fun PsiMethod.isSyntheticEnumValuesMethod() = this.isSyntheticEnumFunction() && this.name == "values"
private fun PsiMethod.isSyntheticEnumValueOfMethod() = this.isSyntheticEnumFunction() && this.name == "valueOf"
private fun PsiMethod.isSyntheticEnumFunction() = this is SyntheticElement && this.containingClass?.isEnum == true

Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,13 @@ class JavadocParser(

override fun parseDocumentation(element: PsiNamedElement): DocumentationNode {
return when(val comment = findClosestDocComment(element, logger)){
is JavaDocComment -> parseDocumentation(comment, element)
is JavaDocComment -> parseDocComment(comment.comment, element)
is KotlinDocComment -> parseDocumentation(comment)
else -> DocumentationNode(emptyList())
}
}

private fun parseDocumentation(element: JavaDocComment, context: PsiNamedElement): DocumentationNode {
val docComment = element.comment
internal fun parseDocComment(docComment: PsiDocComment, context: PsiNamedElement): DocumentationNode {
val nodes = listOfNotNull(docComment.getDescription()) + docComment.tags.mapNotNull { tag ->
parseDocTag(tag, docComment, context)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Returns the enum constant of this type with the specified
* name.
* The string must match exactly an identifier used to declare
* an enum constant in this type. (Extraneous whitespace
* characters are not permitted.)
*
* @return the enum constant with the specified name
* @throws IllegalArgumentException if this enum type has no
* constant with the specified name
*/

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Returns an array containing the constants of this enum
* type, in the order they're declared. This method may be
* used to iterate over the constants.
*
* @return an array containing the constants of this enum
* type, in the order they're declared
*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Returns the enum constant of this type with the specified name. The string must match exactly an identifier used
to declare an enum constant in this type. (Extraneous whitespace characters are not permitted.)

@throws kotlin.IllegalArgumentException if this enum type has no constant with the specified name
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Returns an array containing the constants of this enum type, in the order they're declared.

This method may be used to iterate over the constants.
46 changes: 0 additions & 46 deletions plugins/base/src/test/kotlin/enums/JavaEnumsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -68,50 +68,4 @@ class JavaEnumsTest : BaseAbstractTest() {
}
}
}

@Test
fun `should mark synthetic functions generated for Kotlin as obvious`() {
val writerPlugin = TestOutputWriterPlugin()
testInline(
"""
|/src/main/java/basic/JavaEnum.java
|package testpackage
|
|public enum JavaEnum {
| ONE, TWO
|}
""".trimMargin(),
basicConfiguration,
pluginOverrides = listOf(writerPlugin)
) {
documentablesCreationStage = { modules ->
val pckg = modules.flatMap { it.packages }.single { it.packageName == "testpackage" }
val enum = pckg.children.single { it is DEnum } as DEnum

// there's two with the same name, one inherited from
// java.lang.Enum and one is synthetic for Kotlin interop
enum.functions.filter { it.name == "valueOf" }.let { valueOfMethods ->
assertEquals(2, valueOfMethods.size)

val valueOfFromKotlin = valueOfMethods[0]
assertEquals(
"testpackage/JavaEnum/valueOf/#java.lang.String/PointingToDeclaration/",
valueOfFromKotlin.dri.toString()
)
assertNotNull(valueOfFromKotlin.extra[ObviousMember])

val valueOfFromJava = valueOfMethods[1]
assertEquals(
"java.lang/Enum/valueOf/#java.lang.Class<T>#java.lang.String/PointingToDeclaration/",
valueOfFromJava.dri.toString()
)
assertNotNull(valueOfFromJava.extra[ObviousMember])
}

val valuesMethod = enum.functions.single { it.name == "values" }
assertEquals("testpackage/JavaEnum/values/#/PointingToDeclaration/", valuesMethod.dri.toString())
assertNotNull(valuesMethod.extra[ObviousMember])
}
}
}
}
2 changes: 1 addition & 1 deletion plugins/base/src/test/kotlin/enums/KotlinEnumsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class KotlinEnumsTest : BaseAbstractTest() {
val testEnumNode = packagePage.children[0]
assertEquals("TestEnum", testEnumNode.name)

val enumEntries = testEnumNode.children
val enumEntries = testEnumNode.children.filterIsInstance<ClasslikePage>()
assertEquals(10, enumEntries.size)

assertEquals("ZERO", enumEntries[0].name)
Expand Down
Loading