Skip to content

Commit

Permalink
Update messages for intersecting source roots and add a check for int…
Browse files Browse the repository at this point in the history 
…ersecting samples (#3806)
  • Loading branch information
@vmishenev
vmishenev authored Sep 19, 2024
1 parent e6ac5b8 commit a2c3e0e
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 21 deletions.
70 changes: 68 additions & 2 deletions ...nalysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/SourceRootChecker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,29 @@ class SourceRootChecker {
)
)

val dokkaSampleConfiguration = DokkaConfigurationImpl(
sourceSets = listOf(
DokkaSourceSetImpl(
analysisPlatform = Platform.jvm,
displayName = "KotlinJvmSourceSet",
sourceSetID = KotlinJvmTestProject.DEFAULT_SOURCE_SET_ID,
samples = setOf(File("/src/d1/d2")),
),
DokkaSourceSetImpl(
analysisPlatform = Platform.jvm,
displayName = "KotlinJvmSourceSet2",
sourceSetID = KotlinJvmTestProject.DEFAULT_SOURCE_SET_ID,
samples = setOf(File("/src/d1"), File("/src/d1/d2/d3/../d4")),
),
DokkaSourceSetImpl(
analysisPlatform = Platform.jvm,
displayName = "KotlinJvmSourceSet3",
sourceSetID = KotlinJvmTestProject.DEFAULT_SOURCE_SET_ID,
samples = setOf(File("/src/c")),
),
)
)

@Test
@OnlySymbols("K1 supports all source roots including intersected ones")
fun `pre-generation check should failed if source roots are intersected`() {
Expand All @@ -58,7 +81,31 @@ class SourceRootChecker {
Pair(true, emptyList<String>())
) { acc, checker -> checker() + acc }

val expectedMessage = "Source sets 'KotlinJvmSourceSet' and 'KotlinJvmSourceSet2' have the common source roots: /src/d1/d2, /src/d1, /src/d1/d2/d4. Every Kotlin source file should belong to only one source set (module).".replace('/', File.separatorChar)
val expectedMessage =
"Source sets 'KotlinJvmSourceSet' and 'KotlinJvmSourceSet2' have the common source roots: /src/d1/d2, /src/d1, /src/d1/d2/d4. Every Kotlin source file should belong to only one source set (module). ".replace('/', File.separatorChar) +
"\nAlso, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701"
assertFalse(preGenerationCheckResult)
assertEquals(listOf(expectedMessage), checkMessages)
}

@Test
@OnlySymbols("K1 supports all source roots including intersected ones")
fun `pre-generation check should failed if samples are intersected`() {
val collectingLogger = CollectingDokkaConsoleLogger()

val context = DokkaContext.create(
configuration = dokkaSampleConfiguration,
logger = collectingLogger,
pluginOverrides = listOf()
)

val (preGenerationCheckResult, checkMessages) = context[CoreExtensions.preGenerationCheck].fold(
Pair(true, emptyList<String>())
) { acc, checker -> checker() + acc }

val expectedMessage =
"Source sets 'KotlinJvmSourceSet' and 'KotlinJvmSourceSet2' have the common sample roots: /src/d1/d2, /src/d1, /src/d1/d2/d4. Every Kotlin source file should belong to only one source set (module). ".replace('/', File.separatorChar) +
"\nAlso, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701"
assertFalse(preGenerationCheckResult)
assertEquals(listOf(expectedMessage), checkMessages)
}
Expand All @@ -78,7 +125,26 @@ class SourceRootChecker {
Pair(true, emptyList<String>())
) { acc, checker -> checker() + acc }
assertEquals(checkMessages, emptyList())
assertTrue(collectingLogger.collectedLogMessages.contains("Source sets 'KotlinJvmSourceSet' and 'KotlinJvmSourceSet2' have the common source roots: /src/d1/d2, /src/d1, /src/d1/d2/d4. Every Kotlin source file should belong to only one source set (module).".replace('/', File.separatorChar) + "\nIn Dokka K2 it will be an error. Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues"))
assertTrue(collectingLogger.collectedLogMessages.contains("Source sets 'KotlinJvmSourceSet' and 'KotlinJvmSourceSet2' have the common source roots: /src/d1/d2, /src/d1, /src/d1/d2/d4. Every Kotlin source file should belong to only one source set (module).".replace('/', File.separatorChar) + "\nIn Dokka K2 it will be an error. Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701"))
assertTrue(preGenerationCheckResult)
}

