Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport "Fix wunused false positive when deriving alias type" #17264

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 93 additions & 65 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
@@ -1,34 +1,30 @@
package dotty.tools.dotc.transform

import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.ast.tpd.TreeTraverser
import dotty.tools.dotc.ast.tpd.{Inlined, TreeTraverser}
import dotty.tools.dotc.ast.untpd
import dotty.tools.dotc.ast.untpd.ImportSelector
import dotty.tools.dotc.config.ScalaSettings
import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.core.Decorators.{em, i}
import dotty.tools.dotc.core.Flags._
import dotty.tools.dotc.core.Flags.*
import dotty.tools.dotc.core.Phases.Phase
import dotty.tools.dotc.core.StdNames
import dotty.tools.dotc.report
import dotty.tools.dotc.reporting.Message
import dotty.tools.dotc.typer.ImportInfo
import dotty.tools.dotc.util.Property
import dotty.tools.dotc.util.{Property, SrcPos}
import dotty.tools.dotc.core.Mode
import dotty.tools.dotc.core.Types.TypeTraverser
import dotty.tools.dotc.core.Types.Type
import dotty.tools.dotc.core.Types.AnnotatedType
import dotty.tools.dotc.core.Types.{AnnotatedType, ConstantType, NameFilter, NoType, TermRef, Type, TypeTraverser}
import dotty.tools.dotc.core.Flags.flagsString
import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Names.Name
import dotty.tools.dotc.transform.MegaPhase.MiniPhase
import dotty.tools.dotc.core.Annotations
import dotty.tools.dotc.core.Definitions
import dotty.tools.dotc.core.Types.ConstantType
import dotty.tools.dotc.core.NameKinds.WildcardParamName
import dotty.tools.dotc.core.Types.TermRef
import dotty.tools.dotc.core.Types.NameFilter

import dotty.tools.dotc.core.Symbols.Symbol
import dotty.tools.dotc.core.StdNames.nme


/**
Expand All @@ -46,12 +42,11 @@ class CheckUnused extends MiniPhase:
*/
private val _key = Property.Key[UnusedData]

extension (k: Property.Key[UnusedData])
private def unusedDataApply[U](f: UnusedData => U)(using Context): Context =
ctx.property(_key).foreach(f)
ctx
private def getUnusedData(using Context): Option[UnusedData] =
ctx.property(_key)
private def unusedDataApply[U](f: UnusedData => U)(using Context): Context =
ctx.property(_key).foreach(f)
ctx
private def getUnusedData(using Context): Option[UnusedData] =
ctx.property(_key)

override def phaseName: String = CheckUnused.phaseName

Expand All @@ -71,7 +66,7 @@ class CheckUnused extends MiniPhase:
// ========== END + REPORTING ==========

override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree =
_key.unusedDataApply(ud => reportUnused(ud.getUnused))
unusedDataApply(ud => reportUnused(ud.getUnused))
tree

// ========== MiniPhase Prepare ==========
Expand All @@ -80,16 +75,24 @@ class CheckUnused extends MiniPhase:
traverser.traverse(tree)
ctx

override def prepareForInlined(tree: tpd.Inlined)(using Context): Context =
traverser.traverse(tree.call)
ctx

override def prepareForIdent(tree: tpd.Ident)(using Context): Context =
if tree.symbol.exists then
_key.unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))
if tree.symbol.exists then
val prefixes = LazyList.iterate(tree.typeOpt.normalizedPrefix)(_.normalizedPrefix).takeWhile(_ != NoType)
.take(10) // Failsafe for the odd case if there was an infinite cycle
for prefix <- prefixes do
unusedDataApply(_.registerUsed(prefix.classSymbol, None))
unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))
else if tree.hasType then
_key.unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name)))
unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name)))
else
ctx

override def prepareForSelect(tree: tpd.Select)(using Context): Context =
_key.unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))
unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))

override def prepareForBlock(tree: tpd.Block)(using Context): Context =
pushInBlockTemplatePackageDef(tree)
Expand All @@ -101,33 +104,40 @@ class CheckUnused extends MiniPhase:
pushInBlockTemplatePackageDef(tree)

