Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dotty rejects all overrides of some Java methods as duplicate methods #7467

Closed
arthurp opened this issue Oct 29, 2019 · 1 comment · Fixed by #9601
Closed

Dotty rejects all overrides of some Java methods as duplicate methods #7467

arthurp opened this issue Oct 29, 2019 · 1 comment · Fixed by #9601

Comments

@arthurp
Copy link

arthurp commented Oct 29, 2019

It appears that there is no way to override Java methods which have "incorrectly" typed (from Scala's perspective) overrides in the superclass even though the superclass's override is allowed by the Java compiler. Specifically javax.swing.DefaultListCellRenderer.getListCellRendererComponent's first argument is JList<? extends E> in ListCellRenderer<E>, but JList<?> in DefaultListCellRenderer which implements ListCellRenderer<Object>. I suspect that the Java type system allows this, since for Java ? extends Object is equivalent to ?.

This is actually a very old issue from the Scala 2.9 days. The Java code is questionable, but it is accepted by the Java compiler and is still present in OpenJDK 14. Scala 2.11 also rejects this code with a similar error.

minimized code

import javax.swing._
import java.awt._

class DuplicateSymbolError_DirectSuperclass extends DefaultListCellRenderer() {
  override def getListCellRendererComponent(list: JList[_ <: Object], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ???
}

class DuplicateSymbolError_IndirectInterface extends DefaultListCellRenderer() {
  override def getListCellRendererComponent(list: JList[_], value: Object, index: Int, isSelected: Boolean, cellHasFocus: Boolean): Component = ???
}

expectation

I would expect some class declaration overriding this method to be accepted by the Scala compiler. I would select DuplicateSymbolError_IndirectInterface myself, but I'm not picky. Currently (and in Scala 2.11) both are rejected.

Or failing that Scala could produce an error message hinting that the Java classes may have inconsistent types to simplify the debugging process for programmers who encounter this.

PS: In this specific case, the work around is to use composition instead of inheritance. But, if some of the superclass called the problem method, there would be no work around (other than a hacky class written in Java to forward the virtual call to the wrapper).

@smarter
Copy link
Member

smarter commented Jun 5, 2020

Scala 2.13 accepts both possible overrides of getListCellRendererComponent (JList[_] and JList[_ <: Object] are the same type), I'm working on porting the mechanism it uses to dotty.

@smarter smarter self-assigned this Jun 5, 2020
smarter added a commit to dotty-staging/dotty that referenced this issue Aug 19, 2020
In Java unlike Scala, `Object` is the top type, this leads to various
usability problems when attempting to call or override a Java method
from Scala. So far, we relied mainly on one mechanism to improve the
situation: in the ClassfileParser, some references to `Object` in
signatures were replaced by `Any` (cf `objToAny`). But this had several
shortcomings:
- To compensate for this substitution,
  `TypeComparer#matchingMethodParams` had to special case Any in Java
  methods and treat it like Object
- There were various situation were this substitution was not applied,
  notably when using varargs (`Object... args`) or when jointly
  compiling .java sources since this is handled by JavaParser and not
  ClassfileParser.

This commit replaces all of this by a more systematic solution: all
references to `Object` in Java definitions (both in classfiles and .java
source files) are replaced by a special type `FromJavaObject` which is a
type alias of `Object` with one extra subtyping rule:

   tp <:< FromJavaObject

is equivalent to:

   tp <:< Any

See the documentation of `FromJavaObjectSymbol` for details on
why this makes sense.

This solution is very much inspired by
scala/scala#7966 (which was refined in
scala/scala#8049 and
scala/scala#8638) with two main differences:
- We use a type with its own symbol and name distinct from
  `java.lang.Object`, because this type can show up in inferred types
  and therefore needs to be preserved when pickling so that unpickled
  trees pass `-Ycheck`.
- Unlike in Scala 2, we cannot get rid of `JavaMethodType` because when
  calling `Type#signature` we need to know whether we're in a Java
  method or not, because signatures are based on erased types and erasure
  differs between Scala and Java (we cannot ignore this and always
  base signatures on the Scala erasure, because signatures are used
  to distinguish between overloads and overrides so they must agree
  with the actual JVM bytecode signature).

Fixes scala#7467, scala#7963, scala#8588, scala#8977.
smarter added a commit to dotty-staging/dotty that referenced this issue Aug 21, 2020
In Java unlike Scala, `Object` is the top type, this leads to various
usability problems when attempting to call or override a Java method
from Scala. So far, we relied mainly on one mechanism to improve the
situation: in the ClassfileParser, some references to `Object` in
signatures were replaced by `Any` (cf `objToAny`). But this had several
shortcomings:
- To compensate for this substitution,
  `TypeComparer#matchingMethodParams` had to special case Any in Java
  methods and treat it like Object
- There were various situation were this substitution was not applied,
  notably when using varargs (`Object... args`) or when jointly
  compiling .java sources since this is handled by JavaParser and not
  ClassfileParser.

This commit replaces all of this by a more systematic solution: all
references to `Object` in Java definitions (both in classfiles and .java
source files) are replaced by a special type `FromJavaObject` which is a
type alias of `Object` with one extra subtyping rule:

   tp <:< FromJavaObject

is equivalent to:

   tp <:< Any

See the documentation of `FromJavaObjectSymbol` for details on
why this makes sense.

This solution is very much inspired by
scala/scala#7966 (which was refined in
scala/scala#8049 and
scala/scala#8638) with two main differences:
- We use a type with its own symbol and name distinct from
  `java.lang.Object`, because this type can show up in inferred types
  and therefore needs to be preserved when pickling so that unpickled
  trees pass `-Ycheck`.
- Unlike in Scala 2, we cannot get rid of `JavaMethodType` because when
  calling `Type#signature` we need to know whether we're in a Java
  method or not, because signatures are based on erased types and erasure
  differs between Scala and Java (we cannot ignore this and always
  base signatures on the Scala erasure, because signatures are used
  to distinguish between overloads and overrides so they must agree
  with the actual JVM bytecode signature).

Fixes scala#7467, scala#7963, scala#8588, scala#8977.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants