From 5f791a7fd8cb82bde0ce021b275bd8baeccf4bc8 Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Wed, 3 Dec 2025 13:50:30 +0900 Subject: [PATCH 1/2] Fix isJvmAccessible to handle protected Java classes #24507 Fixes #24507 The previous implementation of `isJvmAccessible` failed to recognize protected Java nested classes as accessible. And it causes compilation errors when extending them from subclasses. Previous implementation: ```scala def isJvmAccessible(cls: Symbol): Boolean = !cls.is(Flags.JavaDefined) || { val boundary = cls.accessBoundary(cls.owner)(using preErasureCtx) (boundary eq defn.RootClass) || (ctx.owner.enclosingPackageClass eq boundary) } ``` For protected nested classes like B below, the access boundary is the enclosing class (`A` in this case), not a `package a` or root. However, `B` should be accessible through a public class `A`. ```java package a; public class A { protected class B {} } ``` This commit replace the manual boundary checks with `cls.isAccessibleFrom`, which properly handles all Java access modifiers. This aligns with the Scala 2 implementation which uses the equivalent `context.isAccessible(cls, cls.owner.thisType)`. https://github.com/scala/scala/blob/efb71845113639bd1da231c917e18af33519fc07/src/compiler/scala/tools/nsc/transform/Erasure.scala#L1451-L1455 For example, - `cls` will be `protected class B` - `cls.owner.thisType` will be `public class A` - And, `cls.isAccessibleFrom(cls.owner.thisType)` is true. On the other hand, `isJvmAccessible` returns false for Java-defined package protected class case like `java-package-protected` case. https://github.com/tanishiking/scala3/tree/b2850be4c79996d8ebd9afaf36ed12f46febb0d1/tests/run/java-package-protected In this case, - `cls` = `class B` (package protected under `a`) - `cls.owner.thisType` = `package a` - And, `cls.isAccessibleFrom(cls.owner.thisType)` is false. because `B` is package protected and, we cannot access B through `a.B` from another package. In this case, cast to `A` happens. --- Note that, this regression starts from e7d479f but the true commit is https://github.com/scala/scala3/pull/9364/commits/d3ce551b3a74c03cb28a1c77cd6ed1b0517bcb6a but the reason why we stopped using `isAccessibleFrom` seems to be an optimization and the `tests/pos/trait-access` still compiles with this change. --- compiler/src/dotty/tools/dotc/transform/Erasure.scala | 9 ++------- tests/pos/i24507-jframe.scala | 2 ++ tests/pos/i24507/A.java | 5 +++++ tests/pos/i24507/Test.scala | 3 +++ 4 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 tests/pos/i24507-jframe.scala create mode 100644 tests/pos/i24507/A.java create mode 100644 tests/pos/i24507/Test.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index a2edb50b3982..8403a55b6b36 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -748,13 +748,8 @@ object Erasure { // Scala classes are always emitted as public, unless the // `private` modifier is used, but a non-private class can never // extend a private class, so such a class will never be a cast target. - !cls.is(Flags.JavaDefined) || { - // We can't rely on `isContainedWith` here because packages are - // not nested from the JVM point of view. - val boundary = cls.accessBoundary(cls.owner)(using preErasureCtx) - (boundary eq defn.RootClass) || - (ctx.owner.enclosingPackageClass eq boundary) - } + !cls.is(Flags.JavaDefined) || + cls.isAccessibleFrom(cls.owner.thisType)(using preErasureCtx) @tailrec def recur(qual: Tree): Tree = diff --git a/tests/pos/i24507-jframe.scala b/tests/pos/i24507-jframe.scala new file mode 100644 index 000000000000..ec67ff9e9b49 --- /dev/null +++ b/tests/pos/i24507-jframe.scala @@ -0,0 +1,2 @@ +class Test extends javax.swing.JFrame { class Acc extends AccessibleJFrame } + diff --git a/tests/pos/i24507/A.java b/tests/pos/i24507/A.java new file mode 100644 index 000000000000..ae747cf16b3d --- /dev/null +++ b/tests/pos/i24507/A.java @@ -0,0 +1,5 @@ +package a; + +public class A { + protected class B {} +} diff --git a/tests/pos/i24507/Test.scala b/tests/pos/i24507/Test.scala new file mode 100644 index 000000000000..be3301105515 --- /dev/null +++ b/tests/pos/i24507/Test.scala @@ -0,0 +1,3 @@ +import a._ + +class Test extends A { class Acc extends B } From 2a35dedd006b794fb248bef6945471988c8c237d Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Wed, 3 Dec 2025 22:09:55 +0900 Subject: [PATCH 2/2] Add run test for #24507 --- tests/pos/i24507/A.java | 5 ----- tests/pos/i24507/Test.scala | 3 --- tests/run/i24507.check | 1 + tests/run/i24507/A.java | 9 +++++++++ tests/run/i24507/test.scala | 12 ++++++++++++ 5 files changed, 22 insertions(+), 8 deletions(-) delete mode 100644 tests/pos/i24507/A.java delete mode 100644 tests/pos/i24507/Test.scala create mode 100644 tests/run/i24507.check create mode 100644 tests/run/i24507/A.java create mode 100644 tests/run/i24507/test.scala diff --git a/tests/pos/i24507/A.java b/tests/pos/i24507/A.java deleted file mode 100644 index ae747cf16b3d..000000000000 --- a/tests/pos/i24507/A.java +++ /dev/null @@ -1,5 +0,0 @@ -package a; - -public class A { - protected class B {} -} diff --git a/tests/pos/i24507/Test.scala b/tests/pos/i24507/Test.scala deleted file mode 100644 index be3301105515..000000000000 --- a/tests/pos/i24507/Test.scala +++ /dev/null @@ -1,3 +0,0 @@ -import a._ - -class Test extends A { class Acc extends B } diff --git a/tests/run/i24507.check b/tests/run/i24507.check new file mode 100644 index 000000000000..257cc5642cb1 --- /dev/null +++ b/tests/run/i24507.check @@ -0,0 +1 @@ +foo diff --git a/tests/run/i24507/A.java b/tests/run/i24507/A.java new file mode 100644 index 000000000000..489beb4dd6cb --- /dev/null +++ b/tests/run/i24507/A.java @@ -0,0 +1,9 @@ +package a; + +public class A { + protected class B { + public String foo() { + return "foo"; + } + } +} diff --git a/tests/run/i24507/test.scala b/tests/run/i24507/test.scala new file mode 100644 index 000000000000..38f1e6465f98 --- /dev/null +++ b/tests/run/i24507/test.scala @@ -0,0 +1,12 @@ +// scalajs: --skip +import a._ + +class T extends A { + class Acc extends B + val acc = new Acc + def foo(): String = acc.foo() +} + +@main def Test = + val t = new T + println(t.foo())