From 2874097fdda4827d6e597b6822f7f6a84126e180 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 31 Oct 2024 17:54:29 +0000 Subject: [PATCH] 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. --- .../src/dotty/tools/dotc/typer/Typer.scala | 81 ++++++++++--------- tests/pos/i18529/JCode1.java | 9 +++ tests/pos/i18529/JCode2.java | 9 +++ tests/pos/i18529/List.java | 3 + tests/pos/i18529/SCode1.scala | 9 +++ tests/pos/i18529/SCode2.scala | 9 +++ tests/pos/i18529/Test.scala | 1 + 7 files changed, 82 insertions(+), 39 deletions(-) create mode 100644 tests/pos/i18529/JCode1.java create mode 100644 tests/pos/i18529/JCode2.java create mode 100644 tests/pos/i18529/List.java create mode 100644 tests/pos/i18529/SCode1.scala create mode 100644 tests/pos/i18529/SCode2.scala create mode 100644 tests/pos/i18529/Test.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 817e7baf1c8c..33d26e3a562a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -238,38 +238,40 @@ 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 + newResult else if (prevCtx.scope eq ctx.scope) && newPrec.beats(prevPrec) 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, isExtension = previous.termSymbol.is(ExtensionMethod) && found.termSymbol.is(ExtensionMethod))) - previous + prevResult /** Assemble and check alternatives to an imported reference. This implies: * - If we expand an extension method (i.e. altImports != null), @@ -282,12 +284,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer * 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) @@ -302,20 +305,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 = @@ -405,10 +408,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? @@ -507,7 +510,7 @@ 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 => @@ -515,29 +518,28 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer 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) @@ -556,7 +558,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 diff --git a/tests/pos/i18529/JCode1.java b/tests/pos/i18529/JCode1.java new file mode 100644 index 000000000000..e1f12f852c00 --- /dev/null +++ b/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 xs) { return; } +} diff --git a/tests/pos/i18529/JCode2.java b/tests/pos/i18529/JCode2.java new file mode 100644 index 000000000000..2a1ec812852c --- /dev/null +++ b/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 xs) { return; } +} diff --git a/tests/pos/i18529/List.java b/tests/pos/i18529/List.java new file mode 100644 index 000000000000..caf3c0b8036b --- /dev/null +++ b/tests/pos/i18529/List.java @@ -0,0 +1,3 @@ +package bug.util; + +public final class List {} diff --git a/tests/pos/i18529/SCode1.scala b/tests/pos/i18529/SCode1.scala new file mode 100644 index 000000000000..b6796b1540c6 --- /dev/null +++ b/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 = {} +} diff --git a/tests/pos/i18529/SCode2.scala b/tests/pos/i18529/SCode2.scala new file mode 100644 index 000000000000..30fc7d0e6f91 --- /dev/null +++ b/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 = {} +} diff --git a/tests/pos/i18529/Test.scala b/tests/pos/i18529/Test.scala new file mode 100644 index 000000000000..be7795442a7a --- /dev/null +++ b/tests/pos/i18529/Test.scala @@ -0,0 +1 @@ +class Test