Skip to content

Commit 2874097

Browse files
committed
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.
1 parent 71b0aea commit 2874097

File tree

7 files changed

+82
-39
lines changed

7 files changed

+82
-39
lines changed

Diff for: compiler/src/dotty/tools/dotc/typer/Typer.scala

+42-39
Original file line numberDiff line numberDiff line change
@@ -238,38 +238,40 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
238238
!owner.isEmptyPackage || ctx.owner.enclosingPackageClass.isEmptyPackage
239239
}
240240

241+
import BindingPrec.*
242+
type Result = (Type, BindingPrec)
243+
val NoResult = (NoType, NothingBound)
244+
241245
/** Find the denotation of enclosing `name` in given context `ctx`.
242-
* @param previous A denotation that was found in a more deeply nested scope,
243-
* or else `NoDenotation` if nothing was found yet.
244-
* @param prevPrec The binding precedence of the previous denotation,
245-
* or else `nothingBound` if nothing was found yet.
246+
* @param prevResult A type that was found in a more deeply nested scope,
247+
* and its precedence, or NoResult if nothing was found yet.
246248
* @param prevCtx The context of the previous denotation,
247249
* or else `NoContext` if nothing was found yet.
248250
*/
249-
def findRefRecur(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type = {
250-
import BindingPrec.*
251+
def findRefRecur(prevResult: Result, prevCtx: Context)(using Context): Result = {
252+
val (previous, prevPrec) = prevResult
251253

252254
/** Check that any previously found result from an inner context
253255
* does properly shadow the new one from an outer context.
254-
* @param found The newly found result
255-
* @param newPrec Its precedence
256+
* @param newResult The newly found type and its precedence.
256257
* @param scala2pkg Special mode where we check members of the same package, but defined
257258
* in different compilation units under Scala2. If set, and the
258259
* previous and new contexts do not have the same scope, we select
259260
* the previous (inner) definition. This models what scalac does.
260261
*/
261-
def checkNewOrShadowed(found: Type, newPrec: BindingPrec, scala2pkg: Boolean = false)(using Context): Type =
262+
def checkNewOrShadowed(newResult: Result, scala2pkg: Boolean = false)(using Context): Result =
263+
val (found, newPrec) = newResult
262264
if !previous.exists || TypeComparer.isSameRef(previous, found) then
263-
found
265+
newResult
264266
else if (prevCtx.scope eq ctx.scope) && newPrec.beats(prevPrec) then
265267
// special cases: definitions beat imports, and named imports beat
266268
// wildcard imports, provided both are in contexts with same scope
267-
found
269+
newResult
268270
else
269271
if !scala2pkg && !previous.isError && !found.isError then
270272
fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx,
271273
isExtension = previous.termSymbol.is(ExtensionMethod) && found.termSymbol.is(ExtensionMethod)))
272-
previous
274+
prevResult
273275

274276
/** Assemble and check alternatives to an imported reference. This implies:
275277
* - If we expand an extension method (i.e. altImports != null),
@@ -282,12 +284,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
282284
* shadowed. This order of checking is necessary since an outer package-level
283285
* definition might trump two conflicting inner imports, so no error should be
284286
* issued in that case. See i7876.scala.
285-
* @param previous the previously found reference (which is an import)
286-
* @param prevPrec the precedence of the reference (either NamedImport or WildImport)
287+
* @param prevResult the previously found reference (which is an import), and
288+
* the precedence of the reference (either NamedImport or WildImport)
287289
* @param prevCtx the context in which the reference was found
288290
* @param using_Context the outer context of `precCtx`
289291
*/
290-
def checkImportAlternatives(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
292+
def checkImportAlternatives(prevResult: Result, prevCtx: Context)(using Context): Result =
293+
val (previous, prevPrec) = prevResult
291294

292295
def addAltImport(altImp: TermRef) =
293296
if !TypeComparer.isSameRef(previous, altImp)
@@ -302,20 +305,20 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
302305
if prevPrec == WildImport then
303306
// Discard all previously found references and continue with `altImp`
304307
altImports.clear()
305-
checkImportAlternatives(altImp, NamedImport, ctx)(using ctx.outer)
308+
checkImportAlternatives((altImp, NamedImport), ctx)(using ctx.outer)
306309
else
307310
addAltImport(altImp)
308-
checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
311+
checkImportAlternatives(prevResult, prevCtx)(using ctx.outer)
309312
case _ =>
310313
if prevPrec == WildImport then
311314
wildImportRef(curImport) match
312315
case altImp: TermRef => addAltImport(altImp)
313316
case _ =>
314-
checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
317+
checkImportAlternatives(prevResult, prevCtx)(using ctx.outer)
315318
else
316-
val found = findRefRecur(previous, prevPrec, prevCtx)
317-
if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
318-
else found
319+
val foundResult = findRefRecur(prevResult, prevCtx)
320+
if foundResult._1 eq previous then checkNewOrShadowed(foundResult)(using prevCtx)
321+
else foundResult
319322
end checkImportAlternatives
320323

321324
def selection(imp: ImportInfo, name: Name, checkBounds: Boolean): Type =
@@ -405,10 +408,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
405408
!noImports &&
406409
(prevPrec.ordinal < prec.ordinal || prevPrec == prec && (prevCtx.scope eq ctx.scope))
407410

408-
@tailrec def loop(lastCtx: Context)(using Context): Type =
409-
if (ctx.scope eq EmptyScope) previous
411+
@tailrec def loop(lastCtx: Context)(using Context): Result =
412+
if (ctx.scope eq EmptyScope) prevResult
410413
else {
411-
var result: Type = NoType
414+
var result: Result = NoResult
412415
val curOwner = ctx.owner
413416

414417
/** Is curOwner a package object that should be skipped?
@@ -507,37 +510,36 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
507510
effectiveOwner.thisType.select(name, defDenot)
508511
}
509512
if !curOwner.is(Package) || isDefinedInCurrentUnit(defDenot) then
510-
result = checkNewOrShadowed(found, Definition) // no need to go further out, we found highest prec entry
513+
result = checkNewOrShadowed((found, Definition)) // no need to go further out, we found highest prec entry
511514
found match
512515
case found: NamedType
513516
if curOwner.isClass && isInherited(found.denot) && !ctx.compilationUnit.isJava =>
514517
checkNoOuterDefs(found.denot, ctx, ctx)
515518
case _ =>
516519
else
517520
if migrateTo3 && !foundUnderScala2.exists then
518-
foundUnderScala2 = checkNewOrShadowed(found, Definition, scala2pkg = true)
521+
foundUnderScala2 = checkNewOrShadowed((found, Definition), scala2pkg = true)._1
519522
if (defDenot.symbol.is(Package))
520-
result = checkNewOrShadowed(previous orElse found, PackageClause)
523+
result = checkNewOrShadowed((previous orElse found, PackageClause))
521524
else if (prevPrec.ordinal < PackageClause.ordinal)
522-
result = findRefRecur(found, PackageClause, ctx)(using ctx.outer)
525+
result = findRefRecur((found, PackageClause), ctx)(using ctx.outer)
523526
}
524527

525-
if result.exists then result
528+
if result._1.exists then result
526529
else { // find import
527530
val outer = ctx.outer
528531
val curImport = ctx.importInfo
529-
def updateUnimported() =
530-
if (curImport.nn.unimported ne NoSymbol) unimported += curImport.nn.unimported
531532
if (curOwner.is(Package) && curImport != null && curImport.isRootImport && previous.exists)
532-
previous // no more conflicts possible in this case
533-
else if (isPossibleImport(NamedImport) && (curImport nen outer.importInfo)) {
534-
val namedImp = namedImportRef(curImport.uncheckedNN)
533+
prevResult // no more conflicts possible in this case
534+
else if (isPossibleImport(NamedImport) && curImport != null && (curImport ne outer.importInfo)) {
535+
def updateUnimported() = if curImport.unimported ne NoSymbol then unimported += curImport.unimported
536+
val namedImp = namedImportRef(curImport)
535537
if (namedImp.exists)
536-
checkImportAlternatives(namedImp, NamedImport, ctx)(using outer)
537-
else if (isPossibleImport(WildImport) && !curImport.nn.importSym.isCompleting) {
538-
val wildImp = wildImportRef(curImport.uncheckedNN)
538+
checkImportAlternatives((namedImp, NamedImport), ctx)(using outer)
539+
else if (isPossibleImport(WildImport) && !curImport.importSym.isCompleting) {
540+
val wildImp = wildImportRef(curImport)
539541
if (wildImp.exists)
540-
checkImportAlternatives(wildImp, WildImport, ctx)(using outer)
542+
checkImportAlternatives((wildImp, WildImport), ctx)(using outer)
541543
else {
542544
updateUnimported()
543545
loop(ctx)(using outer)
@@ -556,7 +558,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
556558
loop(NoContext)
557559
}
558560

559-
findRefRecur(NoType, BindingPrec.NothingBound, NoContext)
561+
val (foundRef, foundPrec) = findRefRecur(NoResult, NoContext)
562+
foundRef
560563
}
561564

562565
/** If `tree`'s type is a `TermRef` identified by flow typing to be non-null, then

Diff for: tests/pos/i18529/JCode1.java

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package bug.code;
2+
3+
import bug.util.List;
4+
import bug.util.*;
5+
import java.util.*;
6+
7+
public class JCode1 {
8+
public void m1(List<Integer> xs) { return; }
9+
}

Diff for: tests/pos/i18529/JCode2.java

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package bug.code;
2+
3+
import bug.util.*;
4+
import bug.util.List;
5+
import java.util.*;
6+
7+
public class JCode2 {
8+
public void m1(List<Integer> xs) { return; }
9+
}

Diff for: tests/pos/i18529/List.java

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package bug.util;
2+
3+
public final class List<E> {}

Diff for: tests/pos/i18529/SCode1.scala

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package bug.code
2+
3+
import bug.util.List
4+
import bug.util.*
5+
import java.util.*
6+
7+
class SCode1 {
8+
def work(xs: List[Int]): Unit = {}
9+
}

Diff for: tests/pos/i18529/SCode2.scala

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package bug.code
2+
3+
import bug.util.*
4+
import bug.util.List
5+
import java.util.*
6+
7+
class SCode2 {
8+
def work(xs: List[Int]): Unit = {}
9+
}

Diff for: tests/pos/i18529/Test.scala

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
class Test

0 commit comments

Comments
 (0)