SI-9111 / SI-10107 resolution of static references in Java code

Scala puts static members of Java classes into the companion
object symbol. When parsing Java code, references to static
members need to be looked up in these companions.

In 2.11.x, a synthetic `import A._` was added to Java classes
having static members, which lead to some bugs (SI-9111). The
scheme was changed in 4e822d7 (2.12.x only): when a lookup
of `T` / `A.T` fails, the typer searches in the companion of the
current class.

This fixes some issues, but not all
  - In Java, `A.T` can refer to a static member defined in a
    parent of `A`
  - If a static `A.T` exists, but a parent of `A` defines a
    non-static member class `T`, the non-static one would be
    chosen (wrongly)

This commit pushes the logic to consider companion objects into
`FindMembers` and makes sure that for ever class in the base type
sequence, the companion is considered before moving to the next
base type.

Author: lrytz

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

@@ -6,8 +6,9 @@
 package scala.tools.nsc
 package typechecker
 
-import scala.collection.{ immutable, mutable }
+import scala.collection.{immutable, mutable}
 import scala.annotation.tailrec
+import scala.reflect.internal.Flags
 import scala.reflect.internal.util.shortClassOfInstance
 import scala.tools.nsc.reporters.Reporter
 
@@ -984,6 +985,10 @@ trait Contexts { self: Analyzer =>
 
     def isNameInScope(name: Name) = lookupSymbol(name, _ => true).isSuccess
 
+    def isPlainClassInJavaUnit(sym: Symbol): Boolean =
+      unit.isJava && sym.isJavaDefined && sym.isClass && !sym.isModuleClass && !sym.isPackageClass
+
+
     /** Find the symbol of a simple name starting from this context.
      *  All names are filtered through the "qualifies" predicate,
      *  the search continuing as long as no qualifying name is found.
@@ -1017,14 +1022,11 @@ trait Contexts { self: Analyzer =>
         )
       )
       def lookupInPrefix(name: Name)    = {
-        val sym = pre.member(name).filter(qualifies)
-        def isNonPackageNoModuleClass(sym: Symbol) =
-          sym.isClass && !sym.isModuleClass && !sym.isPackageClass
-        if (!sym.exists && unit.isJava && isNonPackageNoModuleClass(pre.typeSymbol)) {
-          // TODO factor out duplication with Typer::inCompanionForJavaStatic
-          val pre1 = companionSymbolOf(pre.typeSymbol, this).typeOfThis
-          pre1.member(name).filter(qualifies).andAlso(_ => pre = pre1)
-        } else sym
+        val searchCompanions = isPlainClassInJavaUnit(pre.typeSymbol)
+        val sym = pre.findMember(name, excludedFlags = Flags.BridgeFlags, requiredFlags = 0, stableOnly = false, searchCompanions).filter(qualifies)
+        if (searchCompanions && sym.owner.isModuleClass)
+          pre = sym.owner.typeOfThis
+        sym
       }
       def accessibleInPrefix(s: Symbol) = isAccessible(s, pre, superAccess = false)
 

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -668,8 +668,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         case SuperType(clazz, _) if callSiteWithinClass(clazz.typeSymbol) => true
         case _                                                            => phase.next.erasedTypes
       }
-      if (includeLocals) qual.tpe member name
-      else qual.tpe nonLocalMember name
+      val excluded = BridgeFlags | (if (includeLocals) 0 else LOCAL)
+      val searchCompanions = context.isPlainClassInJavaUnit(qual.tpe.typeSymbol)
+      qual.tpe.findMember(name, excludedFlags = excluded, requiredFlags = 0, stableOnly = false, searchCompanions)
     }
 
     def silent[T](op: Typer => T,
@@ -4795,16 +4796,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             if (isStableContext(tree, mode, pt)) tree setType clazz.thisType else tree
         }
 
-
-      // For Java, instance and static members are in the same scope, but we put the static ones in the companion object
-      // so, when we can't find a member in the class scope, check the companion
-      def inCompanionForJavaStatic(pre: Type, cls: Symbol, name: Name): Symbol =
-        if (!(context.unit.isJava && cls.isClass && !cls.isModuleClass)) NoSymbol else {
-          val companion = companionSymbolOf(cls, context)
-          if (!companion.exists) NoSymbol
-          else member(gen.mkAttributedRef(pre, companion), name) // assert(res.isStatic, s"inCompanionJavaStatic($pre, $cls, $name) = $res ${res.debugFlagString}")
-        }
-
       /* Attribute a selection where `tree` is `qual.name`.
        * `qual` is already attributed.
        */
@@ -4831,7 +4822,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           dyna.wrapErrors(t, (_.typed1(t, mode, pt)))
         }
 
