Skip to content

Commit a1c3a32

Browse files
committed
Respect prefix when checking if selector selects
1 parent 2d0e373 commit a1c3a32

File tree

3 files changed

+123
-89
lines changed

3 files changed

+123
-89
lines changed

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

Lines changed: 79 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import dotty.tools.dotc.core.Symbols.Symbol
3434
import dotty.tools.dotc.core.StdNames.nme
3535
import dotty.tools.dotc.util.Spans.Span
3636
import scala.math.Ordering
37-
37+
import scala.util.chaining.given
3838

3939
/**
4040
* A compiler phase that checks for unused imports or definitions
@@ -68,16 +68,14 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
6868
tree.getAttachment(_key).foreach(oldData =>
6969
data.unusedAggregate = oldData.unusedAggregate
7070
)
71-
val fresh = ctx.fresh.setProperty(_key, data)
72-
tree.putAttachment(_key, data)
73-
fresh
71+
ctx.fresh.setProperty(_key, data).tap(_ => tree.putAttachment(_key, data))
7472

7573
// ========== END + REPORTING ==========
7674

7775
override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree =
7876
unusedDataApply { ud =>
7977
ud.finishAggregation()
80-
if(phaseMode == PhaseMode.Report) then
78+
if phaseMode == PhaseMode.Report then
8179
ud.unusedAggregate.foreach(reportUnused)
8280
}
8381
tree
@@ -99,20 +97,21 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
9997
def loopOnNormalizedPrefixes(prefix: Type, depth: Int): Unit =
10098
// limit to 10 as failsafe for the odd case where there is an infinite cycle
10199
if depth < 10 && prefix.exists then
102-
ud.registerUsed(prefix.classSymbol, None)
100+
ud.registerUsed(prefix.classSymbol, None, prefix)
103101
loopOnNormalizedPrefixes(prefix.normalizedPrefix, depth + 1)
104102

105-
loopOnNormalizedPrefixes(tree.typeOpt.normalizedPrefix, depth = 0)
106-
ud.registerUsed(tree.symbol, Some(tree.name))
103+
val prefix = tree.typeOpt.normalizedPrefix
104+
loopOnNormalizedPrefixes(prefix, depth = 0)
105+
ud.registerUsed(tree.symbol, Some(tree.name), prefix)
107106
}
108107
else if tree.hasType then
109-
unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name)))
108+
unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.normalizedPrefix))
110109
else
111110
ctx
112111

113112
override def prepareForSelect(tree: tpd.Select)(using Context): Context =
114113
val name = tree.removeAttachment(OriginalName)
115-
unusedDataApply(_.registerUsed(tree.symbol, name, includeForImport = tree.qualifier.span.isSynthetic))
114+
unusedDataApply(_.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic))
116115

117116
override def prepareForBlock(tree: tpd.Block)(using Context): Context =
118117
pushInBlockTemplatePackageDef(tree)
@@ -125,12 +124,12 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
125124

126125
override def prepareForValDef(tree: tpd.ValDef)(using Context): Context =
127126
unusedDataApply{ud =>
128-
// do not register the ValDef generated for `object`
129127
traverseAnnotations(tree.symbol)
128+
// do not register the ValDef generated for `object`
130129
if !tree.symbol.is(Module) then
131130
ud.registerDef(tree)
132-
if tree.name.startsWith("derived$") && tree.typeOpt != NoType then
133-
ud.registerUsed(tree.typeOpt.typeSymbol, None, isDerived = true)
131+
if tree.name.startsWith("derived$") && tree.hasType then
132+
ud.registerUsed(tree.tpe.typeSymbol, None, tree.tpe.normalizedPrefix, isDerived = true)
134133
ud.addIgnoredUsage(tree.symbol)
135134
}
136135

@@ -278,10 +277,10 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
278277
/** This is a type traverser which catch some special Types not traversed by the term traverser above */
279278
private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser:
280279
override def traverse(tp: Type): Unit =
281-
if tp.typeSymbol.exists then dt(_.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name)))
280+
if tp.typeSymbol.exists then dt(_.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name), tp.normalizedPrefix))
282281
tp match
283282
case AnnotatedType(_, annot) =>
284-
dt(_.registerUsed(annot.symbol, None))
283+
dt(_.registerUsed(annot.symbol, None, annot.symbol.info.normalizedPrefix))
285284
traverseChildren(tp)
286285
case _ =>
287286
traverseChildren(tp)
@@ -293,25 +292,23 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
293292