@Test
@OnlyDescriptors("K1 supports all source roots including intersected ones")
fun `pre-generation check should log warning if samples are intersected`() {
val collectingLogger = CollectingDokkaConsoleLogger()

val context = DokkaContext.create(
configuration = dokkaSampleConfiguration,
logger = collectingLogger,
pluginOverrides = listOf()
)

val (preGenerationCheckResult, checkMessages) = context[CoreExtensions.preGenerationCheck].fold(
Pair(true, emptyList<String>())
) { acc, checker -> checker() + acc }
assertEquals(checkMessages, emptyList())
assertTrue(collectingLogger.collectedLogMessages.contains("Source sets 'KotlinJvmSourceSet' and 'KotlinJvmSourceSet2' have the common sample roots: /src/d1/d2, /src/d1, /src/d1/d2/d4. Every Kotlin source file should belong to only one source set (module).".replace('/', File.separatorChar) + "\nIn Dokka K2 it will be an error. Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701"))
assertTrue(preGenerationCheckResult)
}
}
19 changes: 13 additions & 6 deletions ...rg/jetbrains/dokka/analysis/kotlin/descriptors/compiler/K1SourceRootIndependentChecker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import java.io.File
/**
* It just logs a warning if different source sets should have disjoint source roots and samples.
*
* K2's analysis API does not support having two different [org.jetbrains.kotlin.analysis.project.structure.KtSourceModule] with the same file system directory or intersecting files, it throws the error "Modules are inconsistent".
* K2's analysis API does not support having two different [org.jetbrains.kotlin.analysis.api.projectStructure.KaSourceModule] with the same file system directory or intersecting files, it throws the error "Modules are inconsistent".
* Meanwhile, K1 support it.
*/
internal class K1SourceRootIndependentChecker(
Expand All @@ -25,20 +25,27 @@ internal class K1SourceRootIndependentChecker(
for (i in sourceSets.indices) {
for (j in i + 1 until sourceSets.size) {
// check source roots
val sourceRoot1 = sourceSets[i].sourceRoots.normalize()
val sourceRoot2 = sourceSets[j].sourceRoots.normalize()
val intersection = intersect(sourceRoot1, sourceRoot2)
val intersection = intersect(sourceSets[i].sourceRoots, sourceSets[j].sourceRoots)
if (intersection.isNotEmpty()) {
logger.warn("Source sets '${sourceSets[i].displayName}' and '${sourceSets[j].displayName}' have the common source roots: ${intersection.joinToString()}. Every Kotlin source file should belong to only one source set (module).\n" +
"In Dokka K2 it will be an error. Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues")
"In Dokka K2 it will be an error. Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701")
}

//check sample roots
val sampleIntersection = intersect(sourceSets[i].samples, sourceSets[j].samples)
if (sampleIntersection.isNotEmpty()) {
logger.warn("Source sets '${sourceSets[i].displayName}' and '${sourceSets[j].displayName}' have the common sample roots: ${sampleIntersection.joinToString()}. Every Kotlin source file should belong to only one source set (module).\n" +
"In Dokka K2 it will be an error. Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701")
}
}
}
return PreGenerationCheckerOutput(true, emptyList())
}

private fun intersect(paths: Set<File>, paths2: Set<File>) : Set<File> = intersectOfNormalizedPaths(paths.normalize(), paths2.normalize())

