Resolve name when named imp is behind wild imps

When a named import (such as `import bug.util.List`) is defined before
two clashing wildcard imports (`import bug.util.*; import java.util.*`)
the name "List" should resolve to it, rather than a resolution error
being emitted.

This was due to the fact that `findRefRecur` didn't return the
precedence at which it found that import, `checkImportAlternatives` used
the `prevPrec` to `checkNewOrShadowed`.  Now we check against the entire
`foundResult`, allowing an early named import to be picked over later
wildcard imports.

Author: dwijnand

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

@@ -205,57 +205,54 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         !owner.isEmptyPackage || ctx.owner.enclosingPackageClass.isEmptyPackage
       }
 
+    import BindingPrec.*
+    type Result = (Type, BindingPrec)
+    val NoResult = (NoType, NothingBound)
+
     /** Find the denotation of enclosing `name` in given context `ctx`.
-     *  @param previous    A denotation that was found in a more deeply nested scope,
-     *                     or else `NoDenotation` if nothing was found yet.
-     *  @param prevPrec    The binding precedence of the previous denotation,
-     *                     or else `nothingBound` if nothing was found yet.
+     *  @param prevResult  A type that was found in a more deeply nested scope,
+     *                     and its precedence, or NoResult if nothing was found yet.
      *  @param prevCtx     The context of the previous denotation,
      *                     or else `NoContext` if nothing was found yet.
      */