294293
/** Do the actual reporting given the result of the anaylsis */
295294
private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit =
296-
res.warnings.toList.sortBy(_.pos.span.point)(using Ordering[Int]).foreach { s =>
297-
s match
298-
case UnusedSymbol(t, _, WarnTypes.Imports) =>
299-
report.warning(UnusedSymbolMessage.imports, t)
300-
case UnusedSymbol(t, _, WarnTypes.LocalDefs) =>
301-
report.warning(UnusedSymbolMessage.localDefs, t)
302-
case UnusedSymbol(t, _, WarnTypes.ExplicitParams) =>
303-
report.warning(UnusedSymbolMessage.explicitParams, t)
304-
case UnusedSymbol(t, _, WarnTypes.ImplicitParams) =>
305-
report.warning(UnusedSymbolMessage.implicitParams, t)
306-
case UnusedSymbol(t, _, WarnTypes.PrivateMembers) =>
307-
report.warning(UnusedSymbolMessage.privateMembers, t)
308-
case UnusedSymbol(t, _, WarnTypes.PatVars) =>
309-
report.warning(UnusedSymbolMessage.patVars, t)
310-
case UnusedSymbol(t, _, WarnTypes.UnsetLocals) =>
311-
report.warning("unset local variable, consider using an immutable val instead", t)
312-
case UnusedSymbol(t, _, WarnTypes.UnsetPrivates) =>
313-
report.warning("unset private variable, consider using an immutable val instead", t)
314-
}
295+
res.warnings.toArray.sortInPlaceBy(_.pos.span.point).foreach:
296+
case UnusedSymbol(t, _, WarnTypes.Imports) =>
297+
report.warning(UnusedSymbolMessage.imports, t)
298+
case UnusedSymbol(t, _, WarnTypes.LocalDefs) =>
299+
report.warning(UnusedSymbolMessage.localDefs, t)
300+
case UnusedSymbol(t, _, WarnTypes.ExplicitParams) =>
301+
report.warning(UnusedSymbolMessage.explicitParams, t)
302+
case UnusedSymbol(t, _, WarnTypes.ImplicitParams) =>
303+
report.warning(UnusedSymbolMessage.implicitParams, t)
304+
case UnusedSymbol(t, _, WarnTypes.PrivateMembers) =>
305+
report.warning(UnusedSymbolMessage.privateMembers, t)
306+
case UnusedSymbol(t, _, WarnTypes.PatVars) =>
307+
report.warning(UnusedSymbolMessage.patVars, t)
308+
case UnusedSymbol(t, _, WarnTypes.UnsetLocals) =>
309+
report.warning("unset local variable, consider using an immutable val instead", t)
310+
case UnusedSymbol(t, _, WarnTypes.UnsetPrivates) =>
311+
report.warning("unset private variable, consider using an immutable val instead", t)
315312

316313
end CheckUnused
317314

@@ -352,69 +349,58 @@ object CheckUnused:
352349
* - usage
353350
*/
354351
private class UnusedData:
355-
import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack, ListBuffer => MutList}
352+
import collection.mutable as mut, mut.Stack, mut.ListBuffer
356353
import UnusedData.*
357354

358355
/** The current scope during the tree traversal */
359-
val currScopeType: MutStack[ScopeType] = MutStack(ScopeType.Other)
356+
val currScopeType: Stack[ScopeType] = Stack(ScopeType.Other)
360357

361358
var unusedAggregate: Option[UnusedResult] = None
362359

363360
/* IMPORTS */
364-
private val impInScope = MutStack(MutList[ImportSelectorData]())
365-
/**
366-
* We store the symbol along with their accessibility without import.
367-
* Accessibility to their definition in outer context/scope
368-
*
369-
* See the `isAccessibleAsIdent` extension method below in the file
370-
*/
371-
private val usedInScope = MutStack(MutSet[(Symbol, Option[Name], Boolean)]())
372-
private val usedInPosition = MutMap.empty[Name, MutSet[Symbol]]
361+
private val impInScope = Stack(ListBuffer.empty[ImportSelectorData])
362+
private val usedInScope = Stack(mut.Set.empty[Usage])
363+
private val usedInPosition = mut.Map.empty[Name, mut.Set[Symbol]]
373364
/* unused import collected during traversal */
374-
private val unusedImport = MutList.empty[ImportSelectorData]
365+
private val unusedImport = ListBuffer.empty[ImportSelectorData]
375366

376367
/* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */
377-
private val localDefInScope = MutList.empty[tpd.MemberDef]
378-
private val privateDefInScope = MutList.empty[tpd.MemberDef]
379-
private val explicitParamInScope = MutList.empty[tpd.MemberDef]
380-
private val implicitParamInScope = MutList.empty[tpd.MemberDef]
381-
private val patVarsInScope = MutList.empty[tpd.Bind]
368+
private val localDefInScope = ListBuffer.empty[tpd.MemberDef]
369+
private val privateDefInScope = ListBuffer.empty[tpd.MemberDef]
370+
private val explicitParamInScope = ListBuffer.empty[tpd.MemberDef]
371+
private val implicitParamInScope = ListBuffer.empty[tpd.MemberDef]
372+
private val patVarsInScope = ListBuffer.empty[tpd.Bind]
382373

