Improve handling of references to Object coming from Java code

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 #7467, #7963, #8588, #8977.

Author: smarter

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -195,7 +195,7 @@ class Definitions {
     RootClass, nme.EMPTY_PACKAGE, (emptypkg, emptycls) => ctx.base.rootLoader(emptypkg)).entered
   @tu lazy val EmptyPackageClass: ClassSymbol = EmptyPackageVal.moduleClass.asClass
 
-  /** A package in which we can place all methods that are interpreted specially by the compiler */
+  /** A package in which we can place all methods and types that are interpreted specially by the compiler */
   @tu lazy val OpsPackageVal: TermSymbol = newCompletePackageSymbol(RootClass, nme.OPS_PACKAGE).entered
   @tu lazy val OpsPackageClass: ClassSymbol = OpsPackageVal.moduleClass.asClass
 
@@ -310,6 +310,103 @@ class Definitions {
   }
   def ObjectType: TypeRef = ObjectClass.typeRef
 
+  /** A type alias of Object used to represent any reference to Object in a Java
+   *  signature, the secret sauce is that subtype checking treats it specially:
+   *
+   *    tp <:< FromJavaObject
+   *
+   *  is equivalent to:
+   *
+   *    tp <:< Any
+   *
+   *  This is useful to avoid usability problems when interacting with Java
+   *  code where Object is the top type. This is safe because this type will
+   *  only appear in signatures of Java definitions in positions where `Object`
+   *  might appear, let's enumerate all possible cases this gives us:
+   *
+   *  1. At the top level:
+   *
+   *       // A.java
+   *       void meth1(Object arg) {}
+   *       <T> void meth2(T arg) {} // T implicitly extends Object
+   *
+   *       // B.scala
+   *       meth1(1) // OK
+   *       meth2(1) // OK
+   *
+   *     This is safe even though Int is not a subtype of Object, because Erasure
+   *     will detect the mismatch and box the value type.
+   *
+   *  2. In a class type parameter:
+   *
+   *       // A.java
+   *       void meth3(scala.List<Object> arg) {}
+   *       <T> void meth4(scala.List<T> arg) {}
+   *
+   *       // B.scala
+   *       meth3(List[Int](1)) // OK
+   *       meth4(List[Int](1)) // OK
+   *
+   *     At erasure, type parameters are removed and value types are boxed.
+   *
+   *  3. As the type parameter of an array:
+   *
+   *       // A.java
+   *       void meth5(Object[] arg) {}
+   *       <T> void meth6(T[] arg) {}
+   *
+   *       // B.scala
+   *       meth5(Array[Int](1)) // error: Array[Int] is not a subtype of Array[Object]
+   *       meth6(Array[Int](1)) // error: Array[Int] is not a subtype of Array[T & Object]
+   *
+   *
+   *     This is a bit more subtle: at erasure, Arrays keep their type parameter,
+   *     and primitive Arrays are not subtypes of reference Arrays on the JVM,
+   *     so we can't pass an Array of Int where a reference Array is expected.
+   *     Array is invariant in Scala, so `meth5` is safe even if we use `FromJavaObject`,
+   *     but generic Arrays are treated specially: we always add `& Object` (and here
+   *     we mean the normal java.lang.Object type) to these types when they come from
+   *     Java signatures (see `translateJavaArrayElementType`), this ensure that `meth6`
+   *     is safe to use.
+   *
+   *  4. As the repeated argument of a varargs method:
+   *
+   *       // A.java
+   *       void meth7(Object... args) {}
+   *       <T> void meth8(T... args) {}
+   *
+   *       // B.scala
+   *       meth7(1) // OK
+   *       meth8(1) // OK
+   *       val ai = Array[Int](1)
+   *       meth7(ai: _*) // OK (will copy the array)
+   *       meth8(ai: _*) // OK (will copy the array)
+   *
+   *     Java repeated arguments are erased to arrays, so it would be safe to treat
+   *     them in the same way: add an `& Object` to the parameter type to disallow
+   *     passing primitives, but that would be very inconvenient as it is common to
+   *     want to pass a primitive to an Object repeated argument (e.g.
+   *     `String.format("foo: %d", 1)`). So instead we type them _without_ adding the
+   *     `& Object` and let `ElimRepeated` take care of doing any necessary adaptation
+   *     (it would be more consistent to treat arrays in the same way, but adapting a
+   *     primitive array to a reference array requires copying the whole array, while
+   *     this seems reasonable for repeated arguments which are typically not very
+   *     long, doing this implicitly for arbitrary arrays would be surprising).
+   *
+   *
+   *  This mechanism is similar to `ObjectTpeJavaRef` in Scala 2, except that we
+   *  create a new symbol with its own name, this is needed because this type
+   *  can show up in inferred types and therefore needs to be preserved when
+   *  pickling so that unpickled trees pass `-Ycheck`.
+   *
+   *  Note that by default we pretty-print `FromJavaObject` as `Object` or simply omit it
+   *  if it's the sole upper-bound of a type parameter, use `-Yprint-debug` to explicitly
+   *  display it.
+   */
+  @tu lazy val FromJavaObjectSymbol: TypeSymbol =
+    newPermanentSymbol(OpsPackageClass, tpnme.FromJavaObject, JavaDefined, TypeAlias(ObjectType)).entered
+  def FromJavaObjectType: TypeRef = FromJavaObjectSymbol.typeRef
+
   @tu lazy val AnyRefAlias: TypeSymbol = enterAliasType(tpnme.AnyRef, ObjectType)
   def AnyRefType: TypeRef = AnyRefAlias.typeRef
 

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -199,6 +199,7 @@ object StdNames {
     final val Null: N                = "Null"
     final val UncheckedNull: N            = "UncheckedNull"
     final val Object: N              = "Object"
+    final val FromJavaObject: N      = "<FromJavaObject>"
     final val Product: N             = "Product"
     final val PartialFunction: N     = "PartialFunction"
     final val PrefixType: N          = "PrefixType"

compiler/src/dotty/tools/dotc/core/TypeApplications.scala

@@ -430,7 +430,7 @@ class TypeApplications(val self: Type) extends AnyVal {
    *  So hopefully our grand-children will not have to deal with this non-sense!
    */
   def translateJavaArrayElementType(using Context): Type =
-    if self.typeSymbol.isAbstractOrParamType && !self.derivesFrom(defn.ObjectClass) then
+    if self.typeSymbol.isAbstractOrParamType && (self.classSymbol eq defn.ObjectClass) then
       AndType(self, defn.ObjectType)
     else
       self

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

@@ -275,7 +275,11 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
             case _ =>
               secondTry
         end compareNamed
-        compareNamed(tp1, tp2)
+        // See the documentation of `FromJavaObjectSymbol`
+        if !ctx.erasedTypes && tp2.isFromJavaObject then
+          recur(tp1, defn.AnyType)
+        else
+          compareNamed(tp1, tp2)
       case tp2: ProtoType =>
         isMatchedByProto(tp2, tp1)
       case tp2: BoundType =>
@@ -1769,35 +1773,15 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
   }
 
   /** Do the parameter types of `tp1` and `tp2` match in a way that allows `tp1`
-   *  to override `tp2` ? This is the case if they're pairwise =:=, as a special
-   *  case, we allow `Any` in Java methods to match `Object`.
+   *  to override `tp2` ? This is the case if they're pairwise `=:=`.
    */
   def matchingMethodParams(tp1: MethodType, tp2: MethodType): Boolean = {
     def loop(formals1: List[Type], formals2: List[Type]): Boolean = formals1 match {
       case formal1 :: rest1 =>
         formals2 match {
           case formal2 :: rest2 =>
             val formal2a = if (tp2.isParamDependent) formal2.subst(tp2, tp1) else formal2
-            // The next two definitions handle the special case mentioned above, where
-            // the Java argument has type 'Any', and the Scala argument has type 'Object' or
-            // 'Object|Null', depending on whether explicit nulls are enabled.
-            def formal1IsObject =
-              if (ctx.explicitNulls) formal1 match {
-                case OrNull(formal1b) => formal1b.isAnyRef
-                case _ => false
-              }
-              else formal1.isAnyRef
-            def formal2IsObject =
-              if (ctx.explicitNulls) formal2 match {
-                case OrNull(formal2b) => formal2b.isAnyRef
-                case _ => false
-              }
-              else formal2.isAnyRef
-            (isSameTypeWhenFrozen(formal1, formal2a)
-             || tp1.isJavaMethod && formal2IsObject && formal1.isAny
-             || tp2.isJavaMethod && formal1IsObject && formal2.isAny
-            )
-            && loop(rest1, rest2)
+            isSameTypeWhenFrozen(formal1, formal2a) && loop(rest1, rest2)
           case nil =>
             false
         }

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -205,6 +205,8 @@ object Types {
     def isAnyRef(using Context): Boolean  = isRef(defn.ObjectClass, skipRefined = false)
     def isAnyKind(using Context): Boolean = isRef(defn.AnyKindClass, skipRefined = false)
 
+    def isFromJavaObject(using Context): Boolean = typeSymbol eq defn.FromJavaObjectSymbol
+
     /** Does this type refer exactly to class symbol `sym`, instead of to a subclass of `sym`?
      *  Implemented like `isRef`, but follows more types: all type proxies as well as and- and or-types
      */

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -305,10 +305,6 @@ class ClassfileParser(
     }
   }
 
-  /** Map direct references to Object to references to Any */
-  final def objToAny(tp: Type)(using Context): Type =
-    if (tp.isDirectRef(defn.ObjectClass) && !ctx.phase.erasedTypes) defn.AnyType else tp
-
   def constantTagToType(tag: Int)(using Context): Type =
     (tag: @switch) match {
       case BYTE_TAG   => defn.ByteType
@@ -356,14 +352,14 @@ class ClassfileParser(
                     case variance @ ('+' | '-' | '*') =>
                       index += 1
                       variance match {
-                        case '+' => objToAny(TypeBounds.upper(sig2type(tparams, skiptvs)))
+                        case '+' => TypeBounds.upper(sig2type(tparams, skiptvs))
                         case '-' =>
-                          val tp = sig2type(tparams, skiptvs)
-                          // sig2type seems to return AnyClass regardless of the situation:
-                          // we don't want Any as a LOWER bound.
-                          if (tp.isDirectRef(defn.AnyClass)) TypeBounds.empty
-                          else TypeBounds.lower(tp)
-                        case '*' => TypeBounds.empty
+                          val argTp = sig2type(tparams, skiptvs)
+                          // Interpret `sig2type` returning `Any` as "no bounds";
+                          // morally equivalent to TypeBounds.empty, but we're representing Java code, so use FromJavaObjectType as the upper bound
+                          if (argTp.typeSymbol == defn.AnyClass) TypeBounds.upper(defn.FromJavaObjectType)
+                          else TypeBounds(argTp, defn.FromJavaObjectType)
+                        case '*' => TypeBounds.upper(defn.FromJavaObjectType)
                       }
                     case _ => sig2type(tparams, skiptvs)
                   }
@@ -379,7 +375,8 @@ class ClassfileParser(
           }
 
           val classSym = classNameToSymbol(subName(c => c == ';' || c == '<'))
-          var tpe = processClassType(processInner(classSym.typeRef))
+          val classTpe = if (classSym eq defn.ObjectClass) defn.FromJavaObjectType else classSym.typeRef
+          var tpe = processClassType(processInner(classTpe))
           while (sig(index) == '.') {
             accept('.')
             val name = subName(c => c == ';' || c == '<' || c == '.').toTypeName
@@ -426,7 +423,7 @@ class ClassfileParser(
                 // `ElimRepeated` is responsible for correctly erasing this.
                 defn.RepeatedParamType.appliedTo(elemType)
               else
-                objToAny(sig2type(tparams, skiptvs))
+                sig2type(tparams, skiptvs)
             }
 
           index += 1
@@ -448,7 +445,7 @@ class ClassfileParser(
       while (sig(index) == ':') {
         index += 1
         if (sig(index) != ':') // guard against empty class bound
-          ts += objToAny(sig2type(tparams, skiptvs))
+          ts += sig2type(tparams, skiptvs)
       }
       val bound = if ts.isEmpty then defn.AnyType else ts.reduceLeft(AndType.apply)
       TypeBounds.upper(bound)
@@ -497,7 +494,7 @@ class ClassfileParser(
         classTParams = tparams
         val parents = new ListBuffer[Type]()
         while (index < end)
-          parents += sig2type(tparams, skiptvs = false)  // here the variance doesn't matter
+          parents += sig2type(tparams, skiptvs = false) // here the variance doesn't matter
         TempClassInfoType(parents.toList, instanceScope, owner)
       }
     if (ownTypeParams.isEmpty) tpe else TempPolyType(ownTypeParams, tpe)

compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala

@@ -304,7 +304,7 @@ object JavaParsers {
         if (in.token == QMARK) {
           val offset = in.offset
           in.nextToken()
-          val hi = if (in.token == EXTENDS) { in.nextToken() ; typ() } else EmptyTree
+          val hi = if (in.token == EXTENDS) { in.nextToken() ; typ() } else javaLangObject()
           val lo = if (in.token == SUPER)   { in.nextToken() ; typ() } else EmptyTree
           atSpan(offset) {
             /*
@@ -434,7 +434,7 @@ object JavaParsers {
     def typeParam(flags: FlagSet): TypeDef =
       atSpan(in.offset) {
         val name = identForType()
-        val hi = if (in.token == EXTENDS) { in.nextToken() ; bound() } else EmptyTree
+        val hi = if (in.token == EXTENDS) { in.nextToken() ; bound() } else javaLangObject()
         TypeDef(name, TypeBoundsTree(EmptyTree, hi)).withMods(Modifiers(flags))
       }
 

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

@@ -3,6 +3,7 @@ package printing
 
 import core._
 import Texts._, Types._, Flags._, Names._, Symbols._, NameOps._, Constants._, Denotations._
+import StdNames._
 import Contexts._
 import Scopes.Scope, Denotations.Denotation, Annotations.Annotation
 import StdNames.nme
@@ -89,7 +90,10 @@ class PlainPrinter(_ctx: Context) extends Printer {
     || (sym.name == nme.PACKAGE)               // package
   )
 
-  def nameString(name: Name): String = name.toString
+  def nameString(name: Name): String =
+    if (name eq tpnme.FromJavaObject) && !printDebug
+    then nameString(tpnme.Object)
+    else name.toString
 
   def toText(name: Name): Text = Str(nameString(name))
 
@@ -123,11 +127,13 @@ class PlainPrinter(_ctx: Context) extends Printer {
     })
 
   /** Direct references to these symbols are printed without their prefix for convenience.
-   *  They are either aliased in scala.Predef or in the scala package object.
+   *  They are either aliased in scala.Predef or in the scala package object, as well as `Object`
    */
   private lazy val printWithoutPrefix: Set[Symbol] =
     (defn.ScalaPredefModule.termRef.typeAliasMembers
       ++ defn.ScalaPackageObject.termRef.typeAliasMembers).map(_.info.classSymbol).toSet
+    + defn.ObjectClass
+    + defn.FromJavaObjectSymbol
 
   def toText(tp: Type): Text = controlled {
     homogenize(tp) match {
@@ -267,7 +273,9 @@ class PlainPrinter(_ctx: Context) extends Printer {
     simpleNameString(sym) + idString(sym) // + "<" + (if (sym.exists) sym.owner else "") + ">"
 
   def fullNameString(sym: Symbol): String =
-    if (sym.isRoot || sym == NoSymbol || sym.owner.isEffectiveRoot)
+    if (sym eq defn.FromJavaObjectSymbol) && !printDebug then
+      fullNameString(defn.ObjectClass)
+    else if sym.isRoot || sym == NoSymbol || sym.owner.isEffectiveRoot then
       nameString(sym)
     else
       fullNameString(fullNameOwner(sym)) + "." + nameString(sym)
@@ -365,7 +373,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
             " = " ~ toText(tp.alias)
           case TypeBounds(lo, hi) =>
             (if (lo isRef defn.NothingClass) Text() else " >: " ~ toText(lo))
-            ~ (if hi.isAny then Text() else " <: " ~ toText(hi))
+            ~ (if hi.isAny || (!printDebug && hi.isFromJavaObject) then Text() else " <: " ~ toText(hi))
         tparamStr ~ binder
       case tp @ ClassInfo(pre, cls, cparents, decls, selfInfo) =>
         val preText = toTextLocal(pre)

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala

@@ -76,7 +76,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
   }
 
   override def nameString(name: Name): String =
-    if (ctx.settings.YdebugNames.value) name.debugString else name.toString
+    if ctx.settings.YdebugNames.value
+    then name.debugString
+    else super.nameString(name)
 
   override protected def simpleNameString(sym: Symbol): String =
     nameString(if (ctx.property(XprintMode).isEmpty) sym.initial.name else sym.name)