private fun Set<File>.normalize() = mapTo(mutableSetOf()) { it.normalize() }
private fun intersect(normalizedPaths: Set<File>, normalizedPaths2: Set<File>): Set<File> {
private fun intersectOfNormalizedPaths(normalizedPaths: Set<File>, normalizedPaths2: Set<File>): Set<File> {
val result = mutableSetOf<File>()
for (p1 in normalizedPaths) {
for (p2 in normalizedPaths2) {
Expand Down
21 changes: 14 additions & 7 deletions ...kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/plugin/SourceRootIndependentChecker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import java.io.File
* It checks that different source sets should have disjoint source roots and samples.
*
*
* K2's analysis API does not support having two different [org.jetbrains.kotlin.analysis.project.structure.KtSourceModule] with the same file system directory or intersecting files, it throws the error "Modules are inconsistent".
* K2's analysis API does not support having two different [org.jetbrains.kotlin.analysis.api.projectStructure.KaSourceModule] with the same file system directory or intersecting files, it throws the error "Modules are inconsistent".
*
* @see org.jetbrains.kotlin.analysis.project.structure.KtModule.contentScope
* @see org.jetbrains.kotlin.analysis.api.projectStructure.KaModule.contentScope
* @see org.jetbrains.kotlin.analysis.project.structure.ProjectStructureProvider.getModule
*/
internal class SourceRootIndependentChecker(
Expand All @@ -28,20 +28,27 @@ internal class SourceRootIndependentChecker(
for (i in sourceSets.indices) {
for (j in i + 1 until sourceSets.size) {
// check source roots
val sourceRoot1 = sourceSets[i].sourceRoots.normalize()
val sourceRoot2 = sourceSets[j].sourceRoots.normalize()
val intersection = intersect(sourceRoot1, sourceRoot2)
val intersection = intersect(sourceSets[i].sourceRoots, sourceSets[j].sourceRoots)
if (intersection.isNotEmpty()) {
messages += "Source sets '${sourceSets[i].displayName}' and '${sourceSets[j].displayName}' have the common source roots: ${intersection.joinToString()}. Every Kotlin source file should belong to only one source set (module)."
messages += "Source sets '${sourceSets[i].displayName}' and '${sourceSets[j].displayName}' have the common source roots: ${intersection.joinToString()}. Every Kotlin source file should belong to only one source set (module). \n" +
"Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701"
}

//check sample roots
val sampleIntersection = intersect(sourceSets[i].samples, sourceSets[j].samples)
if (sampleIntersection.isNotEmpty()) {
messages += "Source sets '${sourceSets[i].displayName}' and '${sourceSets[j].displayName}' have the common sample roots: ${sampleIntersection.joinToString()}. Every Kotlin source file should belong to only one source set (module). \n" +
"Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701"
}
}
}
return PreGenerationCheckerOutput(messages.isEmpty(), messages)
}

private fun intersect(paths: Set<File>, paths2: Set<File>) : Set<File> = intersectOfNormalizedPaths(paths.normalize(), paths2.normalize())

private fun Set<File>.normalize() = mapTo(mutableSetOf()) { it.normalize() }
private fun intersect(normalizedPaths: Set<File>, normalizedPaths2: Set<File>): Set<File> {
private fun intersectOfNormalizedPaths(normalizedPaths: Set<File>, normalizedPaths2: Set<File>): Set<File> {
val result = mutableSetOf<File>()
for (p1 in normalizedPaths) {
for (p2 in normalizedPaths2) {
Expand Down
6 changes: 0 additions & 6 deletions dokka-subprojects/plugin-base/src/test/kotlin/content/samples/ContentForSamplesTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,13 @@ class ContentForSamplesTest : BaseAbstractTest() {
analysisPlatform = "jvm"
dependentSourceSets = setOf(common.value.sourceSetID)
sourceRoots = listOf("src/jvmMain/kotlin/pageMerger/Test.kt")
samples = listOf(
Paths.get("$testDataDir/samples.kt").toString(),
)
}
sourceSet {
name = "linuxX64"
displayName = "linuxX64"
analysisPlatform = "native"
dependentSourceSets = setOf(common.value.sourceSetID)
sourceRoots = listOf("src/linuxX64Main/kotlin/pageMerger/Test.kt")
samples = listOf(
Paths.get("$testDataDir/samples.kt").toString(),
)
}
}
}
Expand Down

0 comments on commit a2c3e0e

Please sign in to comment.