Skip to content

Commit bbb4678

Browse files
ADFA-6381: Add each module dependency once
Now that addLibrary returns one shared KtLibraryModule per jar, a jar on both the boot classpath and a module's compile classpath was added to that module's dependencies twice as the same instance. Both module builders now keep dependencies in a LinkedHashSet: O(1) dedupe, order preserved. Tests: compile-classpath jars shared across modules resolve to one instance; a jar on both classpaths is one dependency (fails without the builder change: size 2).
1 parent c63f304 commit bbb4678

3 files changed

Lines changed: 73 additions & 20 deletions

File tree

‎lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtLibraryModule.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ internal class KtLibraryModule(
4747
) {
4848
lateinit var id: String
4949
private val contentRoots = mutableSetOf<Path>()
50-
private val dependencies = mutableListOf<KtModule>()
50+
51+
// A set, like KtSourceModule.Builder: no module is a dependency twice.
52+
private val dependencies = linkedSetOf<KtModule>()
5153
var isSdk: Boolean = false
5254
var jvmTarget: JvmTarget = DEFAULT_JVM_TARGET
5355
var librarySources: KaLibrarySourceModule? = null

‎lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ internal class KtSourceModule(
3131
private val project: Project,
3232
) {
3333
lateinit var module: ModuleProject
34-
private val dependencies = mutableListOf<KtModule>()
34+
35+
// A set: a jar on both the boot and compile classpath is one shared module (ADFA-6381).
36+
private val dependencies = linkedSetOf<KtModule>()
3537

3638
fun addDependency(dep: KtModule) {
3739
dependencies.add(dep)

‎lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/CollectKtModulesTest.kt‎

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,85 @@ package com.itsaky.androidide.lsp.kotlin.compiler
22

33
import com.google.common.truth.Truth.assertThat
44
import com.itsaky.androidide.lsp.kotlin.compiler.modules.KtLibraryModule
5+
import com.itsaky.androidide.lsp.kotlin.compiler.modules.KtSourceModule
56
import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest
67
import com.itsaky.androidide.project.AndroidModels
78
import com.itsaky.androidide.project.GradleModels
9+
import com.itsaky.androidide.project.JavaModels
810
import com.itsaky.androidide.projects.api.AndroidModule
911
import com.itsaky.androidide.projects.api.GradleProject
12+
import com.itsaky.androidide.projects.api.JavaModule
13+
import com.itsaky.androidide.projects.api.ModuleProject
1014
import com.itsaky.androidide.projects.api.Workspace
1115
import org.junit.Test
16+
import java.nio.file.Path
1217
import kotlin.io.path.createFile
1318
import kotlin.io.path.pathString
1419

1520
class CollectKtModulesTest : KtLspTest() {
1621
@Test
1722
fun `modules sharing android jar share one library module`() {
18-
val androidJar =
19-
lspTestRule.tempDir.root
20-
.toPath()
21-
.resolve("android.jar")
22-
.createFile()
23+
val androidJar = tempJar("android.jar")
2324
val modulePaths = listOf(":a", ":b", ":c")
24-
val workspace =
25-
Workspace(
26-
rootProject = GradleProject(gradleProject(":")),
27-
subProjects = modulePaths.map { AndroidModule(gradleProject(it, androidJar.pathString)) },
28-
syncIssues = emptyList(),
29-
)
3025

31-
val sourceModules = workspace.collectKtModules(env.project, env.applicationEnv)
26+
val sourceModules = collect(modulePaths.map { AndroidModule(gradleProject(it, bootClassPath = androidJar)) })
3227

33-
val androidJarDeps =
34-
sourceModules.map { module ->
35-
module.directRegularDependencies.filterIsInstance<KtLibraryModule>().filter { it.id == androidJar.pathString }
36-
}
28+
val androidJarDeps = sourceModules.map { it.libraryDeps(androidJar) }
3729
assertThat(sourceModules).hasSize(modulePaths.size)
3830
// Before ADFA-6381 each source module depended on one fresh copy per Android module (3 x 3 here).
3931
androidJarDeps.forEach { assertThat(it).hasSize(1) }
4032
androidJarDeps.flatten().forEach { assertThat(it).isSameInstanceAs(androidJarDeps.first().first()) }
4133
}
4234

35+
@Test
36+
fun `modules sharing a compile classpath jar share one library module`() {
37+
val libJar = tempJar("lib.jar")
38+
39+
val sourceModules = collect(listOf(":a", ":b").map { JavaModule(gradleProject(it, compileJar = libJar)) })
40+
41+
val libJarDeps = sourceModules.map { it.libraryDeps(libJar) }
42+
libJarDeps.forEach { assertThat(it).hasSize(1) }
43+
assertThat(libJarDeps[1].single()).isSameInstanceAs(libJarDeps[0].single())
44+
}
45+
46+
@Test
47+
fun `a jar on both the boot and compile classpath is one dependency`() {
48+
val androidJar = tempJar("android.jar")
49+
50+
// Boot classpaths from every Android module go to every source module, so :j sees
51+
// android.jar twice: once from :a's boot classpath, once from its own compile classpath.
52+
val sourceModules =
53+
collect(
54+
listOf(
55+
AndroidModule(gradleProject(":a", bootClassPath = androidJar)),
56+
JavaModule(gradleProject(":j", compileJar = androidJar)),
57+
),
58+
)
59+
60+
assertThat(sourceModules.single { it.id == ":j" }.libraryDeps(androidJar)).hasSize(1)
61+
}
62+
63+
private fun tempJar(name: String): Path =
64+
lspTestRule.tempDir.root
65+
.toPath()
66+
.resolve(name)
67+
.createFile()
68+
69+
private fun collect(subProjects: List<ModuleProject>): List<KtSourceModule> =
70+
Workspace(
71+
rootProject = GradleProject(gradleProject(":")),
72+
subProjects = subProjects,
73+
syncIssues = emptyList(),
74+
).collectKtModules(env.project, env.applicationEnv)
75+
.filterIsInstance<KtSourceModule>()
76+
77+
private fun KtSourceModule.libraryDeps(jar: Path) =
78+
directRegularDependencies.filterIsInstance<KtLibraryModule>().filter { it.id == jar.pathString }
79+
4380
private fun gradleProject(
4481
path: String,
45-
bootClassPath: String? = null,
82+
bootClassPath: Path? = null,
83+
compileJar: Path? = null,
4684
): GradleModels.GradleProject {
4785
val dir = "/tmp/collect-kt-modules${path.replace(':', '/')}"
4886
return GradleModels.GradleProject
@@ -58,7 +96,18 @@ class CollectKtModulesTest : KtLspTest() {
5896
AndroidModels.AndroidProject
5997
.newBuilder()
6098
.setProjectType(AndroidModels.ProjectType.LibraryProject)
61-
.addBootClassPaths(bootClassPath),
99+
.addBootClassPaths(bootClassPath.pathString),
100+
)
101+
}
102+
if (compileJar != null) {
103+
setJavaProject(
104+
JavaModels.JavaProject.newBuilder().addDependency(
105+
JavaModels.JavaDependency
106+
.newBuilder()
107+
.setJarFilePath(compileJar.pathString)
108+
.setScope(JavaModule.SCOPE_COMPILE)
109+
.setExternalLibrary(JavaModels.JavaExternalLibraryDependency.getDefaultInstance()),
110+
),
62111
)
63112
}
64113
}.build()

0 commit comments

Comments
 (0)