Skip to content

Commit

Permalink
Fix some corner cases
Browse files Browse the repository at this point in the history
  • Loading branch information
IgnatBeresnev committed Oct 12, 2023
1 parent a300860 commit 59dc29d
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ internal class DefaultSamplesTransformer(val context: DokkaContext) : PageTransf
}

private fun createSampleBody(imports: List<String>, body: String) =
""" |${imports.takeIf { it.isNotEmpty() }?.joinToString(prefix = "import ") { "\n" } ?: ""}
""" |${imports.takeIf { it.isNotEmpty() }?.joinToString { "import $it\n" } ?: ""}
|fun main() {
| //sampleStart
| $body
Expand Down
21 changes: 11 additions & 10 deletions plugins/base/src/test/kotlin/linkableContent/LinkableContentTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package linkableContent

import org.jetbrains.dokka.SourceLinkDefinitionImpl
import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.jetbrains.dokka.base.transformers.pages.DefaultSamplesTransformer
import org.jetbrains.dokka.base.transformers.pages.sourcelinks.SourceLinksTransformer
import org.jetbrains.dokka.model.WithGenerics
import org.jetbrains.dokka.model.dfs
Expand Down Expand Up @@ -193,10 +194,8 @@ class LinkableContentTest : BaseAbstractTest() {
}

testFromData(configuration) {
renderingStage = { rootPageNode, _ ->
// TODO [beresnev] :(((
// val newRoot = DefaultSamplesTransformer(dokkaContext).invoke(rootPageNode)
val newRoot = rootPageNode
renderingStage = { rootPageNode, dokkaContext ->
val newRoot = DefaultSamplesTransformer(dokkaContext).invoke(rootPageNode)
val moduleChildren = newRoot.children
assertEquals(1, moduleChildren.size)
val packageChildren = moduleChildren.first().children
Expand All @@ -213,12 +212,14 @@ class LinkableContentTest : BaseAbstractTest() {
.let { it as ContentCodeBlock }.children.single()
.let { it as ContentText }.text
assertEquals(
"""|import p2.${name}Class
|fun main() {
| //sampleStart
| ${name}Class().printWithExclamation("Hi, $name")
| //sampleEnd
|}""".trimMargin(),
"""
|import p2.${name}Class
|
|fun main() {
| //sampleStart
| ${name}Class().printWithExclamation("Hi, $name")
| //sampleEnd
|}""".trimMargin(),
text
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import org.jetbrains.dokka.DokkaConfiguration
* It's best to limit the scope of use and lifetime of this environment as it takes up
* additional resources which could be freed once the samples have been analyzed.
* Therefore, it's best to use it through the [SampleAnalysisEnvironmentCreator.use] lambda.
*
* For example, if you need to process all samples in an arbitrary project, it's best to do it
* in one iteration and at the same time, so that the environment is created once and lives for
* as little is possible, as opposed to creating it again and again for every individual sample.
*/
public interface SampleAnalysisEnvironment {

Expand All @@ -22,12 +26,12 @@ public interface SampleAnalysisEnvironment {
* for which [DokkaConfiguration#samples] or [DokkaConfiguration#sourceRoots]
* have been configured with the sample's sources.
* @param fullyQualifiedLink fully qualified path to the sample function, including all middle packages
* and the name of the function. Only links to Kotlin functions are valid.
* The package must be the same as the package declared in the sample file.
* The function must be resolvable by Dokka, meaning it must reside either
* in the main sources of the project or its sources must be included in
* [DokkaConfiguration#samples] or [DokkaConfiguration#sourceRoots].
* Example: `com.example.pckg.topLevelKotlinFunction`
* and the name of the function. Only links to Kotlin functions are valid,
* which can reside within a class. The package must be the same as the package
* declared in the sample file. The function must be resolvable by Dokka,
* meaning it must reside either in the main sources of the project or its
* sources must be included in [DokkaConfiguration#samples] or
* [DokkaConfiguration#sourceRoots]. Example: `com.example.pckg.topLevelKotlinFunction`
*
* @return a sample code snippet which includes import statements and the function body,
* or null if the link could not be resolved (examine the logs to find out the reason).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class SampleAnalysisTest {
}

@Test
fun `should resolve a sample in the root package`() {
fun `should resolve a valid sample in the root package`() {
val testProject = kotlinJvmTestProject {
dokkaConfiguration {
kotlinSourceSet {
Expand Down Expand Up @@ -196,6 +196,80 @@ class SampleAnalysisTest {
}
}

@Test
fun `should resolve a valid sample function from a class in the root package`() {
val testProject = kotlinJvmTestProject {
dokkaConfiguration {
kotlinSourceSet {
samples = setOf("/samples/RootClassSample.kt")
}
}

sampleFile("/samples/RootClassSample.kt", fqPackageName = "") {
+"""
import org.jetbrains.dokka.DokkaConfiguration
class RootClass {
fun foo() {
println("hello from within a root class")
}
}
"""
}
}

testProject.useServices { context ->
val sample = sampleAnalysisEnvironmentCreator.use {
resolveSample(context.singleSourceSet(), "RootClass.foo")
}
assertNotNull(sample)

val expectedImports = listOf("org.jetbrains.dokka.DokkaConfiguration")
val expectedBody = "println(\"hello from within a root class\")"

assertEquals(expectedImports, sample.imports)
assertEquals(expectedBody, sample.body)
}
}

@Test
fun `should resolve a valid sample function from a class`() {
val testProject = kotlinJvmTestProject {
dokkaConfiguration {
kotlinSourceSet {
samples = setOf("/samples/SampleWithinClass.kt")
}
}

sampleFile("/samples/SampleWithinClass.kt", fqPackageName = "samples") {
+"""
import org.jetbrains.dokka.DokkaConfiguration
package samples
class SampleWithinClass {
fun foo() {
println("hello from within a class")
}
}
"""
}
}

testProject.useServices { context ->
val sample = sampleAnalysisEnvironmentCreator.use {
resolveSample(context.singleSourceSet(), "samples.SampleWithinClass.foo")
}
assertNotNull(sample)

val expectedImports = listOf("org.jetbrains.dokka.DokkaConfiguration")
val expectedBody = "println(\"hello from within a class\")"

assertEquals(expectedImports, sample.imports)
assertEquals(expectedBody, sample.body)
}
}

@Test
fun `should return null for non-existing sample`() {
val testProject = kotlinJvmTestProject {
Expand Down Expand Up @@ -258,7 +332,7 @@ class SampleAnalysisTest {
}

@Test
fun `should return null if sample is resolved by class name`() {
fun `should return null if sample is resolved just by class name`() {
val testProject = kotlinJvmTestProject {
dokkaConfiguration {
kotlinSourceSet {
Expand Down Expand Up @@ -457,7 +531,7 @@ class SampleAnalysisTest {
}

@Test
fun `should return equal hashcode for two equal sample snippets`() {
fun `should return same hashcode for two equal sample snippets`() {
val firstSnippet = createHardcodedSnippet()
val secondSnippet = createHardcodedSnippet()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,8 @@ import java.io.File

/**
* The default logger used for running Dokka and analyzing projects.
*
* Changing the level to [LoggingLevel.DEBUG] can help with debugging faulty tests
* or tricky corner cases.
*/
val defaultAnalysisLogger = DokkaConsoleLogger(minLevel = LoggingLevel.INFO)
val defaultAnalysisLogger = DokkaConsoleLogger(minLevel = LoggingLevel.DEBUG)

/**
* Analyzer of the test projects, it is essentially a very simple Dokka runner.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.psi.KtDeclarationWithBody
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
import org.jetbrains.kotlin.resolve.lazy.ResolveSession
import org.jetbrains.kotlin.resolve.lazy.descriptors.LazyPackageDescriptor
import org.jetbrains.kotlin.resolve.source.KotlinSourceElement

internal class DescriptorSampleAnalysisEnvironmentCreator(
Expand All @@ -37,6 +38,9 @@ internal class DescriptorSampleAnalysisEnvironmentCreator(
private val descriptorAnalysisPlugin = context.plugin<CompilerDescriptorAnalysisPlugin>()

override fun <T> use(block: SampleAnalysisEnvironment.() -> T): T {
// Run from the thread of Dispatchers.Default as it can help
// avoid memory leaks through the compiler's ThreadLocals.
// Might not be relevant if the project stops using coroutines.
return runBlocking(Dispatchers.Default) {
@OptIn(DokkaPluginApiPreview::class)
SamplesKotlinAnalysis(
Expand Down Expand Up @@ -86,13 +90,10 @@ internal class DescriptorSampleAnalysisEnvironment(
dokkaSourceSet: DokkaConfiguration.DokkaSourceSet,
fqLink: String,
): PsiElement? {
val isRootPackage = !fqLink.contains('.')
// supposed because we expect the last element to be a top-level function, not class+function
val supposedPackageName = if (isRootPackage) "" else fqLink.substringBeforeLast(".")
val packageDescriptor = this.getPackageFragment(FqName(supposedPackageName))
val packageDescriptor = resolveNearestPackageDescriptor(fqLink)
if (packageDescriptor == null) {
dokkaLogger.debug(
"Unable to resolve package descriptor for @sample: \"$fqLink\"; Package name: \"$supposedPackageName\""
"Unable to resolve package descriptor for @sample: \"$fqLink\";"
)
return null
}
Expand All @@ -107,7 +108,7 @@ internal class DescriptorSampleAnalysisEnvironment(
if (kdocLink == null) {
dokkaLogger.warn(
"Unable to resolve a @sample link: \"$fqLink\". Is it used correctly? " +
"Expecting a link to a reachable (resolvable) top-level Kotlin function."
"Expecting a link to a reachable (resolvable) Kotlin function."
)
return null
} else if (kdocLink.toSourceElement !is KotlinSourceElement) {
Expand All @@ -120,6 +121,49 @@ internal class DescriptorSampleAnalysisEnvironment(
return DescriptorToSourceUtils.descriptorToDeclaration(kdocLink)
}

/**
* Tries to resolve [fqLink]'s package.
*
* Since [fqLink] can be both a link to a top-level function and a link to a function within a class,
* we cannot tell for sure if [fqLink] contains a class name or not (relying on case letters is error-prone,
* there are exceptions). But we know for sure that the last element in the link is the function.
*
* So we start with what we think is the deepest package path, and if we cannot find a package descriptor
* for it - we drop one level and try again, until we find something or reach root.
*
* This function should also account for links to declarations within the root package (`""`).
*
* Here are some examples:
*
* Given [fqLink] = `com.example.ClassName.functionName`:
* 1) First pass, trying to resolve package `com.example.ClassName`. Failure.
* 2) Second pass, trying to resolve package `com.example`. Success.
*
* Given [fqLink] = `com.example.functionName`:
* 1) First pass, trying to resolve package `com.example`. Success.
*
* Given [fqLink] = `ClassName.functionName` (root package):
* 1) First pass, trying to resolve package `ClassName`. Failure.
* 2) Second pass, trying to resolve package `""`. Success.
*/
private fun ResolveSession.resolveNearestPackageDescriptor(fqLink: String): LazyPackageDescriptor? {
val isRootPackage = !fqLink.contains('.')
val supposedPackageName = if (isRootPackage) "" else fqLink.substringBeforeLast(".")

val packageDescriptor = this.getPackageFragment(FqName(supposedPackageName))
if (packageDescriptor != null) {
return packageDescriptor
}
dokkaLogger.debug("Failed to resolve package \"$supposedPackageName\" for sample \"$fqLink\"")

if (isRootPackage) {
// cannot go any deeper
return null
}

return resolveNearestPackageDescriptor(supposedPackageName.substringBeforeLast("."))
}

private fun processImports(sampleElement: PsiElement): List<String> {
val psiFile = sampleElement.containingFile

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ private class SymbolSampleAnalysisEnvironment(
private fun findPsiElement(sourceSet: DokkaSourceSet, fqLink: String): PsiElement? {
val analysisContext = kotlinAnalysis[sourceSet]
return analyze(analysisContext.mainModule) {
// TODO the logic below is incorrect as it assumes the samples can only link to top-level functions.
// TODO should be corrected to be able to work with functions inside classes. See Descriptor's impl.
val isRootPackage = !fqLink.contains('.')
// supposed because we expect the last element to be a top-level function, not class/class+function
val supposedFunctionName = if (isRootPackage) fqLink else fqLink.substringAfterLast(".")
val supposedPackageName = if (isRootPackage) "" else fqLink.substringBeforeLast(".")

Expand Down

0 comments on commit 59dc29d

Please sign in to comment.