Skip to content

Commit

Permalink
No warn only toplevel imports, absolve more patvars by selector
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Jan 12, 2025
1 parent d669381 commit 4539e60
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 41 deletions.
35 changes: 16 additions & 19 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
refInfos.asss.addOne(sym)
tree

override def prepareForMatch(tree: Match)(using Context): Context =
// exonerate case.pat against tree.selector (simple var pat only for now)
tree.selector match
case Ident(nm) => tree.cases.foreach(k => absolveVariableBindings(List(nm), List(k.pat)))
case _ =>
ctx
override def transformMatch(tree: Match)(using Context): tree.type =
if tree.isInstanceOf[InlineMatch] && tree.selector.isEmpty then
val sf = defn.Compiletime_summonFrom
Expand Down Expand Up @@ -195,8 +201,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
tree

override def prepareForTemplate(tree: Template)(using Context): Context =
if tree.symbol.name.isReplWrapperName then
refInfos.isRepl = true
ctx.fresh.setProperty(resolvedKey, Resolved())

override def prepareForPackageDef(tree: PackageDef)(using Context): Context =
Expand Down Expand Up @@ -439,6 +443,7 @@ object CheckUnused:
if inliners == 0
&& languageImport(imp.expr).isEmpty
&& !imp.isGeneratedByEnum
&& !ctx.outer.owner.name.isReplWrapperName
then
imps.put(imp, ())
case tree: Bind =>
Expand All @@ -454,7 +459,6 @@ object CheckUnused:
if tree.symbol ne NoSymbol then
defs.addOne((tree.symbol, tree.srcPos))

var isRepl = false // have we seen a REPL artifact? avoid import warning, for example
val inlined = Stack.empty[SrcPos] // enclosing call.srcPos of inlined code
var inliners = 0 // depth of inline def
end RefInfos
Expand Down Expand Up @@ -582,7 +586,7 @@ object CheckUnused:
// TODO check for unused masking import
import scala.jdk.CollectionConverters.given
import Rewrites.ActionPatch
if (ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn) && !infos.isRepl then
if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then
type ImpSel = (Import, ImportSelector)
def isUsable(imp: Import, sel: ImportSelector): Boolean =
sel.isImportExclusion || infos.sels.containsKey(sel) || imp.isLoose(sel)
Expand Down Expand Up @@ -626,7 +630,7 @@ object CheckUnused:
val selector = textAt(sel.srcPos) // keep original
s"$qual.$selector" // don't succumb to vagaries of show
// begin actionable
val sortedImps = infos.imps.keySet.nn.asScala.toArray.sorta(_.srcPos.span.point) // sorted by pos
val sortedImps = infos.imps.keySet.nn.asScala.toArray.sortBy(_.srcPos.span.point) // sorted by pos
var index = 0
while index < sortedImps.length do
val nextImport = sortedImps.indexSatisfying(from = index + 1)(_.isPrimaryClause) // next import statement
Expand Down Expand Up @@ -706,20 +710,21 @@ object CheckUnused:
index = nextImport
end while

warnings.result().sorta(_._2.span.point)
warnings.result().sortBy(_._2.span.point)
end warnings

// Specific exclusions
def ignoreTree(tree: Tree): Boolean =
tree.hasAttachment(ForArtifact) || tree.hasAttachment(Ignore)

def absolveVariableBindings(ok: List[Name], args: List[Tree]): Unit =
ok.zip(args).foreach: (param, arg) =>
arg match
case Bind(`param`, _) => arg.withAttachment(NoWarn, ())
case _ =>

// NoWarn Binds if the name matches a "canonical" name, e.g. case element name
val nowarner = new TreeTraverser:
def absolveVariableBindings(ok: List[Name], args: List[Tree]): Unit =
ok.zip(args).foreach: (param, arg) =>
arg match
case Bind(`param`, _) => arg.withAttachment(NoWarn, ())
case _ =>
def traverse(tree: Tree)(using Context) = tree match
case UnApply(fun, _, args) =>
def untuple = defn.PairClass.companionModule.requiredMethod("unapply")
Expand Down Expand Up @@ -837,15 +842,7 @@ object CheckUnused:
extension (pos: SrcPos)
def isZeroExtentSynthetic: Boolean = pos.span.isSynthetic && pos.span.start == pos.span.end

