Fix 2403: Tweak places where we look for new definition

Given a sequence of nested scope (inner to outer), typedIdent has to
decode when to look for definitions. Originally his was the lawas packagst
scope before the owner changes. A recent fix changed it to the first
scope after the owner changed. But that is wrong for package scopes
because for them we first have to analyze any neted imports before
considering the package scope.

This commit fixes the problem by treating package scopes specially.
Also, the ambiguous import message was improved.

Author: odersky

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -1222,17 +1222,24 @@ object messages {
   case class AmbiguousImport(name: Names.Name, newPrec: Int, prevPrec: Int, prevCtx: Context)(implicit ctx: Context)
     extends Message(AmbiguousImportID) {
 
-    import typer.Typer.BindingPrec._
+    import typer.Typer.BindingPrec
 
     /** A string which explains how something was bound; Depending on `prec` this is either
       *      imported by <tree>
       *  or  defined in <symbol>
       */
-    private def bindingString(prec: Int, whereFound: Context, qualifier: String = "") =
-      if (isImportPrec(prec)) {
-        ex"""imported$qualifier by ${hl"${whereFound.importInfo}"}"""
+    private def bindingString(prec: Int, whereFound: Context, qualifier: String = "") = {
+      val howVisible = prec match {
+        case BindingPrec.definition => "defined"
+        case BindingPrec.namedImport => "imported by name"
+        case BindingPrec.wildImport => "imported"
+        case BindingPrec.packageClause => "found"
+      }
+      if (BindingPrec.isImportPrec(prec)) {
+        ex"""$howVisible$qualifier by ${hl"${whereFound.importInfo}"}"""
       } else
-        ex"""defined$qualifier in ${hl"${whereFound.owner.toString}"}"""
+        ex"""$howVisible$qualifier in ${hl"${whereFound.owner}"}"""
+    }
 
 
     val msg =

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

@@ -244,11 +244,21 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
         else {
           var result: Type = NoType
 
-          // find definition
-          if ((lastCtx eq ctx) || (ctx.scope ne lastCtx.scope) || (ctx.owner ne lastCtx.owner)) {
+          val curOwner = ctx.owner
+
+          // Can this scope contain new definitions? This is usually the first
+          // context where either the scope or the owner changes wrt the
+          // context immediately nested in it. But for package contexts, it's
+          // the opposite: the last context before the package changes. This distinction
+          // is made so that top-level imports following a package clause are
+          // logically nested in that package clause.
+          val isNewDefScope =
+            if (curOwner is Package) curOwner ne ctx.outer.owner
+            else (ctx.scope ne lastCtx.scope) || (curOwner ne lastCtx.owner)
+
+          if (isNewDefScope) {
             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)
@@ -297,7 +307,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
       }
 
       // begin findRef
-      loop(ctx)(ctx)
+      loop(NoContext)(ctx)
     }
 
     // begin typedIdent

tests/neg/i1641.scala

@@ -2,7 +2,7 @@ package bar { object bippy extends (Double => String) { def apply(x: Double): St
 package object println { def bippy(x: Int, y: Int, z: Int) = "(Int, Int, Int)" }
 object Test {
   def main(args: Array[String]): Unit = {
-    println(bar.bippy(5.5)) // error
+    println(bar.bippy(5.5))
     println(bar.bippy(1, 2, 3)) // error
   }
 }