Report unused masking import selectors

[Cherry-picked 8b73a79]

Author: som-snytt

compiler/src/dotty/tools/dotc/ast/untpd.scala

@@ -138,6 +138,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
       case Ident(rename: TermName) => rename
       case _ => name
 
+    /** It's a masking import if `!isWildcard`. */
     def isUnimport = rename == nme.WILDCARD
   }
 

compiler/src/dotty/tools/dotc/transform/CheckUnused.scala

@@ -25,6 +25,7 @@ import dotty.tools.dotc.util.chaining.*
 
 import java.util.IdentityHashMap
 
+import scala.annotation.*
 import scala.collection.mutable, mutable.{ArrayBuilder, ListBuffer, Stack}
 
 import CheckUnused.*
@@ -277,6 +278,8 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
              alt.symbol == sym
           || nm.isTypeName && alt.symbol.isAliasType && alt.info.dealias.typeSymbol == sym
         sameSym && alt.symbol.isAccessibleFrom(qtpe)
+      def hasAltMemberNamed(nm: Name) = qtpe.member(nm).hasAltWith(_.symbol.isAccessibleFrom(qtpe))
+
       def loop(sels: List[ImportSelector]): ImportSelector | Null = sels match
         case sel :: sels =>
           val matches =
@@ -293,9 +296,17 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
                 else
                   !sym.is(Given) // Normal wildcard, check that the symbol is not a given (but can be implicit)
               }
+            else if sel.isUnimport then
+              val masksMatchingMember =
+                   name != nme.NO_NAME
+                && sels.exists(x => x.isWildcard && !x.isGiven)
+                && !name.exists(_.toTermName != sel.name) // import a.b as _, b must match name
+                && (hasAltMemberNamed(sel.name) || hasAltMemberNamed(sel.name.toTypeName))
+              if masksMatchingMember then
+                refInfos.sels.put(sel, ()) // imprecise due to precedence but errs on the side of false negative
+              false
             else
-              // if there is an explicit name, it must match
-                 !name.exists(_.toTermName != sel.rename)
+                 !name.exists(_.toTermName != sel.rename) // if there is an explicit name, it must match
               && (prefix.eq(NoPrefix) || qtpe =:= prefix)
               && (hasAltMember(sel.name) || hasAltMember(sel.name.toTypeName))
           if matches then sel else loop(sels)
@@ -626,11 +637,11 @@ object CheckUnused:
               warnAt(pos)(UnusedSymbol.unsetPrivates)
 
     def checkImports() =
-      // TODO check for unused masking import
       import scala.jdk.CollectionConverters.given
       import Rewrites.ActionPatch
       type ImpSel = (Import, ImportSelector)
-      // true if used or might be used, to imply don't warn about it
+      def isUsed(sel: ImportSelector): Boolean = infos.sels.containsKey(sel)
+      @unused // avoid merge conflict
       def isUsable(imp: Import, sel: ImportSelector): Boolean =
         sel.isImportExclusion || infos.sels.containsKey(sel)
       def warnImport(warnable: ImpSel, actions: List[CodeAction] = Nil): Unit =
@@ -641,7 +652,7 @@ object CheckUnused:
         warnAt(sel.srcPos)(msg, origin)
 
       if !actionable then
-        for imp <- infos.imps.keySet.nn.asScala; sel <- imp.selectors if !isUsable(imp, sel) do
+        for imp <- infos.imps.keySet.nn.asScala; sel <- imp.selectors if !isUsed(sel) do
           warnImport(imp -> sel)
       else
         // If the rest of the line is blank, include it in the final edit position. (Delete trailing whitespace.)
@@ -696,7 +707,7 @@ object CheckUnused:
         while index < sortedImps.length do
           val nextImport = sortedImps.indexSatisfying(from = index + 1)(_.isPrimaryClause) // next import statement
           if sortedImps.indexSatisfying(from = index, until = nextImport): imp =>
-              imp.selectors.exists(!isUsable(imp, _)) // check if any selector in statement was unused
+              imp.selectors.exists(!isUsed(_)) // check if any selector in statement was unused
           < nextImport then
             // if no usable selectors in the import statement, delete it entirely.
             // if there is exactly one usable selector, then replace with just that selector (i.e., format it).
@@ -705,7 +716,7 @@ object CheckUnused:
             // Reminder that first clause span includes the keyword, so delete point-to-start instead.
             val existing = sortedImps.slice(index, nextImport)
             val (keeping, deleting) = existing.iterator.flatMap(imp => imp.selectors.map(imp -> _)).toList
-                                      .partition(isUsable(_, _))
+                                      .partition((imp, sel) => isUsed(sel))
             if keeping.isEmpty then
               val editPos = existing.head.srcPos.sourcePos.withSpan:
                 Span(start = existing.head.srcPos.span.start, end = existing.last.srcPos.span.end)
@@ -930,12 +941,11 @@ object CheckUnused:
     def boundTpe: Type = sel.bound match
       case untpd.TypedSplice(tree) => tree.tpe
       case _ => NoType
-    /** This is used to ignore exclusion imports of the form import `qual.member as _`
-     *  because `sel.isUnimport` is too broad for old style `import concurrent._`.
+    /** Is a "masking" import of the form import `qual.member as _`.
+     *  Both conditions must be checked.
      */
-    def isImportExclusion: Boolean = sel.renamed match
-      case untpd.Ident(nme.WILDCARD) => true
-      case _ => false
+    @unused // matchingSelector checks isWildcard first
+    def isImportExclusion: Boolean = sel.isUnimport && !sel.isWildcard
 
   extension (imp: Import)(using Context)
     /** Is it the first import clause in a statement? `a.x` in `import a.x, b.{y, z}` */

tests/warn/i15503a.scala

@@ -85,12 +85,12 @@ object InnerMostCheck:
     val a = Set(1)
 
 object IgnoreExclusion:
-  import collection.mutable.{Set => _} // OK
-  import collection.mutable.{Map => _} // OK
+  import collection.mutable.{Map => _, Set => _, *} // OK??
   import collection.mutable.{ListBuffer} // warn
   def check =
     val a = Set(1)
     val b = Map(1 -> 2)
+    def c = Seq(42)
 /**
   * Some given values for the test
   */

tests/warn/i23758.scala

@@ -0,0 +1,11 @@
+//> using options -Wunused:imports
+
+import scala.util.Try as _ // warn
+
+class Promise(greeting: String):
+  override def toString = greeting
+
+@main def test = println:
+  import scala.concurrent.{Promise as _, *}, ExecutionContext.Implicits.given
+  val promise = new Promise("world")
+  Future(s"hello, $promise")