// incredibly, there is no "sort in place" for array
extension [A <: AnyRef](arr: Array[A])
def sorta[B](f: A => B)(using ord: Ordering[B]): arr.type =
import java.util.{Arrays, Comparator}
val cmp = new Comparator[A] {
def compare(x: A, y: A): Int = ord.compare(f(x), f(y))
}
Arrays.sort(arr.asInstanceOf[Array[Object]], cmp.asInstanceOf[Comparator[Object]])
arr
// returns `until` if not satisfied
def indexSatisfying(from: Int, until: Int = arr.length)(p: A => Boolean): Int =
var i = from
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/repl/ReplCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import dotty.tools.dotc.core.Phases.Phase
import dotty.tools.dotc.core.StdNames.*
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.reporting.Diagnostic
import dotty.tools.dotc.transform.PostTyper
import dotty.tools.dotc.transform.{CheckUnused, CheckShadowing, PostTyper}
import dotty.tools.dotc.typer.ImportInfo.{withRootImports, RootRef}
import dotty.tools.dotc.typer.TyperPhase
import dotty.tools.dotc.util.Spans.*
Expand All @@ -37,6 +37,7 @@ class ReplCompiler extends Compiler:
List(Parser()),
List(ReplPhase()),
List(TyperPhase(addRootImports = false)),
List(CheckUnused.PostTyper(), CheckShadowing()),
List(CollectTopLevelImports()),
List(PostTyper()),
)
Expand Down
2 changes: 1 addition & 1 deletion tests/pos-macros/i18409.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//> using options -Werror -Wunused:all
//> using options -Werror -Wunused:imports

import scala.quoted.*

Expand Down
9 changes: 0 additions & 9 deletions tests/pos/i3323.scala

This file was deleted.

6 changes: 6 additions & 0 deletions tests/warn/i15503d.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,9 @@ def untuple(t: Tuple) =
case Tuple() =>
case h *: t => // no warn canonical names taken from tuple element types, (Head, Tail) -> (head, tail)
//case head *: tail => // no warn canonical names taken from tuple element types, (Head, Tail) -> (head, tail)

// empty case class:
// def equals(other) = other match { case other => true } // exonerated name
object i15967:
sealed trait A[-Z]
final case class B[Y]() extends A[Y]
7 changes: 6 additions & 1 deletion tests/warn/i15503i.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ class A {
y + g

def g(x: Int): Int = x match
case x: 1 => 0 // warn
case x: 1 => 0 // no warn same name as selector (for shadowing or unused)
case x: 2 => x // OK
case _ => 1 // OK

def g(x: Int): Int = x match
case y: 1 => 0 // warn unused despite trivial type and RHS
case y: 2 => y // OK
case _ => 1 // OK
}

/* ---- CHECK scala.annotation.unused ---- */
Expand Down
1 change: 0 additions & 1 deletion tests/pos/i15967.scala → tests/warn/i15967.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//> using options -Werror
sealed trait A[-Z]
final case class B[Y]() extends A[Y]

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//> using options -Werror
trait Foo:
type Bar[_]

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//> using options -Werror
import scala.util.*

trait Transaction {
Expand Down
8 changes: 8 additions & 0 deletions tests/warn/i3323.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

class Foo {
def foo[A](lss: List[List[A]]): Unit = {
lss match {
case xss: List[List[A]] => // no warn erasure
}
}
}
10 changes: 5 additions & 5 deletions tests/warn/scala2-t11681.scala
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class Revaluing(u: Int) { def f = u } // OK

case class CaseyKasem(k: Int) // OK

case class CaseyAtTheBat(k: Int)(s: String) // ok
case class CaseyAtTheBat(k: Int)(s: String) // warn unused s

trait Ignorance {
def f(readResolve: Int) = answer // warn now
Expand Down Expand Up @@ -94,14 +94,14 @@ trait Anonymous {

def f2: Int => Int = _ + 1 // OK

def g = for (i <- List(1)) yield answer // warn
def g = for (i <- List(1)) yield answer // no warn (that is a patvar)
}
trait Context[A]
trait Implicits {
def f[A](implicit ctx: Context[A]) = answer // warn
def g[A: Context] = answer // warn
def f[A](implicit ctx: Context[A]) = answer // warn implicit param even though only marker
def g[A: Context] = answer // no warn bound that is marker only
}
class Bound[A: Context] // warn
class Bound[A: Context] // no warn bound that is marker only
object Answers {
def answer: Int = 42
}
Expand Down
4 changes: 2 additions & 2 deletions tests/warn/unused-params.scala
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ trait Anonymous {
trait Context[A] { def m(a: A): A = a }
trait Implicits {
def f[A](implicit ctx: Context[A]) = answer // warn
def g[A: Context] = answer // no warn
def g[A: Context] = answer // warn
def h[A](using Context[A]) = answer // warn
}
class Bound[A: Context] // no warn
class Bound[A: Context] // warn
object Answers {
def answer: Int = 42
}
Expand Down

0 comments on commit 4539e60

Please sign in to comment.