scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.88k stars 1.06k forks source link

Import outside a package clause is not shadowed by an import inside the package clause #21405

Closed som-snytt closed 4 days ago

som-snytt commented 2 months ago

Compiler version

3.4.2

Minimized code

package p {
  class IO
}
import java.io.*
package q {
  import p.*
  class D extends IO
}

Output

-- [E049] Reference Error: masked.scala:8:18 ---------------------------------------------------------------------------
8 |  class D extends IO
  |                  ^^
  |                  Reference to IO is ambiguous.
  |                  It is both imported by import java.io._
  |                  and imported subsequently by import p._
  |
  | longer explanation available when compiling with `-explain`
1 error found

Expectation

p.IO shadows java.io.IO as in Scala 2.

This test requires JDK 23, but let me minimize properly, which also fails in the same way:

package r {
  class IO
}
package p {
  class IO
}
//import java.io.*
import r.*
package q {
  import p.*
  class D extends IO

  object Test extends App {
    println(new D().getClass.getSuperclass.getName)
  }
}

Note that -Yimports:java.io,p works correctly (without imports in source), as expected.

The issue is urgent where p is cats.effect.

mbovel commented 2 months ago

This issue was picked for the Scala Issue Spree of tomorrow, September 3rd. @dwijnand, @hamzaremmal and @nmcb will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

SethTisue commented 2 months ago

(just a snapshot of our thinking/investigating so far:)

Dale notes that it works as expected with object but fails with package. Entering object creates a new Scope but entering package does not, which we think is why this code (in Typer.scala, added in 2014 in 3affebe539b3461e7ea84411fe0a3943ccf35b4b):

      /** Would import of kind `prec` be not shadowed by a nested higher-precedence definition? */
      def isPossibleImport(prec: BindingPrec)(using Context) =
        !noImports &&
        (prevPrec.ordinal < prec.ordinal || prevPrec == prec && (prevCtx.scope eq ctx.scope))

gives a different answer in the two cases (because of the scope check), leading to the different overall answer.

Perhaps the logic in this method just needs updating? We're not sure yet.

SethTisue commented 2 months ago

I wondered if it was crucial that we have an import that's totally top level (not inside any package ... { } or after any un-braced package ...), since then it's like where even are we, the empty package, the root package? but Dale says that the bug still happens even if you precede the whole business with package z, say.

SethTisue commented 2 months ago

some relevant spec links: https://scala-lang.org/files/archive/spec/3.4/09-top-level-definitions.html#compilation-units , https://scala-lang.org/files/archive/spec/3.4/09-top-level-definitions.html#packagings , https://scala-lang.org/files/archive/spec/3.4/04-basic-definitions.html#non-given-imports

the bug can't be reproduced with just package p syntax because those can only come first in a compilation unit. you need the package p { ... } syntax (a "packaging", in spec-ese). that syntax isn't used much, so it seems a bit niche