383374
/** All variables sets*/
384-
private val setVars = MutSet[Symbol]()
375+
private val setVars = mut.Set.empty[Symbol]
385376

386377
/** All used symbols */
387-
private val usedDef = MutSet[Symbol]()
378+
private val usedDef = mut.Set.empty[Symbol]
388379
/** Do not register as used */
389-
private val doNotRegister = MutSet[Symbol]()
380+
private val doNotRegister = mut.Set.empty[Symbol]
390381

391382
/** Trivial definitions, avoid registering params */
392-
private val trivialDefs = MutSet[Symbol]()
393-
394-
private val paramsToSkip = MutSet[Symbol]()
383+
private val trivialDefs = mut.Set.empty[Symbol]
395384

385+
private val paramsToSkip = mut.Set.empty[Symbol]
396386

397387
def finishAggregation(using Context)(): Unit =
398-
val unusedInThisStage = this.getUnused
399-
this.unusedAggregate match {
388+
unusedAggregate = unusedAggregate match
400389
case None =>
401-
this.unusedAggregate = Some(unusedInThisStage)
390+
Some(getUnused)
402391
case Some(prevUnused) =>
403-
val intersection = unusedInThisStage.warnings.intersect(prevUnused.warnings)
404-
this.unusedAggregate = Some(UnusedResult(intersection))
405-
}
406-
392+
Some(UnusedResult(getUnused.warnings.intersect(prevUnused.warnings)))
407393

408394
/**
409395
* Register a found (used) symbol along with its name
410396
*
411397
* The optional name will be used to target the right import
412398
* as the same element can be imported with different renaming
413399
*/
414-
def registerUsed(sym: Symbol, name: Option[Name], includeForImport: Boolean = true, isDerived: Boolean = false)(using Context): Unit =
400+
def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true, isDerived: Boolean = false)(using Context): Unit =
415401
if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then
416402
if sym.isConstructor then
417-
registerUsed(sym.owner, None, includeForImport) // constructor are "implicitly" imported with the class
403+
registerUsed(sym.owner, None, prefix, includeForImport) // constructor are "implicitly" imported with the class
418404
else
419405
// If the symbol is accessible in this scope without an import, do not register it for unused import analysis
420406
val includeForImport1 =
@@ -425,13 +411,13 @@ object CheckUnused:
425411
if sym.exists then
426412
usedDef += sym
427413
if includeForImport1 then
428-
usedInScope.top += ((sym, name, isDerived))
414+
usedInScope.top += Usage(sym, name, prefix, isDerived)
429415
addIfExists(sym)
430416
addIfExists(sym.companionModule)
431417
addIfExists(sym.companionClass)
432418
if sym.sourcePos.exists then
433419
for n <- name do
434-
usedInPosition.getOrElseUpdate(n, MutSet.empty) += sym
420+
usedInPosition.getOrElseUpdate(n, mut.Set.empty) += sym
435421

436422
/** Register a symbol that should be ignored */
437423
def addIgnoredUsage(sym: Symbol)(using Context): Unit =
@@ -455,12 +441,12 @@ object CheckUnused:
455441
val qualTpe = imp.expr.tpe
456442

457443
// Put wildcard imports at the end, because they have lower priority within one Import
458-
val reorderdSelectors =
444+
val reorderedSelectors =
459445
val (wildcardSels, nonWildcardSels) = imp.selectors.partition(_.isWildcard)
460446
nonWildcardSels ::: wildcardSels
461447

462448
val newDataInScope =
463-
for sel <- reorderdSelectors yield
449+
for sel <- reorderedSelectors yield
464450
val data = new ImportSelectorData(qualTpe, sel)
465451
if shouldSelectorBeReported(imp, sel) || isImportExclusion(sel) || isImportIgnored(imp, sel) then
466452
// Immediately mark the selector as used
@@ -492,8 +478,8 @@ object CheckUnused:
492478
def pushScope(newScopeType: ScopeType): Unit =
493479
// unused imports :
494480
currScopeType.push(newScopeType)
495-
impInScope.push(MutList())
496-
usedInScope.push(MutSet())
481+
impInScope.push(ListBuffer.empty)
482+
usedInScope.push(mut.Set.empty)
497483

