Qualifiers when overriding Java methods

[kynthus]

reproduction steps

using Scala 2.13.5, on Oracle Java SE Development Kit 8u202.

I tried overriding a method in a Java class.

// javapkg/JavaSuper.java

package javapkg;

public class JavaSuper {
    public void fn() { System.out.println("JavaSuper#fn()"); }
}

// pkg/ScalaSub.scala

package pkg

import javapkg.JavaSuper

object Cls {

  class ScalaSub extends JavaSuper {
    override def fn(): Unit = println("ScalaSub#fn()")
  }

}

I changed each other's access modifiers as shown in the table below and verified.
It summarizes whether it can be compiled and whether it is the expected result.

Rules for overriding from Java.

Modifier(JavaSuper)	Modifier(ScalaSub)	Compile	Expected
public	none(public)	○	yes
public	protected	○	no
public	private	×	yes
public	protected[Cls]	○	no
public	private[Cls]	○	no
public	protected[pkg]	○	no
public	private[pkg]	○	no
public	protected[this]	○	no
public	private[this]	○	no
Java's protected	none(public)	○	yes
Java's protected	protected	○	no
Java's protected	private	×	yes
Java's protected	protected[Cls]	○	no
Java's protected	private[Cls]	×	yes
Java's protected	protected[pkg]	○	no
Java's protected	private[pkg]	×	yes
Java's protected	protected[this]	○	no
Java's protected	private[this]	○	no
package access	none(public)	○	yes
package access	protected	○	no
package access	private	×	yes
package access	protected[Cls]	○	no
package access	private[Cls]	○	no
package access	protected[pkg]	○	yes
package access	private[pkg]	○	yes
package access	protected[this]	○	no
package access	private[this]	○	no
private	none(public)	×	yes
private	protected	×	yes
private	private	×	yes
private	protected[Cls]	×	yes
private	private[Cls]	×	yes
private	protected[pkg]	×	yes
private	private[pkg]	×	yes
private	protected[this]	×	yes
private	private[this]	×	yes

problem

1. Subclasses have stricter access privileges.

   Cases

   Modifier(JavaSuper)	Modifier(ScalaSub)
   public	protected
   public	protected[Cls]
   public	private[Cls]
   public	protected[pkg]
   public	private[pkg]

2. It becomes inaccessible from directly under the package to which the class JavaSuper.

   Cases

   Modifier(JavaSuper)	Modifier(ScalaSub)
   Java's protected	protected
   Java's protected	protected[Cls]
   package access	protected
   package access	protected[Cls]
   package access	private[Cls]

3. If pkg or its subpackages are unrelated to the javapkg, they will be inaccessible from javapkg.

   Cases

   Modifier(JavaSuper)	Modifier(ScalaSub)
   Java's protected	protected[pkg]

4. Access modifiers on the subclass side do not work.

   Cases

   Modifier(JavaSuper)	Modifier(ScalaSub)
   public	protected[this]
   Java's protected	protected[this]
   package access	protected[this]

   ※Treated the same as the modifier on the JavaSuper side. (Very confusing!)

5. Calling via ScalaSub throw java.lang.IllegalAccessError, calling via JavaSuper calls fn() on the JavaSuper side.

   Cases

   As reported in private[this] subverts override accessibility check #11913.

   Modifier(JavaSuper)	Modifier(ScalaSub)
   public	private[this]
   Java's protected	private[this]
   package access	private[this]

5. is not mentioned here because the issue already exists.
2. and 3. may be unavoidable due to Java package access specifications, so I'm not going to go into it.

I think problem is 1. and 4..
I didn't expect that public methods could be overridden with stricter modifiers or protected[this] wouldn't work.
Please let me know if these are specifications.

// javapkg/JavaSuper.java

package javapkg;

public class JavaSuper {
    public void fn1() { System.out.println("JavaSuper#fn1()"); }
    public void fn2() { System.out.println("JavaSuper#fn2()"); }
}

