Skip to content

Commit 48de883

Browse files
som-snytttgodzik
authored andcommitted
Report unused masking import selectors
1 parent 6c20869 commit 48de883

File tree

4 files changed

+37
-9
lines changed

4 files changed

+37
-9
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
138138
case Ident(rename: TermName) => rename
139139
case _ => name
140140

141+
/** It's a masking import if `!isWildcard`. */
141142
def isUnimport = rename == nme.WILDCARD
142143
}
143144

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import dotty.tools.dotc.util.chaining.*
2626

2727
import java.util.IdentityHashMap
2828

29+
import scala.annotation.*
2930
import scala.collection.mutable, mutable.{ArrayBuilder, ListBuffer, Stack}
3031

3132
import CheckUnused.*
@@ -288,6 +289,8 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
288289
alt.symbol == sym
289290
|| nm.isTypeName && alt.symbol.isAliasType && alt.info.dealias.typeSymbol == sym
290291
sameSym && alt.symbol.isAccessibleFrom(qtpe)
292+
def hasAltMemberNamed(nm: Name) = qtpe.member(nm).hasAltWith(_.symbol.isAccessibleFrom(qtpe))
293+
291294
def loop(sels: List[ImportSelector]): ImportSelector | Null = sels match
292295
case sel :: sels =>
293296
val matches =
@@ -304,9 +307,17 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
304307
else
305308
!sym.is(Given) // Normal wildcard, check that the symbol is not a given (but can be implicit)
306309
}
310+
else if sel.isUnimport then
311+
val masksMatchingMember =
312+
name != nme.NO_NAME
313+
&& sels.exists(x => x.isWildcard && !x.isGiven)
314+
&& !name.exists(_.toTermName != sel.name) // import a.b as _, b must match name
315+
&& (hasAltMemberNamed(sel.name) || hasAltMemberNamed(sel.name.toTypeName))
316+
if masksMatchingMember then
317+
refInfos.sels.put(sel, ()) // imprecise due to precedence but errs on the side of false negative
318+
false
307319
else
308-
// if there is an explicit name, it must match
309-
!name.exists(_.toTermName != sel.rename)
320+
!name.exists(_.toTermName != sel.rename) // if there is an explicit name, it must match
310321
&& (prefix.eq(NoPrefix) || qtpe =:= prefix)
311322
&& (hasAltMember(sel.name) || hasAltMember(sel.name.toTypeName))
312323
if matches then sel else loop(sels)
@@ -697,11 +708,11 @@ object CheckUnused:
697708
warnAt(pos)(UnusedSymbol.unsetPrivates)
698709

699710
def checkImports() =
700-
// TODO check for unused masking import
701711
import scala.jdk.CollectionConverters.given
702712
import Rewrites.ActionPatch
703713
type ImpSel = (Import, ImportSelector)
704-
// true if used or might be used, to imply don't warn about it
714+
def isUsed(sel: ImportSelector): Boolean = infos.sels.containsKey(sel)
715+
@unused // avoid merge conflict
705716
def isUsable(imp: Import, sel: ImportSelector): Boolean =
706717
sel.isImportExclusion || infos.sels.containsKey(sel)
707718
def warnImport(warnable: ImpSel, actions: List[CodeAction] = Nil): Unit =
@@ -712,7 +723,7 @@ object CheckUnused:
712723
warnAt(sel.srcPos)(msg, origin)
713724

714725
if !actionable then
715-
for imp <- infos.imps.keySet.nn.asScala; sel <- imp.selectors if !isUsable(imp, sel) do
726+
for imp <- infos.imps.keySet.nn.asScala; sel <- imp.selectors if !isUsed(sel) do
716727
warnImport(imp -> sel)
717728
else
718729
// If the rest of the line is blank, include it in the final edit position. (Delete trailing whitespace.)
@@ -767,7 +778,7 @@ object CheckUnused:
767778
while index < sortedImps.length do
768779
val nextImport = sortedImps.indexSatisfying(from = index + 1)(_.isPrimaryClause) // next import statement
769780
if sortedImps.indexSatisfying(from = index, until = nextImport): imp =>
770-
imp.selectors.exists(!isUsable(imp, _)) // check if any selector in statement was unused
781+
imp.selectors.exists(!isUsed(_)) // check if any selector in statement was unused
771782
< nextImport then
772783
// if no usable selectors in the import statement, delete it entirely.
773784
// if there is exactly one usable selector, then replace with just that selector (i.e., format it).
@@ -776,7 +787,7 @@ object CheckUnused:
776787
// Reminder that first clause span includes the keyword, so delete point-to-start instead.
777788
val existing = sortedImps.slice(index, nextImport)
778789
val (keeping, deleting) = existing.iterator.flatMap(imp => imp.selectors.map(imp -> _)).toList
779-
.partition(isUsable(_, _))
790+
.partition((imp, sel) => isUsed(sel))
780791
if keeping.isEmpty then
781792
val editPos = existing.head.srcPos.sourcePos.withSpan:
782793
Span(start = existing.head.srcPos.span.start, end = existing.last.srcPos.span.end)
@@ -1001,6 +1012,11 @@ object CheckUnused:
10011012
def boundTpe: Type = sel.bound match
10021013
case untpd.TypedSplice(tree) => tree.tpe
10031014
case _ => NoType
1015+
/** Is a "masking" import of the form import `qual.member as _`.
1016+
* Both conditions must be checked.
1017+
*/
1018+
@unused // matchingSelector checks isWildcard first
1019+
def isImportExclusion: Boolean = sel.isUnimport && !sel.isWildcard
10041020

10051021
extension (imp: Import)(using Context)
10061022
/** Is it the first import clause in a statement? `a.x` in `import a.x, b.{y, z}` */

tests/warn/i15503a.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,12 @@ object InnerMostCheck:
8585
val a = Set(1)
8686

8787
object IgnoreExclusion:
88-
import collection.mutable.{Set => _} // OK
89-
import collection.mutable.{Map => _} // OK
88+
import collection.mutable.{Map => _, Set => _, *} // OK??
9089
import collection.mutable.{ListBuffer} // warn
9190
def check =
9291
val a = Set(1)
9392
val b = Map(1 -> 2)
93+
def c = Seq(42)
9494
/**
9595
* Some given values for the test
9696
*/

tests/warn/i23758.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//> using options -Wunused:imports
2+
3+
import scala.util.Try as _ // warn
4+
5+
class Promise(greeting: String):
6+
override def toString = greeting
7+
8+
@main def test = println:
9+
import scala.concurrent.{Promise as _, *}, ExecutionContext.Implicits.given
10+
val promise = new Promise("world")
11+
Future(s"hello, $promise")

0 commit comments

Comments
 (0)