From 94e5cafb617fb6ea69225b92ebc3047275be72a0 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 13 Mar 2024 23:17:44 +0100 Subject: [PATCH] JVM: fix "module reads" check in JavaModuleGraph There was a bug in the DFS. Whenever we encountered a "requires transitive X" directive in some module, we recursively invoked DFS not on X but on `dependencyName`, which is the target module that we're trying to find the path to. And that always failed, because no module requires itself. #KT-66275 Fixed --- .../kotlin/cli/jvm/modules/JavaModuleGraph.kt | 2 +- .../moduleA/a/A.java | 5 ++++ .../moduleA/module-info.java | 3 +++ .../moduleB/b/B.java | 5 ++++ .../moduleB/module-info.java | 4 +++ .../moduleC/module-info.java | 3 +++ .../moduleD.txt | 1 + .../moduleD/d/JavaMain.java | 9 +++++++ .../moduleD/d/KotlinMain.kt | 7 +++++ .../moduleD/module-info.java | 4 +++ .../AbstractJavaModulesIntegrationTest.kt | 26 +++++++++++++++++++ 11 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/a/A.java create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/module-info.java create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/b/B.java create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/module-info.java create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleC/module-info.java create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD.txt create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/JavaMain.java create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/KotlinMain.kt create mode 100644 compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/module-info.java diff --git a/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/modules/JavaModuleGraph.kt b/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/modules/JavaModuleGraph.kt index 13f96347516..d57baf54909 100644 --- a/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/modules/JavaModuleGraph.kt +++ b/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/modules/JavaModuleGraph.kt @@ -77,7 +77,7 @@ class JavaModuleGraph(finder: JavaModuleFinder) { is JavaModule.Explicit -> { for ((dependencyModuleName, isTransitive) in module.moduleInfo.requires) { if (dependencyModuleName == dependencyName) return true - if (isTransitive && dfs(dependencyName)) return true + if (isTransitive && dfs(dependencyModuleName)) return true } return false } diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/a/A.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/a/A.java new file mode 100644 index 00000000000..72df5d92e93 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/a/A.java @@ -0,0 +1,5 @@ +package a; + +public class A { + public String ok() { return "OK"; } +} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/module-info.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/module-info.java new file mode 100644 index 00000000000..8e8f5b048e3 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleA/module-info.java @@ -0,0 +1,3 @@ +module moduleA { + exports a; +} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/b/B.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/b/B.java new file mode 100644 index 00000000000..8831b25a0ec --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/b/B.java @@ -0,0 +1,5 @@ +package b; + +import a.A; + +public class B extends A {} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/module-info.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/module-info.java new file mode 100644 index 00000000000..fb7f1c9aac8 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleB/module-info.java @@ -0,0 +1,4 @@ +module moduleB { + requires transitive moduleA; + exports b; +} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleC/module-info.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleC/module-info.java new file mode 100644 index 00000000000..9ebb8a63c79 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleC/module-info.java @@ -0,0 +1,3 @@ +module moduleC { + requires transitive moduleB; +} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD.txt b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD.txt new file mode 100644 index 00000000000..d86bac9de59 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD.txt @@ -0,0 +1 @@ +OK diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/JavaMain.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/JavaMain.java new file mode 100644 index 00000000000..4f3e1d04d88 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/JavaMain.java @@ -0,0 +1,9 @@ +package d; + +import b.B; + +public class JavaMain { + public static void main(String[] args) { + System.out.println(new B().ok()); + } +} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/KotlinMain.kt b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/KotlinMain.kt new file mode 100644 index 00000000000..d4fbd10066c --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/d/KotlinMain.kt @@ -0,0 +1,7 @@ +package d + +import b.B + +fun main() { + println(B().ok()) +} diff --git a/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/module-info.java b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/module-info.java new file mode 100644 index 00000000000..35d31243b68 --- /dev/null +++ b/compiler/testData/javaModules/inheritedDeclarationFromTwiceTransitiveDependency/moduleD/module-info.java @@ -0,0 +1,4 @@ +module moduleD { + requires kotlin.stdlib; + requires moduleC; +} diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/modules/AbstractJavaModulesIntegrationTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/modules/AbstractJavaModulesIntegrationTest.kt index bc95f93155b..ab3e1c8700a 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/modules/AbstractJavaModulesIntegrationTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/modules/AbstractJavaModulesIntegrationTest.kt @@ -210,6 +210,32 @@ abstract class AbstractJavaModulesIntegrationTest( module("moduleD", listOf(c, b, a)) } + fun testInheritedDeclarationFromTwiceTransitiveDependency() { + // module A <-t- module B <-t- module C <--- module D + + // Java: class A { String ok() { /* ... */ } } + val a = module("moduleA") + + // Java: class B extends A + val b = module("moduleB", listOf(a)) + + val c = module("moduleC", listOf(a, b)) + + // Java: new B().ok() + // Kotlin: B().ok() + val d = module("moduleD", listOf(a, b, c)) + + // validate the run-time behavior of Java-compiled code for the sake of comparison + val (javaStdout, javaStderr) = runModule("moduleD/d.JavaMain", listOf(d, c, b, a)) + assertEquals("", javaStderr) + assertEquals("OK", javaStdout) + + // test the run-time behavior of Kotlin-compiled code + val (kotlinStdout, kotlinStderr) = runModule("moduleD/d.KotlinMainKt", listOf(d, c, b, a)) + assertEquals("", kotlinStderr) + assertEquals("OK", kotlinStdout) + } + fun testSpecifyPathToModuleInfoInArguments() { val a = module("moduleA")