// pkg/ScalaSub.scala

package pkg

import javapkg.JavaSuper

object Cls {

  class ScalaSub extends JavaSuper {
    protected       override def fn1(): Unit = println("ScalaSub#fn1()") // 1. OK but strange, access only from subclasses via ScalaSub.
    protected[this] override def fn2(): Unit = println("ScalaSub#fn2()") // 4. This is even more strange, can be accessed anywhere via ScalaSub.
  }

}

import javapkg.JavaSuper
import pkg.Cls.ScalaSub

val j: JavaSuper = new ScalaSub()
val s: ScalaSub  = new ScalaSub()

scala> j.fn1()
ScalaSub#fn1()

scala> j.fn2()
ScalaSub#fn2()

scala> s.fn1() // It's unfamiliar to be inaccessible only from subclasses.
         ^
       error: method fn1 in class ScalaSub cannot be accessed as a member of pkg.Cls.ScalaSub from class $iw
        Access to protected method fn1 not permitted because
        enclosing class $iw is not a subclass of
        class ScalaSub in object Cls where target is defined

scala> s.fn2() // `protected[this]` seems to have been ignored.
ScalaSub#fn2()

Supplement

1. is only occurs if you override a method defined in a Java classes in Scala 2.8.2 or later.

4. is even before Scala 2.8.1, it seems that protected can be overridden with protected[this], allowing access via another instance.
I think the problem became more noticeable as we were able to override public etc, in 2.8.2.

By the way, in Scala 3.0.0-RC1, an error occurred when the JavaSuper side was public and package access, and the result was as expected.
(Java's protected is still suspicious even in Scala 3... But that should be mentioned in Dotty.)

[kynthus]

The following modifications seem to be optimal for 1..
It seems that all modifiers are currently allowed (including public) for settings that allow overrides only when Java protected.

// compiler/scala/tools/nsc/typechecker/RefChecks.scala
/* ... */
          def isOverrideAccessOK = member.isPublic || {      // member is public, definitely same or relaxed access
            (!other.isProtected || member.isProtected) &&    // if o is protected, so is m
            ((!isRootOrNone(ob) && ob.hasTransOwner(mb)) ||  // m relaxes o's access boundary
             other.isJavaDefined)                           // overriding a protected java member, see #3946
          }
/* ... */

Fix it like this(Add && other.isProtected).

// compiler/scala/tools/nsc/typechecker/RefChecks.scala
/* ... */
          def isOverrideAccessOK = member.isPublic || {      // member is public, definitely same or relaxed access
            (!other.isProtected || member.isProtected) &&    // if o is protected, so is m
            ((!isRootOrNone(ob) && ob.hasTransOwner(mb)) ||  // m relaxes o's access boundary
            (other.isJavaDefined && other.isProtected))      // overriding a protected java member, see #3946 #12349
          }
/* ... */

Recreated Pull-Request for the above fix.
Expect the following improvements.

Cases that can be improved.

No	Modifier(JavaSuper)	Modifier(ScalaSub)	Compile	Expected
1.	public	protected	○→×	no→yes
1.	public	protected[Cls]	○→×	no→yes
1.	public	private[Cls]	○→×	no→yes
1.	public	protected[pkg]	○→×	no→yes
1.	public	private[pkg]	○→×	no→yes
4.	public	protected[this]	○→×	no→yes
2.	package access	protected	○→×	no→yes
2.	package access	protected[Cls]	○→×	no→yes
2.	package access	private[Cls]	○→×	no→yes
4.	package access	protected[this]	○→×	no→yes

In the cases shown below, it seems that measures need to be taken continuously.
But, if it is Dotty compliant, I think it's OK.

Cases that cannot be improved.

No	Modifier(JavaSuper)	Modifier(ScalaSub)	Compile	Expected	Remarks
5.	public	private[this]	○	no	Needs separate action
2.	Java's protected	protected	○	no	Dotty compliant
2.	Java's protected	protected[Cls]	○	no	Dotty compliant
3.	Java's protected	protected[pkg]	○	no	Dotty compliant
4.	Java's protected	protected[this]	○	no	Needs separate action
5.	Java's protected	private[this]	○	no	Needs separate action
5.	package access	private[this]	○	no	Needs separate action

[lrytz]

Cases that cannot be improved.

I think 5. is unrelated to Java, it's #9334. Dotty is more strict here, it doesn't allow a private[this] def f if there is also an inherited f. I think we should do the same. Maybe 4. is related?

scala> class C { def f = 0 }; class D extends C { private[this] def f = 1 }
1 |class C { def f = 0 }; class D extends C { private[this] def f = 1 }
  |                                                             ^
  |                      private method f cannot override method f in class C

For 2., I agree, I think that's a pragmatic choice (for the record: Java's protected allows accessing the member from other classes in the same package).

