Skip to content

Commit

Permalink
Wunused: Include import selector bounds in unused checks
Browse files Browse the repository at this point in the history
closes lampepfl#17314
  • Loading branch information
KacperFKorban authored and Kordyjan committed Apr 26, 2023
1 parent 5990252 commit 6d5a606
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 12 deletions.
34 changes: 22 additions & 12 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,11 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
override def traverse(tree: tpd.Tree)(using Context): Unit =
val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx
tree match
case imp:tpd.Import =>
case imp: tpd.Import =>
unusedDataApply(_.registerImport(imp))
imp.selectors.filter(_.isGiven).map(_.bound).collect {
case untpd.TypedSplice(tree1) => tree1
}.foreach(traverse(_)(using newCtx))
traverseChildren(tree)(using newCtx)
case ident: Ident =>
prepareForIdent(ident)
Expand Down Expand Up @@ -449,13 +452,12 @@ object CheckUnused:
val used = usedInScope.pop().toSet
// used imports in this scope
val imports = impInScope.pop()
val kept = used.filterNot { t =>
val (sym, isAccessible, optName, isDerived) = t
val kept = used.filterNot { (sym, isAccessible, optName, isDerived) =>
// 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 =>
val matchedExplicitImport = imports.exists { imp =>
sym.isInImport(imp, isAccessible, optName, isDerived) match
case None => false
case optSel@Some(sel) if sel.isWildcard =>
Expand All @@ -466,11 +468,11 @@ object CheckUnused:
unusedImport -= sel
true
}
if !exists && selWildCard.isDefined then
if !matchedExplicitImport && selWildCard.isDefined then
unusedImport -= selWildCard.get
true // a matching import exists so the symbol won't be kept for outer scope
else
exists
matchedExplicitImport
}

// if there's an outer scope
Expand Down Expand Up @@ -610,12 +612,17 @@ object CheckUnused:
* return true
*/
private def shouldSelectorBeReported(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean =
if ctx.settings.WunusedHas.strictNoImplicitWarn then
ctx.settings.WunusedHas.strictNoImplicitWarn && (
sel.isWildcard ||
imp.expr.tpe.member(sel.name.toTermName).alternatives.exists(_.symbol.isOneOf(GivenOrImplicit)) ||
imp.expr.tpe.member(sel.name.toTypeName).alternatives.exists(_.symbol.isOneOf(GivenOrImplicit))
else
false
)

extension (tree: ImportSelector)
def boundTpe: Type = tree.bound match {
case untpd.TypedSplice(tree1) => tree1.tpe
case _ => NoType
}

extension (sym: Symbol)
/** is accessible without import in current context */
Expand All @@ -628,7 +635,7 @@ object CheckUnused:
&& c.owner.thisType.member(sym.name).alternatives.contains(sym)
}

/** Given an import and accessibility, return an option of selector that match import<->symbol */
/** Given an import and accessibility, return selector that matches import<->symbol */
private def isInImport(imp: tpd.Import, isAccessible: Boolean, symName: Option[Name], isDerived: Boolean)(using Context): Option[ImportSelector] =
val tpd.Import(qual, sels) = imp
val dealiasedSym = dealias(sym)
Expand All @@ -641,9 +648,12 @@ object CheckUnused:
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)))
def givenSelector = if sym.is(Given) || sym.is(Implicit)
then sels.filter(sel => sel.isGiven && !sel.bound.isEmpty).find(sel => sel.boundTpe =:= sym.info)
else None
def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven && sel.bound.isEmpty) || sym.is(Implicit)))
if qualHasSymbol && (!isAccessible || sym.isRenamedSymbol(symName)) && sym.exists then
selector.orElse(dealiasedSelector).orElse(wildcard) // selector with name or wildcard (or given)
selector.orElse(dealiasedSelector).orElse(givenSelector).orElse(wildcard) // selector with name or wildcard (or given)
else
None

Expand Down
14 changes: 14 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i17314b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// scalac: -Wunused:all

package foo:
class Foo[T]
given Foo[Int] = new Foo[Int]


package bar:
import foo.{given foo.Foo[Int]} // error
import foo.Foo

given Foo[Int] = ???

val repro: Foo[Int] = summon[Foo[Int]]
33 changes: 33 additions & 0 deletions tests/pos-special/fatal-warnings/i17314.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// scalac: "-Wunused:all"

import java.net.URI

object circelike {
import scala.compiletime.summonInline
import scala.deriving.Mirror

type Codec[T]
type Configuration
trait ConfiguredCodec[T]
object ConfiguredCodec:
inline final def derived[A](using conf: Configuration)(using
inline mirror: Mirror.Of[A]
): ConfiguredCodec[A] =
new ConfiguredCodec[A]:
val codec = summonInline[Codec[URI]] // simplification
}

object foo {
import circelike.{Codec, Configuration}

given Configuration = ???
given Codec[URI] = ???
}

object bar {
import circelike.Codec
import circelike.{Configuration, ConfiguredCodec}
import foo.{given Configuration, given Codec[URI]}

case class Operator(url: URI) derives ConfiguredCodec
}
12 changes: 12 additions & 0 deletions tests/pos-special/fatal-warnings/i17314a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// scalac: -Wunused:all

package foo:
class Foo[T]
given Foo[Int] = new Foo[Int]


package bar:
import foo.{given foo.Foo[Int]}
import foo.Foo

val repro: Foo[Int] = summon[Foo[Int]]

0 comments on commit 6d5a606

Please sign in to comment.