-    def findRefRecur(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type = {
-      import BindingPrec.*
+    def findRefRecur(prevResult: Result, prevCtx: Context)(using Context): Result = {
+      val (previous, prevPrec) = prevResult
 
       /** 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 newResult The newly found type and 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: BindingPrec, scala2pkg: Boolean = false)(using Context): Type =
+      def checkNewOrShadowed(newResult: Result, scala2pkg: Boolean = false)(using Context): Result =
+        val (found, newPrec) = newResult
         if !previous.exists || TypeComparer.isSameRef(previous, found) then
            found
         else if (prevCtx.scope eq ctx.scope)
                 && (newPrec == Definition || newPrec == NamedImport && prevPrec == WildImport)
         then
           // special cases: definitions beat imports, and named imports beat
           // wildcard imports, provided both are in contexts with same scope
-          found
+          newResult
         else
           if !scala2pkg && !previous.isError && !found.isError then
             fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx))
           previous
-
-      /** Assemble and check alternatives to an imported reference. This implies:
-       *   - If we expand an extension method (i.e. altImports != null),
-       *     search imports on the same level for other possible resolutions of `name`.
-       *     The result and altImports together then contain all possible imported
-       *     references of the highest possible precedence, where `NamedImport` beats
        *     `WildImport`.
        *   - Find a posssibly shadowing reference in an outer context.
        *     If the result is the same as `previous`, check that it is new or
        *     shadowed. This order of checking is necessary since an outer package-level
        *     definition might trump two conflicting inner imports, so no error should be
        *     issued in that case. See i7876.scala.
-       *  @param previous   the previously found reference (which is an import)
-       *  @param prevPrec   the precedence of the reference (either NamedImport or WildImport)
+       *  @param prevResult the previously found reference (which is an import), and
+       *                    the precedence of the reference (either NamedImport or WildImport)
        *  @param prevCtx    the context in which the reference was found
        *  @param using_Context the outer context of `precCtx`
        */
-      def checkImportAlternatives(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
+      def checkImportAlternatives(prevResult: Result, prevCtx: Context)(using Context): Result =
+        val (previous, prevPrec) = prevResult
 
         def addAltImport(altImp: TermRef) =
           if !TypeComparer.isSameRef(previous, altImp)
@@ -270,20 +267,20 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
               if prevPrec == WildImport then
                 // Discard all previously found references and continue with `altImp`
                 altImports.clear()
-                checkImportAlternatives(altImp, NamedImport, ctx)(using ctx.outer)
+                checkImportAlternatives((altImp, NamedImport), ctx)(using ctx.outer)
               else
                 addAltImport(altImp)
-                checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
+                checkImportAlternatives(prevResult, prevCtx)(using ctx.outer)
             case _ =>
               if prevPrec == WildImport then
                 wildImportRef(curImport) match
                   case altImp: TermRef => addAltImport(altImp)
                   case _ =>
-              checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
+              checkImportAlternatives(prevResult, prevCtx)(using ctx.outer)
         else
-          val found = findRefRecur(previous, prevPrec, prevCtx)
-          if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
-          else found
+          val foundResult = findRefRecur(prevResult, prevCtx)
+          if foundResult._1 eq previous then checkNewOrShadowed(foundResult)(using prevCtx)
+          else foundResult
       end checkImportAlternatives
 
       def selection(imp: ImportInfo, name: Name, checkBounds: Boolean): Type =
@@ -370,10 +367,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         !noImports &&
         (prevPrec.ordinal < prec.ordinal || prevPrec == prec && (prevCtx.scope eq ctx.scope))
 
-      @tailrec def loop(lastCtx: Context)(using Context): Type =
-        if (ctx.scope eq EmptyScope) previous
+      @tailrec def loop(lastCtx: Context)(using Context): Result =
+        if (ctx.scope eq EmptyScope) prevResult
         else {
-          var result: Type = NoType
+          var result: Result = NoResult
           val curOwner = ctx.owner
 
           /** Is curOwner a package object that should be skipped?
@@ -472,37 +469,36 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
                   effectiveOwner.thisType.select(name, defDenot)
                 }
               if !curOwner.is(Package) || isDefinedInCurrentUnit(defDenot) then
-                result = checkNewOrShadowed(found, Definition) // no need to go further out, we found highest prec entry
+                result = checkNewOrShadowed((found, Definition)) // no need to go further out, we found highest prec entry
                 found match
                   case found: NamedType
                   if curOwner.isClass && isInherited(found.denot) && !ctx.compilationUnit.isJava =>
                     checkNoOuterDefs(found.denot, ctx, ctx)
                   case _ =>
               else
                 if migrateTo3 && !foundUnderScala2.exists then
-                  foundUnderScala2 = checkNewOrShadowed(found, Definition, scala2pkg = true)
+                  foundUnderScala2 = checkNewOrShadowed((found, Definition), scala2pkg = true)._1
                 if (defDenot.symbol.is(Package))
-                  result = checkNewOrShadowed(previous orElse found, PackageClause)
+                  result = checkNewOrShadowed((previous orElse found, PackageClause))
                 else if (prevPrec.ordinal < PackageClause.ordinal)
-                  result = findRefRecur(found, PackageClause, ctx)(using ctx.outer)
+                  result = findRefRecur((found, PackageClause), ctx)(using ctx.outer)
             }
 
-          if result.exists then result
+          if result._1.exists then result
           else {  // find import
             val outer = ctx.outer
             val curImport = ctx.importInfo
-            def updateUnimported() =
-              if (curImport.nn.unimported ne NoSymbol) unimported += curImport.nn.unimported
             if (curOwner.is(Package) && curImport != null && curImport.isRootImport && previous.exists)
-              previous // no more conflicts possible in this case
-            else if (isPossibleImport(NamedImport) && (curImport nen outer.importInfo)) {
-              val namedImp = namedImportRef(curImport.uncheckedNN)
+              prevResult // no more conflicts possible in this case
+            else if (isPossibleImport(NamedImport) && curImport != null && (curImport ne outer.importInfo)) {
+              def updateUnimported() = if curImport.unimported ne NoSymbol then unimported += curImport.unimported
+              val namedImp = namedImportRef(curImport)
               if (namedImp.exists)
-                checkImportAlternatives(namedImp, NamedImport, ctx)(using outer)
-              else if (isPossibleImport(WildImport) && !curImport.nn.importSym.isCompleting) {
-                val wildImp = wildImportRef(curImport.uncheckedNN)
+                checkImportAlternatives((namedImp, NamedImport), ctx)(using outer)
+              else if (isPossibleImport(WildImport) && !curImport.importSym.isCompleting) {
+                val wildImp = wildImportRef(curImport)
                 if (wildImp.exists)
-                  checkImportAlternatives(wildImp, WildImport, ctx)(using outer)
+                  checkImportAlternatives((wildImp, WildImport), ctx)(using outer)
                 else {
                   updateUnimported()
                   loop(ctx)(using outer)
@@ -521,7 +517,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
       loop(NoContext)
     }
 
-    findRefRecur(NoType, BindingPrec.NothingBound, NoContext)
+    val (foundRef, foundPrec) = findRefRecur(NoResult, NoContext)
+    foundRef
   }
 
   /** If `tree`'s type is a `TermRef` identified by flow typing to be non-null, then

tests/pos/i18529/JCode1.java

@@ -0,0 +1,9 @@
+package bug.code;
+
+import bug.util.List;
+import bug.util.*;
+import java.util.*;
+
+public class JCode1 {
+    public void m1(List<Integer> xs) { return; }
+}

tests/pos/i18529/JCode2.java

@@ -0,0 +1,9 @@
+package bug.code;
+
+import bug.util.*;
+import bug.util.List;
+import java.util.*;
+
+public class JCode2 {
+    public void m1(List<Integer> xs) { return; }
+}

tests/pos/i18529/List.java

@@ -0,0 +1,3 @@
+package bug.util;
+
+public final class List<E> {}

tests/pos/i18529/SCode1.scala

@@ -0,0 +1,9 @@
+package bug.code
+
+import bug.util.List
+import bug.util.*
+import java.util.*
+
+class SCode1 {
+  def work(xs: List[Int]): Unit = {}
+}

tests/pos/i18529/SCode2.scala

@@ -0,0 +1,9 @@
+package bug.code
+
+import bug.util.*
+import bug.util.List
+import java.util.*
+
+class SCode2 {
+  def work(xs: List[Int]): Unit = {}
+}

tests/pos/i18529/Test.scala

@@ -0,0 +1 @@
+class Test