3. I don't understand - can you make an example? If I have

package p; // Java
public class C { protected int f() { return 1; } }

package p
class D extends C { protected[p] override def f = 2 }

Why should that not compile? Is Scala's protected[p] more restrictive?

(Aparently I was already into this kind of stuff >10 years ago... #3946)

[kynthus]

I think 5. is unrelated to Java,

I agree, it's just a by-product of allowing overrides for private[this] in Scala.
(But, it doesn't seem to be an override internally...)

I think we should do the same. Maybe 4. is related?

At 4., protected can be overridden with protected[this] in Scala as well.
On the other hand, none(public) and private[pkg] can not be overridden by protected[this](as expected).

In Scala 3, protected[this] is deprecated and makes little sense, so not unnatural to be able to overrided protected.
I'm curious because protected [this] is still active in Scala 2.

3. I don't understand - can you make an example?

I'm sorry it's complicated, but it's correct that it partially compiles.
There is no problem if the subclass in same package or parent packages,
the problem becomes apparent in the case of completely unrelated packages.

Below is the sample code.

// p/C.java
package p;
public class C { protected int f() { return 1; } }

// q/D.scala
package q
import p.C
class D extends C { protected[q] override def f = 2 } // package q is unrelated for package p.

// p/E.scala
package p

import q.D

object E {

  def main(args: Array[String]): Unit = {
    val c: C = new D()
    val d: D = new D()

    c.f() // OK, protected in Java allows access from the same package.
    d.f() // NG, since p is a package unrelated to q, it cannot be accessed from variables of subclass type.
  }

}

# javac p/C.java
# scalac q/D.scala
# scalac p/E.scala
p/E.scala:13: error: method f in class D cannot be accessed as a member of q.D from object E in package p
 Access to protected method f not permitted because
 enclosing object E in package p is not a subclass of
 class D in package q where target is defined
    d.f() // NG, since p is a package unrelated to q, it cannot be accessed from variables of subclass type.
      ^
1 error

It seems that he closed the lid without knowing it.
Certainly, as you mentioned in #3946, I think the protected of Java itself is destructive.
For example, java.lang.Object#clone() is protected, but if the user overrides it with protected,
it Will not be accessible in java.lang via subclass type variables.
(Such cases are extremely rare...)

So, at least I think it's barren to dig into 3..

Finally, note that Scala is a bit clever when overriding protected[p] with protected[q] in Scala,
as it will result in an error unless q is the p or parent packages of p.

[som-snytt]

Instead of saying, "Java protected includes package visibility," one could say that it extends default access (package access) by including subclasses in a different package. The JLS says the intention is to allow access by "code which is responsible for the implementation of that object."

Because inheritance is brittle, it makes sense that only subclasses and members of the current package can access protected m.

Instead of assimilating the Java model, Scala could do what is done with parens in method signatures of overrides: if an ancestor override is Java-defined, then the Java rule obtains: namely, protected has exactly Java semantics (and can't be widened).

[lrytz]

allowing overrides for private[this] in Scala.
(But, it doesn't seem to be an override internally...)

Yes, the type checker doesn't treat it as an override, but the backend emits the same signature, which turns it into an override. We could rename the method (as it's private). But I think it's better to disallow private[this] methods that would override, as done in Scala 3.

In Scala 3, protected[this] is deprecated and makes little sense

Good ponit. It's under -source future, but I agree it's an obscure enough feature so that it's not a priority.

// p/C.java
package p;
public class C { protected int f() { return 1; } }

// q/D.scala
package q
import p.C
class D extends C { protected[q] override def f = 2 } // package q is unrelated for package p.

The override should be rejected, C.f can be accessed in package p, but D.f can't.

But protected[p] override def f in Scala should remain allowed.

Am I missing something?

protected of Java itself is destructive

Ah, great observation, I didn't realize..! So maybe it makes sense to allow protected[q] override def f above, I don't know...

package a;
public class A {
  protected int f() { return 1; }

  void foo(b.B b) {
    b.f();      // not allowed
    ((A)b).f(); // ok
  }
}
package b;
public class B extends a.A {
  @Override protected int f() { return 2; }
}

[kynthus]

So maybe it makes sense to allow protected[q] override def f above, I don't know...

Unfortunately, I don't know the answer either...
However, I would like to give a minimum view.

Override Java's protected with Scala's protected

I don't think this should be forgiven.
Because Scala's protected is more stringent than Java's protected and deprives you of access from the same package.

Override Java's protected with protected[Cls or pkg]

I think it's appropriate to allow packages that all parent classes for which protected method are defined belong to in common, or that parent packages.
e.g. suppose you want to override the clone() of the following two Java classes.

• java.lang.Thread
• java.util.AbstractMap

package java.lang

class EXThread {
  /* Allow `java` or `lang` */
  protected[java] override def clone(): AnyRef = ???
}

package java.util

import java.util

class EXMap[K, V] extends util.AbstractMap[K, V] {
  /* Allow `java` only, because `util` is hidden from` java.lang` */
  protected[java] override def clone(): AnyRef = ???
  /* ...other overrides... */
}

※It is not important here that the clone in java.lang.Thread is java.lang.CloneNotSupportedException...

If the package common to all parent classes is _root_, I think it will not be possible to override with protected[Cls or pkg].
But, it is subtle whether Scala's measures are effective, as Java itself is involved in the act of tightening restrictions.

Override Java's protected with protected[this]

I think it should be banned, but this is a problem with Scala itself.
I was wondering, is it reasonable to allow only protected[this] when overriding protected[this]?

If the above view is wrong, I would be grateful if you could give me some advice.
In addition, above is the same for Dotty.

I didn't include it in scala/scala#9525 to avoid distraction.
Will you take any measures against the above in the future?

[lrytz]

At this point, I believe it's probalby best if the Scala compiler is a little bit too permissive, even if some overrides should be rejected. I think the fixes you did in 9525 are good. But the corner cases of protected in Java and protected[this] in Scala are so hard to understand for an average programmer, I think it's better to keep the rest as-is. Making the checks more precise would probably cause more pain at this point (risk of breaking existing code, especially if we dis-allow overriding Java protected with Scala protected) than remaining slightly too permissive.

I would much rather see #9334 fixed, as this compiles but causes a run-time error.

[kynthus]

Making the checks more precise would probably cause more pain at this point (risk of breaking existing code, especially if we dis-allow overriding Java protected with Scala protected) than remaining slightly too permissive.

Yes, I very much agree.
I can imagine a future where questions will be asked why protected in Java cannot be overridden by protected in Scala.

With the fix of 9525, it will be treated the same as Dotty except for the private[this].
I think this Issue has done its job, so I'll close it.

The modification of private[this] should be done in 9334.
I personally think that it will take a lot of time to fix it.