override def prepareForValDef(tree: tpd.ValDef)(using Context): Context =
_key.unusedDataApply{ud =>
unusedDataApply{ud =>
// do not register the ValDef generated for `object`
if !tree.symbol.is(Module) then
traverseAnnotations(tree.symbol)
if !tree.symbol.is(Module) then
ud.registerDef(tree)
if tree.name.mangledString.startsWith(nme.derived.mangledString + "$")
&& tree.typeOpt != NoType then
ud.registerUsed(tree.typeOpt.typeSymbol, None, true)
ud.addIgnoredUsage(tree.symbol)
}

override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context =
_key.unusedDataApply{ ud =>
unusedDataApply{ ud =>
import ud.registerTrivial
tree.registerTrivial
traverseAnnotations(tree.symbol)
ud.registerDef(tree)
ud.addIgnoredUsage(tree.symbol)
}

override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context =
_key.unusedDataApply{ ud =>
unusedDataApply{ ud =>
if !tree.symbol.is(Param) then // Ignore type parameter (as Scala 2)
traverseAnnotations(tree.symbol)
ud.registerDef(tree)
ud.addIgnoredUsage(tree.symbol)
}

override def prepareForBind(tree: tpd.Bind)(using Context): Context =
_key.unusedDataApply(_.registerPatVar(tree))
traverseAnnotations(tree.symbol)
unusedDataApply(_.registerPatVar(tree))

override def prepareForTypeTree(tree: tpd.TypeTree)(using Context): Context =
typeTraverser(_key.unusedDataApply).traverse(tree.tpe)
if !tree.isInstanceOf[tpd.InferredTypeTree] then typeTraverser(unusedDataApply).traverse(tree.tpe)
ctx

// ========== MiniPhase Transform ==========
Expand All @@ -145,27 +155,27 @@ class CheckUnused extends MiniPhase:
tree

override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree =
_key.unusedDataApply(_.removeIgnoredUsage(tree.symbol))
unusedDataApply(_.removeIgnoredUsage(tree.symbol))
tree

override def transformDefDef(tree: tpd.DefDef)(using Context): tpd.Tree =
_key.unusedDataApply(_.removeIgnoredUsage(tree.symbol))
unusedDataApply(_.removeIgnoredUsage(tree.symbol))
tree

override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree =
_key.unusedDataApply(_.removeIgnoredUsage(tree.symbol))
unusedDataApply(_.removeIgnoredUsage(tree.symbol))
tree

// ---------- MiniPhase HELPERS -----------

private def pushInBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context =
_key.unusedDataApply { ud =>
unusedDataApply { ud =>
ud.pushScope(UnusedData.ScopeType.fromTree(tree))
}
ctx

private def popOutBlockTemplatePackageDef()(using Context): Context =
_key.unusedDataApply { ud =>
unusedDataApply { ud =>
ud.popScope()
}
ctx
Expand All @@ -188,7 +198,7 @@ class CheckUnused extends MiniPhase:
val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx
tree match
case imp:tpd.Import =>
_key.unusedDataApply(_.registerImport(imp))
unusedDataApply(_.registerImport(imp))
traverseChildren(tree)(using newCtx)
case ident: Ident =>
prepareForIdent(ident)
Expand All @@ -198,7 +208,7 @@ class CheckUnused extends MiniPhase:
traverseChildren(tree)(using newCtx)
case _: (tpd.Block | tpd.Template | tpd.PackageDef) =>
//! DIFFERS FROM MINIPHASE
_key.unusedDataApply { ud =>
unusedDataApply { ud =>
ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)(using newCtx))
}
case t:tpd.ValDef =>
Expand All @@ -216,9 +226,10 @@ class CheckUnused extends MiniPhase:
case t: tpd.Bind =>
prepareForBind(t)
traverseChildren(tree)(using newCtx)
case _: tpd.InferredTypeTree =>
case [email protected]() =>
//! DIFFERS FROM MINIPHASE
typeTraverser(_key.unusedDataApply).traverse(t.tpe)
typeTraverser(unusedDataApply).traverse(t.tpe)
traverseChildren(tree)(using newCtx)
case _ =>
//! DIFFERS FROM MINIPHASE
Expand All @@ -228,9 +239,18 @@ class CheckUnused extends MiniPhase:

/** This is a type traverser which catch some special Types not traversed by the term traverser above */
private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser:
override def traverse(tp: Type): Unit = tp match
case AnnotatedType(_, annot) => dt(_.registerUsed(annot.symbol, None))
case _ => traverseChildren(tp)
override def traverse(tp: Type): Unit =
if tp.typeSymbol.exists then dt(_.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name)))
tp match
case AnnotatedType(_, annot) =>
dt(_.registerUsed(annot.symbol, None))
traverseChildren(tp)
case _ =>
traverseChildren(tp)

/** This traverse the annotations of the symbol */
private def traverseAnnotations(sym: Symbol)(using Context): Unit =
sym.denot.annotations.foreach(annot => traverser.traverse(annot.tree))

/** Do the actual reporting given the result of the anaylsis */
private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit =
Expand Down Expand Up @@ -274,7 +294,6 @@ object CheckUnused:
private class UnusedData:
import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult
import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack}
import dotty.tools.dotc.core.Symbols.Symbol
import UnusedData.ScopeType

/** The current scope during the tree traversal */
Expand All @@ -288,7 +307,8 @@ object CheckUnused:
*
* See the `isAccessibleAsIdent` extension method below in the file
*/
private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name])]())
private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean)]())
private val usedInPosition = MutSet[(SrcPos, Name)]()
/* unused import collected during traversal */
private val unusedImport = MutSet[ImportSelector]()

