Fail the Java protected member case, rather than be wierd

Author: dwijnand

compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala

@@ -33,12 +33,18 @@ object ProtectedAccessors {
       ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
     }
 
-  /** Do we need a protected accessor for accessing sym from the current context's owner? */
-  def needsAccessor(sym: Symbol)(using Context): Boolean =
+  /** Do we need a protected accessor if the current context's owner
+   *  is not in a subclass or subtrait of `sym`?
+   */
+  def needsAccessorIfNotInSubclass(sym: Symbol)(using Context): Boolean =
     sym.isTerm && sym.is(Protected) &&
     !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
-    !insideBoundaryOf(sym) &&
-    (sym.is(JavaDefined) || !ctx.owner.enclosingClass.derivesFrom(sym.owner))
+    !insideBoundaryOf(sym)
+
+  /** Do we need a protected accessor for accessing sym from the current context's owner? */
+  def needsAccessor(sym: Symbol)(using Context): Boolean =
+    needsAccessorIfNotInSubclass(sym) &&
+    !ctx.owner.enclosingClass.derivesFrom(sym.owner)
 }
 
 class ProtectedAccessors extends MiniPhase {

compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala

@@ -175,7 +175,7 @@ class SuperAccessors(thisPhase: DenotTransformer) {
     val sym = sel.symbol
 
     def needsSuperAccessor =
-      ProtectedAccessors.needsAccessor(sym) &&
+      ProtectedAccessors.needsAccessorIfNotInSubclass(sym) &&
       AccessProxies.hostForAccessorOf(sym).is(Trait)
     qual match {
       case _: This if needsSuperAccessor =>
@@ -185,10 +185,19 @@ class SuperAccessors(thisPhase: DenotTransformer) {
          * If T extends C, then we can access it by casting
          * the qualifier of the select to C.
          *
+         * That's because the protected method is actually public,
+         * so we can call it.  For truly protected methods, like from
+         * Java, we error instead of emitting the wrong code (i17021.ext-java).
+         *
          * Otherwise, we need to go through an accessor,
          * which the implementing class will provide an implementation for.
          */
-        superAccessorCall(sel)
+        if ctx.owner.enclosingClass.derivesFrom(sym.owner) then
+          if sym.is(JavaDefined) then
+            report.error(em"${ctx.owner} accesses protected $sym inside a concrete trait method: use super.${sel.name} instead", sel.srcPos)
+          sel
+        else
+          superAccessorCall(sel)
       case Super(_, mix) =>
         transformSuperSelect(sel)
       case _ =>

tests/neg/i17021.ext-java/A.java

@@ -0,0 +1,6 @@
+// Derives from run/i17021.defs, but with a Java protected member
+package p1;
+
+public class A {
+  protected int foo() { return 1; }
+}

tests/neg/i17021.ext-java/Test.scala

@@ -0,0 +1,14 @@
+// Derives from run/i17021.defs
+// but with a Java protected member
+// which leads to a compile error
+package p2:
+  trait B extends p1.A:
+    def bar: Int = foo // error: method bar accesses protected method foo inside a concrete trait method: use super.foo instead
+
+  class C extends B:
+    override def foo: Int = 2
+
+object Test:
+  def main(args: Array[String]): Unit =
+    val n = new p2.C().bar
+    assert(n == 2, n)

tests/run/i17021.ext-java/Test.scala

@@ -1,14 +1,14 @@
 // Derives from run/i17021.defs
 // but with a Java protected member
-// which changes the behaviour
+// and fixed calling code, that uses super
 package p2:
   trait B extends p1.A:
-    def bar: Int = foo
+    def bar: Int = super.foo
 
   class C extends B:
     override def foo: Int = 2
 
 object Test:
   def main(args: Array[String]): Unit =
     val n = new p2.C().bar
-    assert(n == 1, n) // B can only call super.foo
+    assert(n == 1, n)