Merge pull request #1465 from dotty-staging/fix-#1457

Fix #1457: Three incompatbilities with scalac

Author: odersky

src/dotty/tools/dotc/typer/ImportInfo.scala

@@ -64,6 +64,7 @@ class ImportInfo(symf: => Symbol, val selectors: List[untpd.Tree], val isRootImp
             myExcluded += name
           case Pair(Ident(from: TermName), Ident(to: TermName)) =>
             myMapped = myMapped.updated(to, from)
+            myExcluded += from
             myOriginals += from
           case Ident(nme.WILDCARD) =>
             myWildcardImport = true

src/dotty/tools/dotc/typer/Typer.scala

@@ -72,6 +72,13 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
    */
   private var importedFromRoot: Set[Symbol] = Set()
 
+  /** Temporary data item for single call to typed ident:
+   *  This symbol would be found under Scala2 mode, but is not
+   *  in dotty (because dotty conforms to spec section 2
+   *  wrt to package member resolution but scalac doe not).
+   */
+  private var foundUnderScala2: Type = NoType
+
   def newLikeThis: Typer = new Typer
 
   /** Attribute an identifier consisting of a simple name or wildcard
@@ -133,14 +140,20 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
        *      imported by <tree>
        *  or  defined in <symbol>
        */
-      def bindingString(prec: Int, whereFound: Context, qualifier: String = "") =
+      def bindingString(prec: Int, whereFound: Context, qualifier: String = "")(implicit ctx: Context) =
         if (prec == wildImport || prec == namedImport) ex"imported$qualifier by ${whereFound.importInfo}"
         else ex"defined$qualifier in ${whereFound.owner}"
 
       /** Check that any previously found result from an inner context
        *  does properly shadow the new one from an outer context.
+       *  @param found     The newly found result
+       *  @param newPrec   Its precedence
+       *  @param scala2pkg Special mode where we check members of the same package, but defined
+       *                   in different compilation units under Scala2. If set, and the
+       *                   previous and new contexts do not have the same scope, we select
+       *                   the previous (inner) definition. This models what scalac does.
        */
-      def checkNewOrShadowed(found: Type, newPrec: Int): Type =
+      def checkNewOrShadowed(found: Type, newPrec: Int, scala2pkg: Boolean = false)(implicit ctx: Context): Type =
         if (!previous.exists || ctx.typeComparer.isSameRef(previous, found)) found
         else if ((prevCtx.scope eq ctx.scope) &&
                  (newPrec == definition ||
@@ -150,7 +163,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
           found
         }
         else {
-          if (!previous.isError && !found.isError) {
+          if (!scala2pkg && !previous.isError && !found.isError) {
             error(
               ex"""reference to $name is ambiguous;
                   |it is both ${bindingString(newPrec, ctx, "")}
@@ -163,7 +176,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
       /** The type representing a named import with enclosing name when imported
        *  from given `site` and `selectors`.
        */
-      def namedImportRef(site: Type, selectors: List[untpd.Tree]): Type = {
+      def namedImportRef(site: Type, selectors: List[untpd.Tree])(implicit ctx: Context): Type = {
         def checkUnambiguous(found: Type) = {
           val other = namedImportRef(site, selectors.tail)
           if (other.exists && found.exists && (found != other))
@@ -190,7 +203,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
       /** The type representing a wildcard import with enclosing name when imported
        *  from given import info
        */
-      def wildImportRef(imp: ImportInfo): Type = {
+      def wildImportRef(imp: ImportInfo)(implicit ctx: Context): Type = {
         if (imp.isWildcardImport) {
           val pre = imp.site
           if (!isDisabled(imp, pre) && !(imp.excluded contains name.toTermName) && name != nme.CONSTRUCTOR) {
@@ -204,54 +217,71 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
       /** Is (some alternative of) the given predenotation `denot`
        *  defined in current compilation unit?
        */
-      def isDefinedInCurrentUnit(denot: Denotation): Boolean = denot match {
+      def isDefinedInCurrentUnit(denot: Denotation)(implicit ctx: Context): Boolean = denot match {
         case MultiDenotation(d1, d2) => isDefinedInCurrentUnit(d1) || isDefinedInCurrentUnit(d2)
         case denot: SingleDenotation => denot.symbol.sourceFile == ctx.source.file
       }
 
       /** Is `denot` the denotation of a self symbol? */
-      def isSelfDenot(denot: Denotation) = denot match {
+      def isSelfDenot(denot: Denotation)(implicit ctx: Context) = denot match {
         case denot: SymDenotation => denot is SelfName
         case _ => false
       }
 
-      // begin findRef
-      if (ctx.scope == null) previous
-      else {
-        val outer = ctx.outer
-        if ((ctx.scope ne outer.scope) || (ctx.owner ne outer.owner)) {
-          val defDenot = ctx.denotNamed(name)
-          if (qualifies(defDenot)) {
-            val curOwner = ctx.owner
-            val found =
-              if (isSelfDenot(defDenot)) curOwner.enclosingClass.thisType
-              else curOwner.thisType.select(name, defDenot)
-            if (!(curOwner is Package) || isDefinedInCurrentUnit(defDenot))
-              return checkNewOrShadowed(found, definition) // no need to go further out, we found highest prec entry
-            else if (defDenot.symbol is Package)
-              return checkNewOrShadowed(previous orElse found, packageClause)
-            else if (prevPrec < packageClause)
-              return findRef(found, packageClause, ctx)(outer)
+      /** Would import of kind `prec` be not shadowed by a nested higher-precedence definition? */
+      def isPossibleImport(prec: Int)(implicit ctx: Context) =
+        prevPrec < prec || prevPrec == prec && (prevCtx.scope eq ctx.scope)
+
+      @tailrec def loop(implicit ctx: Context): Type = {
+        if (ctx.scope == null) previous
+        else {
+          val outer = ctx.outer
+          var result: Type = NoType
+
+          // find definition
+          if ((ctx.scope ne outer.scope) || (ctx.owner ne outer.owner)) {
+            val defDenot = ctx.denotNamed(name)
+            if (qualifies(defDenot)) {
+              val curOwner = ctx.owner
+              val found =
+                if (isSelfDenot(defDenot)) curOwner.enclosingClass.thisType
+                else curOwner.thisType.select(name, defDenot)
+              if (!(curOwner is Package) || isDefinedInCurrentUnit(defDenot))
+                result = checkNewOrShadowed(found, definition) // no need to go further out, we found highest prec entry
+              else {
+                if (ctx.scala2Mode && !foundUnderScala2.exists)
+                  foundUnderScala2 = checkNewOrShadowed(found, definition, scala2pkg = true)
+                if (defDenot.symbol is Package)
+                  result = checkNewOrShadowed(previous orElse found, packageClause)
+                else if (prevPrec < packageClause)
+                  result = findRef(found, packageClause, ctx)(outer)
+              }
+            }
           }
-        }
-        val curImport = ctx.importInfo
-        if (ctx.owner.is(Package) && curImport != null && curImport.isRootImport && previous.exists)
-          return previous // no more conflicts possible in this case
-        // would import of kind `prec` be not shadowed by a nested higher-precedence definition?
-        def isPossibleImport(prec: Int) =
-          prevPrec < prec || prevPrec == prec && (prevCtx.scope eq ctx.scope)
-        if (isPossibleImport(namedImport) && (curImport ne outer.importInfo) && !curImport.sym.isCompleting) {
-          val namedImp = namedImportRef(curImport.site, curImport.selectors)
-          if (namedImp.exists)
-            return findRef(checkNewOrShadowed(namedImp, namedImport), namedImport, ctx)(outer)
-          if (isPossibleImport(wildImport)) {
-            val wildImp = wildImportRef(curImport)
-            if (wildImp.exists)
-              return findRef(checkNewOrShadowed(wildImp, wildImport), wildImport, ctx)(outer)
+
+          if (result.exists) result
+          else {  // find import
+            val curImport = ctx.importInfo
+            if (ctx.owner.is(Package) && curImport != null && curImport.isRootImport && previous.exists)
+              previous // no more conflicts possible in this case
+            else if (isPossibleImport(namedImport) && (curImport ne outer.importInfo) && !curImport.sym.isCompleting) {
+              val namedImp = namedImportRef(curImport.site, curImport.selectors)
+              if (namedImp.exists)
+                findRef(checkNewOrShadowed(namedImp, namedImport), namedImport, ctx)(outer)
+              else if (isPossibleImport(wildImport)) {
+                val wildImp = wildImportRef(curImport)
+                if (wildImp.exists)
+                  findRef(checkNewOrShadowed(wildImp, wildImport), wildImport, ctx)(outer)
+                else loop(outer)
+              }
+              else loop(outer)
+            }
+            else loop(outer)
           }
         }
-        findRef(previous, prevPrec, prevCtx)(outer)
       }
+
+      loop
     }
 
     // begin typedIdent
@@ -264,12 +294,28 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
         return typed(desugar.patternVar(tree), pt)
     }
 
-    val saved = importedFromRoot
-    importedFromRoot = Set.empty
 
-    val rawType =
-      try findRef(NoType, BindingPrec.nothingBound, NoContext)
-      finally importedFromRoot = saved
+    val rawType = {
+      val saved1 = importedFromRoot
+      val saved2 = foundUnderScala2
+      importedFromRoot = Set.empty
+      foundUnderScala2 = NoType
+      try {
+        var found = findRef(NoType, BindingPrec.nothingBound, NoContext)
+        if (foundUnderScala2.exists && !(foundUnderScala2 =:= found)) {
+          ctx.migrationWarning(
+            ex"""Name resolution will change.
+              | currently selected                     : $foundUnderScala2
+              | in the future, without -language:Scala2: $found""", tree.pos)
+          found = foundUnderScala2
+        }
+        found
+      }
+      finally {
+      	importedFromRoot = saved1
+      	foundUnderScala2 = saved2
+      }
+    }
 
     val ownType =
       if (rawType.exists)

tests/pending/hkt/compiler.error renamed to tests/disabled/not-representable/hkt/compiler.error

@@ -1,3 +1,6 @@
+// This one is unavoidable. Dotty does not allow several overloaded
+// parameterless methods, so it picks the one in the subclass.
+
 import scala.language.higherKinds
 // Minimal reproduction for:
 // scala.collection.mutable.ArrayStack.empty[Int]

tests/pending/hkt/hkt.scala renamed to tests/disabled/not-representable/hkt/hkt.scala

@@ -0,0 +1,10 @@
+// This one should be rejected according to spec. The import takes precedence
+// over the type in the same package because the typeis declared in a
+// different compilation unit. scalac does not conform to spec here.
+package naming.resolution
+
+import java.nio.file._ // Imports `Files`
+
+object Resolution {
+  def gimmeFiles: Files = Files.list(Paths.get("."))
+}

tests/pending/import-rewrite/compiler.error

@@ -0,0 +1,5 @@
+package naming
+
+package object resolution {
+  type Files = java.util.stream.Stream[java.nio.file.Path]
+}