Expand Down Expand Up @@ -324,25 +344,21 @@ object CheckUnused:
execInNewScope
popScope()

/** Register all annotations of this symbol's denotation */
def registerUsedAnnotation(sym: Symbol)(using Context): Unit =
val annotSym = sym.denot.annotations.map(_.symbol)
annotSym.foreach(s => registerUsed(s, None))

/**
* Register a found (used) symbol along with its name
*
* The optional name will be used to target the right import
* as the same element can be imported with different renaming
*/
def registerUsed(sym: Symbol, name: Option[Name])(using Context): Unit =
def registerUsed(sym: Symbol, name: Option[Name], isDerived: Boolean = false)(using Context): Unit =
if !isConstructorOfSynth(sym) && !doNotRegister(sym) then
if sym.isConstructor && sym.exists then
registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class
else
usedInScope.top += ((sym, sym.isAccessibleAsIdent, name))
usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name))
usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name))
usedInScope.top += ((sym, sym.isAccessibleAsIdent, name, isDerived))
usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name, isDerived))
usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name, isDerived))
name.map(n => usedInPosition += ((sym.sourcePos, n)))

/** Register a symbol that should be ignored */
def addIgnoredUsage(sym: Symbol)(using Context): Unit =
Expand All @@ -363,22 +379,19 @@ object CheckUnused:

/** Register (or not) some `val` or `def` according to the context, scope and flags */
def registerDef(memDef: tpd.MemberDef)(using Context): Unit =
// register the annotations for usage
registerUsedAnnotation(memDef.symbol)
if memDef.isValidMemberDef then
if memDef.isValidParam then
if memDef.symbol.isOneOf(GivenOrImplicit) then
implicitParamInScope += memDef
else
explicitParamInScope += memDef
else if currScopeType.top == ScopeType.Local then
else if currScopeType.top == ScopeType.Local then
localDefInScope += memDef
else if memDef.shouldReportPrivateDef then
privateDefInScope += memDef

/** Register pattern variable */
def registerPatVar(patvar: tpd.Bind)(using Context): Unit =
registerUsedAnnotation(patvar.symbol)
if !patvar.symbol.isUnusedAnnot then
patVarsInScope += patvar