498484
def registerSetVar(sym: Symbol): Unit =
499485
setVars += sym
@@ -509,18 +495,15 @@ object CheckUnused:
509495
val selDatas = impInScope.pop()
510496

511497
for usedInfo <- usedInfos do
512-
val (sym, optName, isDerived) = usedInfo
513-
val usedData = selDatas.find { selData =>
514-
sym.isInImport(selData, optName, isDerived)
515-
}
516-
usedData match
517-
case Some(data) =>
518-
data.markUsed()
498+
val Usage(sym, optName, prefix, isDerived) = usedInfo
499+
selDatas.find(sym.isInImport(_, optName, prefix, isDerived)) match
500+
case Some(sel) =>
501+
sel.markUsed()
519502
case None =>
520503
// Propagate the symbol one level up
521504
if usedInScope.nonEmpty then
522505
usedInScope.top += usedInfo
523-
end for // each in `used`
506+
end for // each in usedInfos
524507

525508
for selData <- selDatas do
526509
if !selData.isUsed then
@@ -697,15 +680,14 @@ object CheckUnused:
697680
extension (sym: Symbol)
698681
/** is accessible without import in current context */
699682
private def isAccessibleAsIdent(using Context): Boolean =
700-
ctx.outersIterator.exists{ c =>
683+
ctx.outersIterator.exists: c =>
701684
c.owner == sym.owner
702685
|| sym.owner.isClass && c.owner.isClass
703686
&& c.owner.thisType.baseClasses.contains(sym.owner)
704687
&& c.owner.thisType.member(sym.name).alternatives.contains(sym)
705-
}
706688

707689
/** Given an import and accessibility, return selector that matches import<->symbol */
708-
private def isInImport(selData: ImportSelectorData, altName: Option[Name], isDerived: Boolean)(using Context): Boolean =
690+
private def isInImport(selData: ImportSelectorData, altName: Option[Name], prefix: Type, isDerived: Boolean)(using Context): Boolean =
709691
assert(sym.exists)
710692

711693
val selector = selData.selector
@@ -715,11 +697,13 @@ object CheckUnused:
715697
// if there is an explicit name, it must match
716698
false
717699
else
718-
if isDerived then
719-
// See i15503i.scala, grep for "package foo.test.i17156"
720-
selData.allSymbolsDealiasedForNamed.contains(dealias(sym))
721-
else
722-
selData.allSymbolsForNamed.contains(sym)
700+
(isDerived || prefix.typeSymbol.isPackageObject || selData.qualTpe =:= prefix) && (
701+
if isDerived then
702+
// See i15503i.scala, grep for "package foo.test.i17156"
703+
selData.allSymbolsDealiasedForNamed.contains(dealias(sym))
704+
else
705+
selData.allSymbolsForNamed.contains(sym)
706+
)
723707
else
724708
// Wildcard
725709
if !selData.qualTpe.member(sym.name).hasAltWith(_.symbol == sym) then
@@ -730,6 +714,7 @@ object CheckUnused:
730714
// Further check that the symbol is a given or implicit and conforms to the bound
731715
sym.isOneOf(Given | Implicit)
732716
&& (selector.bound.isEmpty || sym.info.finalResultType <:< selector.boundTpe)
717+
&& selData.qualTpe =:= prefix
733718
else
734719
// Normal wildcard, check that the symbol is not a given (but can be implicit)
735720
!sym.is(Given)
@@ -833,7 +818,7 @@ object CheckUnused:
833818
case _:tpd.Block => Local
834819
case _ => Other
835820

836-
final class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector):
821+
final case class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector):
837822
private var myUsed: Boolean = false
838823

839824
def markUsed(): Unit = myUsed = true
@@ -861,6 +846,11 @@ object CheckUnused:
861846
case class UnusedResult(warnings: Set[UnusedSymbol])
862847
object UnusedResult:
863848
val Empty = UnusedResult(Set.empty)
849+
850+
/** A symbol usage includes the name under which it was observed,
851+
* the prefix from which it was selected, and whether it is in a derived element.
852+
*/
853+
case class Usage(symbol: Symbol, name: Option[Name], prefix: Type, isDerived: Boolean)
864854
end UnusedData
865855

866856
private def dealias(symbol: Symbol)(using Context): Symbol =

compiler/test/dotty/tools/vulpix/ParallelTesting.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
860860

861861
(map.asScala.keys.toList, (unexpected ++ unpositioned).toList)
862862
end getMissingExpectedWarnings
863+
end WarnTest
863864

864865
private final class RewriteTest(testSources: List[TestSource], checkFiles: Map[JFile, JFile], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting)
865866
extends Test(testSources, times, threadLimit, suppressAllOutput) {

0 commit comments

Comments
 (0)