Warning about changed precedence in imports when migrating to 2.13

lrytz · lrytz · commit 4ee4a0294245 · 2022-08-15T10:15:26.000+02:00
diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -1246,6 +1246,14 @@ trait Contexts { self: Analyzer =>
       def lookupImport(imp: ImportInfo, requireExplicit: Boolean) =
         thisContext.importedAccessibleSymbol(imp, name, requireExplicit, record = true) filter qualifies
 
+      var importLookupFor213MigrationWarning = false
+      def depthOk213 = {
+        settings.isScala213 && !thisContext.unit.isJava && !cx(ContextMode.InPackageClauseName) && defSym.exists && isPackageOwnedInDifferentUnit(defSym) && {
+          importLookupFor213MigrationWarning = true
+          true
+        }
+      }
+
       // Java: A single-type-import declaration d in a compilation unit c of package p
       // that imports a type named n shadows, throughout c, the declarations of:
       //
@@ -1259,17 +1267,29 @@ trait Contexts { self: Analyzer =>
       //     package clause in the same compilation unit where the definition occurs have
       //     highest precedence.
       //  2) Explicit imports have next highest precedence.
-      def depthOk(imp: ImportInfo) = (
-           imp.depth > symbolDepth
-        || (thisContext.unit.isJava && imp.isExplicitImport(name) && imp.depth == symbolDepth)
-      )
+      def depthOk(imp: ImportInfo) = {
+        imp.depth > symbolDepth ||
+          (thisContext.unit.isJava && imp.isExplicitImport(name) && imp.depth == symbolDepth) ||
+          depthOk213
+      }
 
       while (!impSym.exists && importCursor.imp1Exists && depthOk(importCursor.imp1)) {
         impSym = lookupImport(imp1, requireExplicit = false)
         if (!impSym.exists)
           importCursor.advanceImp1Imp2()
       }
 
+      if (impSym.exists && importLookupFor213MigrationWarning) {
+        if (impSym != defSym && imp1.depth >= symbolDepth && !imp1.isExplicitImport(name)) {
+          val msg =
+            s"""This wildcard import imports ${impSym.fullName}, which is shadowed by ${defSym.fullName}.
+               |This is not according to the language specification and has changed in Scala 2.13, where ${impSym.fullName} takes precedence.
+               |To keep the same meaning in 2.12 and 2.13, un-import ${name} by adding `$name => _` to the import list.""".stripMargin
+          runReporting.warning(imp1.pos, msg, WarningCategory.Other, "")
+        }
+        impSym = NoSymbol
+      }
+
       if (defSym.exists && impSym.exists) {
         // imported symbols take precedence over package-owned symbols in different compilation units.
         if (isPackageOwnedInDifferentUnit(defSym))
@@ -1706,6 +1726,8 @@ object ContextMode {
   /** Were default arguments used? */
   final val DiagUsedDefaults: ContextMode         = 1 << 18
 
+  final val InPackageClauseName: ContextMode      = 1 << 19
+
   /** TODO: The "sticky modes" are EXPRmode, PATTERNmode, TYPEmode.
    *  To mimic the sticky mode behavior, when captain stickyfingers
    *  comes around we need to propagate those modes but forget the other
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -5411,7 +5411,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
 
       def typedPackageDef(pdef0: PackageDef) = {
         val pdef = treeCopy.PackageDef(pdef0, pdef0.pid, pluginsEnterStats(this, pdef0.stats))
-        val pid1 = typedQualifier(pdef.pid).asInstanceOf[RefTree]
+        val pid1 = context.withMode(ContextMode.InPackageClauseName)(typedQualifier(pdef.pid).asInstanceOf[RefTree])
         assert(sym.moduleClass ne NoSymbol, sym)
         val stats1 = newTyper(context.make(tree, sym.moduleClass, sym.info.decls))
                   .typedStats(pdef.stats, NoSymbol)
diff --git a/test/files/neg/warn-shadowing-import-213.check b/test/files/neg/warn-shadowing-import-213.check
@@ -0,0 +1,8 @@
+Test1.scala:4: warning: This wildcard import imports a.O, which is shadowed by b.O.
+This is not according to the language specification and has changed in Scala 2.13, where a.O takes precedence.
+To keep the same meaning in 2.12 and 2.13, un-import O by adding `O => _` to the import list.
+import a._
+       ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/warn-shadowing-import-213/A.scala b/test/files/neg/warn-shadowing-import-213/A.scala
@@ -0,0 +1,3 @@
+package a
+
+object O { def a = 0 }
diff --git a/test/files/neg/warn-shadowing-import-213/B.scala b/test/files/neg/warn-shadowing-import-213/B.scala
@@ -0,0 +1,3 @@
+package b
+
+object O { def b = 0 }
diff --git a/test/files/neg/warn-shadowing-import-213/Test1.scala b/test/files/neg/warn-shadowing-import-213/Test1.scala
@@ -0,0 +1,8 @@
+// scalac: -Xsource:2.13 -Werror
+
+package b
+import a._
+
+class C {
+  def t = O.b
+}
diff --git a/test/files/neg/warn-shadowing-import-213/Test2.scala b/test/files/neg/warn-shadowing-import-213/Test2.scala
@@ -0,0 +1,8 @@
+// scalac: -Xsource:2.13 -Werror
+
+package b
+import b._ // no warning, imported b.O is the same as package member b.O
+
+class D {
+  def t = O.b
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+package a`
	`2`	`+`
	`3`	`+object O { def a = 0 }`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+package b`
	`2`	`+`
	`3`	`+object O { def b = 0 }`