Expand All @@ -398,16 +411,15 @@ object CheckUnused:
// used symbol in this scope
val used = usedInScope.pop().toSet
// used imports in this scope
val imports = impInScope.pop().toSet
val imports = impInScope.pop()
val kept = used.filterNot { t =>
val (sym, isAccessible, optName) = t
val (sym, isAccessible, optName, isDerived) = t
// keep the symbol for outer scope, if it matches **no** import

// This is the first matching wildcard selector
var selWildCard: Option[ImportSelector] = None

val exists = imports.exists { imp =>
sym.isInImport(imp, isAccessible, optName) match
sym.isInImport(imp, isAccessible, optName, isDerived) match
case None => false
case optSel@Some(sel) if sel.isWildcard =>
if selWildCard.isEmpty then selWildCard = optSel
Expand Down Expand Up @@ -450,6 +462,7 @@ object CheckUnused:
if ctx.settings.WunusedHas.locals then
localDefInScope
.filterNot(d => d.symbol.usedDefContains)
.filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
.map(d => d.namePos -> WarnTypes.LocalDefs).toList
else
Nil
Expand Down Expand Up @@ -478,6 +491,7 @@ object CheckUnused:
if ctx.settings.WunusedHas.patvars then
patVarsInScope
.filterNot(d => d.symbol.usedDefContains)
.filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
.map(d => d.namePos -> WarnTypes.PatVars).toList
else
Nil
Expand Down Expand Up @@ -552,16 +566,29 @@ object CheckUnused:
}

/** Given an import and accessibility, return an option of selector that match import<->symbol */
private def isInImport(imp: tpd.Import, isAccessible: Boolean, symName: Option[Name])(using Context): Option[ImportSelector] =
private def isInImport(imp: tpd.Import, isAccessible: Boolean, symName: Option[Name], isDerived: Boolean)(using Context): Option[ImportSelector] =
val tpd.Import(qual, sels) = imp
val qualHasSymbol = qual.tpe.member(sym.name).alternatives.map(_.symbol).contains(sym)
val dealiasedSym = dealias(sym)
val simpleSelections = qual.tpe.member(sym.name).alternatives
val typeSelections = sels.flatMap(n => qual.tpe.member(n.name.toTypeName).alternatives)
val termSelections = sels.flatMap(n => qual.tpe.member(n.name.toTermName).alternatives)
val selectionsToDealias = typeSelections ::: termSelections
val qualHasSymbol = simpleSelections.map(_.symbol).contains(sym) || (simpleSelections ::: selectionsToDealias).map(_.symbol).map(dealias).contains(dealiasedSym)
def selector = sels.find(sel => (sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) && symName.map(n => n.toTermName == sel.rename).getOrElse(true))
def dealiasedSelector = if(isDerived) sels.flatMap(sel => selectionsToDealias.map(m => (sel, m.symbol))).collect {
case (sel, sym) if dealias(sym) == dealiasedSym => sel
}.headOption else None
def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven) || sym.is(Implicit)))
if qualHasSymbol && !isAccessible && sym.exists then
selector.orElse(wildcard) // selector with name or wildcard (or given)
selector.orElse(dealiasedSelector).orElse(wildcard) // selector with name or wildcard (or given)
else
None


private def dealias(symbol: Symbol)(using Context): Symbol =
if(symbol.isType && symbol.asType.denot.isAliasType) then
symbol.asType.typeRef.dealias.typeSymbol
else symbol
/** Annotated with @unused */
private def isUnusedAnnot(using Context): Boolean =
sym.annotations.exists(a => a.symbol == ctx.definitions.UnusedAnnot)
Expand All @@ -578,10 +605,10 @@ object CheckUnused:
else
false

private def usedDefContains(using Context): Boolean =
private def usedDefContains(using Context): Boolean =
sym.everySymbol.exists(usedDef.apply)

private def everySymbol(using Context): List[Symbol] =
private def everySymbol(using Context): List[Symbol] =
List(sym, sym.companionClass, sym.companionModule, sym.moduleClass).filter(_.exists)

end extension
Expand Down Expand Up @@ -614,10 +641,11 @@ object CheckUnused:
private def isValidParam(using Context): Boolean =
val sym = memDef.symbol
(sym.is(Param) || sym.isAllOf(PrivateParamAccessor | Local, butNot = CaseAccessor)) &&
!isSyntheticMainParam(sym) &&
!sym.shouldNotReportParamOwner
!isSyntheticMainParam(sym) &&
!sym.shouldNotReportParamOwner &&
(!sym.exists || !sym.owner.isAllOf(Synthetic | PrivateLocal))

private def shouldReportPrivateDef(using Context): Boolean =
private def shouldReportPrivateDef(using Context): Boolean =
currScopeType.top == ScopeType.Template && !memDef.symbol.isConstructor && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor)

extension (imp: tpd.Import)
Expand Down
Loading