-        val sym = tree.symbol orElse member(qual, name) orElse inCompanionForJavaStatic(qual.tpe.prefix, qual.symbol, name) orElse {
+        val sym = tree.symbol orElse member(qual, name) orElse {
           // symbol not found? --> try to convert implicitly to a type that does have the required
           // member.  Added `| PATTERNmode` to allow enrichment in patterns (so we can add e.g., an
           // xml member to StringContext, which in turn has an unapply[Seq] method)

src/reflect/scala/reflect/internal/Types.scala

@@ -1022,8 +1022,8 @@ trait Types
      *  @param requiredFlags  Returned members do have these flags
      *  @param stableOnly     If set, return only members that are types or stable values
      */
-    def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean): Symbol = {
-      def findMemberInternal = new FindMember(this, name, excludedFlags, requiredFlags, stableOnly).apply()
+    def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean, searchCompanions: Boolean = false): Symbol = {
+      def findMemberInternal = new FindMember(this, name, excludedFlags, requiredFlags, stableOnly, searchCompanions).apply()
 
       if (this.isGround) findMemberInternal
       else suspendingTypeVars(typeVarsInType(this))(findMemberInternal)
@@ -1115,7 +1115,7 @@ trait Types
     // todo see whether we can do without
     override def isError: Boolean = true
     override def decls: Scope = new ErrorScope(NoSymbol)
-    override def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean): Symbol = {
+    override def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean, searchCompanion: Boolean): Symbol = {
       var sym = decls lookup name
       if (sym == NoSymbol) {
         sym = NoSymbol.newErrorSymbol(name)

src/reflect/scala/reflect/internal/tpe/FindMembers.scala

@@ -13,8 +13,18 @@ trait FindMembers {
   this: SymbolTable =>
 
   /** Implementation of `Type#{findMember, findMembers}` */
-  private[internal] abstract class FindMemberBase[T](tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long) {
-    protected val initBaseClasses: List[Symbol] = tpe.baseClasses
+  private[internal] abstract class FindMemberBase[T](tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long, searchCompanions: Boolean) {
+    protected val initBaseClasses: List[Symbol] = {
+      val bcs = tpe.baseClasses
+      // For Java compilation units, a `T` or `A.T` can refer to a static inner class `T`. In the Scala compiler,
+      // we model static members are members of the companion object. Under `searchCompanions`, we always check the
+      // companion before proceeding to the next base class. This follows Java's semantics of "inheriting" static
+      // members: `A.T` can resolve to a static member defined in a parent of `A`.
+      //   class C { class T }; class B extends C { static class T }; class A extends B
+      // Here, `A.T` resolves to the static class `B.T`, so we need to check B's companion before proceeding to C.
+      if (searchCompanions) bcs.flatMap(bc => List(bc, bc.companionModule.moduleClass))
+      else bcs
+    }
 
     // The first base class, or the symbol of the ThisType
     // e.g in:
@@ -191,7 +201,7 @@ trait FindMembers {
   }
 
   private[reflect] final class FindMembers(tpe: Type, excludedFlags: Long, requiredFlags: Long)
-    extends FindMemberBase[Scope](tpe, nme.ANYname, excludedFlags, requiredFlags) {
+    extends FindMemberBase[Scope](tpe, nme.ANYname, excludedFlags, requiredFlags, searchCompanions = false) {
     private[this] var _membersScope: Scope   = null
     private def membersScope: Scope = {
       if (_membersScope eq null) _membersScope = newFindMemberScope
@@ -215,8 +225,8 @@ trait FindMembers {
     }
   }
 
-  private[reflect] final class FindMember(tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean)
-    extends FindMemberBase[Symbol](tpe, name, excludedFlags, requiredFlags) {
+  private[reflect] class FindMember(tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean, searchCompanions: Boolean)
+    extends FindMemberBase[Symbol](tpe, name, excludedFlags, requiredFlags, searchCompanions) {
     // Gathering the results into a hand rolled ListBuffer
     // TODO Try just using a ListBuffer to see if this low-level-ness is worth it.
     private[this] var member0: Symbol       = NoSymbol

test/files/neg/t9111c.check

@@ -1,6 +1,6 @@
-Test_1.scala:6: error: type mismatch;
- found   : A_1.P.T
- required: A_1.T
-  assert(j.foo(new A_1.P.T()) == 1) // TODO SI-9111: should compile without error (currently fails, required: A_1.T)
+Test_1.scala:5: error: type mismatch;
+ found   : A_1.T
+ required: A_1.P.T
+  assert(j.foo(new A_1.T()) == 1)   // type mismatch, required: A_1.P.T
                ^
 one error found

test/files/neg/t9111c/A_1.java

@@ -8,4 +8,10 @@ public class Deeper {
       public int foo(T t) { return t.f(); }
     }
   }
+
+  public static void crossCheck() {
+    Inner i = new Inner();
+    Inner.Deeper d = i.new Deeper();
+    d.foo(new P.T());
+  }
 }

test/files/neg/t9111c/Test_1.scala

@@ -2,6 +2,6 @@ object Test extends App {
   val i = new A_1.Inner()
   val j = new i.Deeper()
 
-  assert(j.foo(new A_1.T()) == 1)   // TODO SI-9111: should not compile (need an A_1.P.T) (currently passes)
-  assert(j.foo(new A_1.P.T()) == 1) // TODO SI-9111: should compile without error (currently fails, required: A_1.T)
+  assert(j.foo(new A_1.T()) == 1)   // type mismatch, required: A_1.P.T
+  assert(j.foo(new A_1.P.T()) == 1) // compiles, no error, see run/t9111b
 }

test/files/neg/t9111d.check

@@ -0,0 +1,37 @@
+Test_1.scala:4: error: type T is not a member of object A_1.C
+  val f2: A_1.C.T = e.foo()  // error: T is not a member of A_1.C (it's a non-static class)
+                ^
+Test_1.scala:5: error: type mismatch;
+ found   : A_1.D.T
+ required: A_1.C#T
+  val f3: A_1.C#T = e.foo()  // mismatch, found: A_1.D.T
+                         ^
+Test_1.scala:8: error: type mismatch;
+ found   : A_1.C#T
+ required: Test.e.T
+  val b2: e.T     = e.bar()  // mismatch, bar returns A_1.C#T, not e.T - no path dependent types in Java
+                         ^
+Test_1.scala:9: error: type mismatch;
+ found   : A_1.C#T
+ required: A_1.D.T
+  val b3: A_1.D.T = e.bar()  // mismatch, found: A_1.C#T
+                         ^
+Test_1.scala:12: error: type T is not a member of object A_1.E
+  val m2: A_1.E.T = A_1.miz() // error: T is not a member of A_1.E - this selection is OK in Java, but not in Scala
+                ^
+Test_1.scala:13: error: type mismatch;
+ found   : A_1.D.T
+ required: A_1.C#T
+  val m3: A_1.C#T = A_1.miz() // mismatch, found: A_1.D.T
+                           ^
+Test_1.scala:16: error: type mismatch;
+ found   : A_1.D.T
+ required: A_1.C#T
+  val p2: A_1.C#T = A_1.paz() // mismatch, found: A_1.D.T
+                           ^
+Test_1.scala:19: error: type mismatch;
+ found   : A_1.C#T
+ required: A_1.D.T
+  val t2: A_1.D.T = A_1.tux() // mismatch, found: A_1.C#T
+                           ^
+8 errors found

test/files/neg/t9111d/A_1.java

@@ -0,0 +1,37 @@
+public class A_1 {
+  static class C {
+    class T {}
+  }
+  static class D extends C {
+    static class T {}
+  }
+  static class E extends D {
+
+    // An unqualified T resolves to the static class D.T.
+    // The scala compiler puts D.T into the companion object of D.
+    // When performing a lookup of T, we need to make sure to consider the companion object of D
+    // before checking the superclass C, which has an instance member T (SI-9111).
+
+    public T foo() {
+      return new D.T(); // cross-check: T is D.T
+    }
+    public C.T bar() {
+      // C.T is not a static class, `this` is implicitly used as enclosing instance - `this.new T()` doesn't work.
+      return new C.T();
+    }
+  }
+
+  // A qualified E.T also resolves to D.T
+  public static E.T miz() {
+    return new D.T(); // cross-check: E.T is D.T
+  }
+
+  public static D.T paz() {
+    return new E.T();
+  }
+
+  public static C.T tux() {
+    C c = new C();
+    return c.new T();
+  }
+}

test/files/neg/t9111d/Test_1.scala

@@ -0,0 +1,20 @@
+object Test {
+  val e = new A_1.E()
+  val f1: A_1.D.T = e.foo()  // no error
+  val f2: A_1.C.T = e.foo()  // error: T is not a member of A_1.C (it's a non-static class)
+  val f3: A_1.C#T = e.foo()  // mismatch, found: A_1.D.T
+
+  val b1: A_1.C#T = e.bar()  // no error
+  val b2: e.T     = e.bar()  // mismatch, bar returns A_1.C#T, not e.T - no path dependent types in Java
+  val b3: A_1.D.T = e.bar()  // mismatch, found: A_1.C#T
+
+  val m1: A_1.D.T = A_1.miz() // no error
+  val m2: A_1.E.T = A_1.miz() // error: T is not a member of A_1.E - this selection is OK in Java, but not in Scala
+  val m3: A_1.C#T = A_1.miz() // mismatch, found: A_1.D.T
+
+  val p1: A_1.D.T = A_1.paz() // no error
+  val p2: A_1.C#T = A_1.paz() // mismatch, found: A_1.D.T
+
+  val t1: A_1.C#T = A_1.tux() // no error
+  val t2: A_1.D.T = A_1.tux() // mismatch, found: A_1.C#T
+}

test/files/run/t10107/A_1.java

@@ -0,0 +1,5 @@
+public class A_1 {
+  public static class AInner {
+    public static interface InnerInterface { }
+  }
+}

test/files/run/t10107/B_1.java

@@ -0,0 +1,5 @@
+public class B_1 {
+  public static class BInner extends A_1.AInner  {
+    public class ProblemClass implements InnerInterface { }
+  }
+}

test/files/run/t10107/Test_1.scala

@@ -0,0 +1,7 @@
+object Test {
+  def main(args: Array[String]): Unit = {
+    new B_1()
+    val bi = new B_1.BInner
+    new bi.ProblemClass()
+  }
+}

test/files/run/t9111a/A_1.java

@@ -4,4 +4,9 @@ public static final class Inner {
     public static final class T { public int x = 2; }
     public T newT() { return new T(); }
   }
+
+  public static int crossCheck() {
+    Inner i = new Inner();
+    return i.newT().x;
+  }
 }

test/files/run/t9111a/Test_1.scala

@@ -1,4 +1,5 @@
 object Test extends App {
   val i = new A_1.Inner()
   assert(i.newT().x == 2)
+  assert(A_1.crossCheck() == 2)
 }

test/files/run/t9111b/A_1.java

@@ -8,4 +8,10 @@ public class Deeper {
       public int foo(T t) { return t.f(); }
     }
   }
+
+  public static void crossCheck() {
+    Inner i = new Inner();
+    Inner.Deeper d = i.new Deeper();
+    d.foo(new P.T());
+  }
 }

test/files/run/t9111b/Test_1.scala

@@ -2,9 +2,6 @@ object Test extends App {
   val i = new A_1.Inner()
   val j = new i.Deeper()
 
-  // TODO SI-9111: causes a NoSuchMethodError, should fail in mixed compilation.
-  // assert(j.foo(new A_1.T()) == 1)
-
-  // TODO SI-9111: fails in mixed compilation (found: A_1.P.T, required: A_1.T), should pass.
-  // assert(j.foo(new A_1.P.T()) == 1)
+//  assert(j.foo(new A_1.T()) == 1) // type mismatch, required: A_1.P.T, see neg/t9111
+  assert(j.foo(new A_1.P.T()) == 1) // compiles, no error
 }