Skip to content

Commit 1b65f36

Browse files
committed
Fix scala#17667 account for import order
- Import in the same scope, but after the usage are reported. - Add test suit i17667 - Fix test i15503a
1 parent 2d9eb1c commit 1b65f36

File tree

3 files changed

+43
-25
lines changed

3 files changed

+43
-25
lines changed

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

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import dotty.tools.dotc.core.Definitions
2525
import dotty.tools.dotc.core.NameKinds.WildcardParamName
2626
import dotty.tools.dotc.core.Symbols.Symbol
2727
import dotty.tools.dotc.core.StdNames.nme
28-
import scala.math.Ordering
2928

3029

3130
/**
@@ -87,15 +86,15 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
8786
val prefixes = LazyList.iterate(tree.typeOpt.normalizedPrefix)(_.normalizedPrefix).takeWhile(_ != NoType)
8887
.take(10) // Failsafe for the odd case if there was an infinite cycle
8988
for prefix <- prefixes do
90-
unusedDataApply(_.registerUsed(prefix.classSymbol, None))
91-
unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))
89+
unusedDataApply(_.registerUsed(prefix.classSymbol, None, Some(tree.srcPos)))
90+
unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name), Some(tree.srcPos)))
9291
else if tree.hasType then
93-
unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name)))
92+
unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name), Some(tree.srcPos)))
9493
else
9594
ctx
9695

9796
override def prepareForSelect(tree: tpd.Select)(using Context): Context =
98-
unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))
97+
unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name), Some(tree.srcPos)))
9998

10099
override def prepareForBlock(tree: tpd.Block)(using Context): Context =
101100
pushInBlockTemplatePackageDef(tree)
@@ -114,7 +113,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
114113
ud.registerDef(tree)
115114
if tree.name.mangledString.startsWith(nme.derived.mangledString + "$")
116115
&& tree.typeOpt != NoType then
117-
ud.registerUsed(tree.typeOpt.typeSymbol, None, true)
116+
ud.registerUsed(tree.typeOpt.typeSymbol, None, Some(tree.srcPos), true)
118117
ud.addIgnoredUsage(tree.symbol)
119118
}
120119

@@ -261,10 +260,10 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
261260
/** This is a type traverser which catch some special Types not traversed by the term traverser above */
262261
private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser:
263262
override def traverse(tp: Type): Unit =
264-
if tp.typeSymbol.exists then dt(_.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name)))
263+
if tp.typeSymbol.exists then dt(_.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name), None))
265264
tp match
266265
case AnnotatedType(_, annot) =>
267-
dt(_.registerUsed(annot.symbol, None))
266+
dt(_.registerUsed(annot.symbol, None, None))
268267
traverseChildren(tp)
269268
case _ =>
270269
traverseChildren(tp)
@@ -349,7 +348,7 @@ object CheckUnused:
349348
*
350349
* See the `isAccessibleAsIdent` extension method below in the file
351350
*/
352-
private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean)]())
351+
private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean, Option[SrcPos])]())
353352
private val usedInPosition = MutSet[(SrcPos, Name)]()
354353
/* unused import collected during traversal */
355354
private val unusedImport = MutSet[ImportSelector]()
@@ -392,14 +391,14 @@ object CheckUnused:
392391
* The optional name will be used to target the right import
393392
* as the same element can be imported with different renaming
394393
*/
395-
def registerUsed(sym: Symbol, name: Option[Name], isDerived: Boolean = false)(using Context): Unit =
394+
def registerUsed(sym: Symbol, name: Option[Name], srcPos: Option[SrcPos], isDerived: Boolean = false)(using Context): Unit =
396395
if !isConstructorOfSynth(sym) && !doNotRegister(sym) then
397396
if sym.isConstructor && sym.exists then
398-
registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class
397+
registerUsed(sym.owner, None, srcPos) // constructor are "implicitly" imported with the class
399398
else
400-
usedInScope.top += ((sym, sym.isAccessibleAsIdent, name, isDerived))
401-
usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name, isDerived))
402-
usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name, isDerived))
399+
usedInScope.top += ((sym, sym.isAccessibleAsIdent, name, isDerived, srcPos))
400+
usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name, isDerived, srcPos))
401+
usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name, isDerived, srcPos))
403402
if sym.sourcePos.exists then
404403
name.map(n => usedInPosition += ((sym.sourcePos, n)))
405404

@@ -461,21 +460,24 @@ object CheckUnused:
461460
val used = usedInScope.pop().toSet
462461
// used imports in this scope
463462
val imports = impInScope.pop()
464-
val kept = used.filterNot { (sym, isAccessible, optName, isDerived) =>
463+
val kept = used.filterNot { (sym, isAccessible, optName, isDerived, usageSymPosOpt) =>
465464
// keep the symbol for outer scope, if it matches **no** import
466465
// This is the first matching wildcard selector
467466
var selWildCard: Option[ImportSelector] = None
468467

469468
val matchedExplicitImport = imports.exists { imp =>
470-
sym.isInImport(imp, isAccessible, optName, isDerived) match
471-
case None => false
472-
case optSel@Some(sel) if sel.isWildcard =>
473-
if selWildCard.isEmpty then selWildCard = optSel
474-
// We keep wildcard symbol for the end as they have the least precedence
475-
false
476-
case Some(sel) =>
477-
unusedImport -= sel
478-
true
469+
if usageSymPosOpt.map(pos => pos.isStartBefore(imp.srcPos)).getOrElse(false) then
470+
false
471+
else
472+
sym.isInImport(imp, isAccessible, optName, isDerived) match
473+
case None => false
474+
case optSel@Some(sel) if sel.isWildcard =>
475+
if selWildCard.isEmpty then selWildCard = optSel
476+
// We keep wildcard symbol for the end as they have the least precedence
477+
false
478+
case Some(sel) =>
479+
unusedImport -= sel
480+
true
479481
}
480482
if !matchedExplicitImport && selWildCard.isDefined then
481483
unusedImport -= selWildCard.get
@@ -761,6 +763,13 @@ object CheckUnused:
761763
private def isWildcard: Boolean =
762764
thisName == StdNames.nme.WILDCARD || thisName.is(WildcardParamName)
763765

766+
extension (thiz: SrcPos)
767+
/** if `thiz` start position line (and then column) is before `that` */
768+
private def isStartBefore(that: SrcPos)(using Context) =
769+
(thiz.startPos.line < that.startPos.line) ||
770+
(thiz.startPos.line == that.startPos.line && thiz.startPos.column <= that.startPos.column)
771+
772+
764773
end UnusedData
765774

766775
private object UnusedData:

tests/neg-custom-args/fatal-warnings/i15503a.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ object InlineChecks:
6868
inline def getSet = Set(1)
6969

7070
object InlinedBar:
71-
import collection.mutable.Set // ok
71+
import collection.mutable.Set // error
7272
import collection.mutable.Map // error
7373
val a = InlineFoo.getSet
7474

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// scalac: -Wunused:imports
2+
3+
object MyImplicits:
4+
extension (a: Int) def print: Unit = println(s"Hello, I am $a")
5+
6+
import MyImplicits.print // ok
7+
object Foo:
8+
def printInt(a: Int): Unit = a.print
9+
import MyImplicits._ // error

0 commit comments

Comments
 (0)