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

Make unused-import warnings easier to silence (support filtering by origin=) #9939

Merged
merged 2 commits into from Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/compiler/scala/tools/nsc/Reporting.scala
Expand Up @@ -111,6 +111,7 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
private def issueWarning(warning: Message): Unit = {
def verbose = warning match {
case Message.Deprecation(_, msg, site, origin, version) => s"[${warning.category.name} @ $site | origin=$origin | version=${version.filterString}] $msg"
case Message.Origin(_, msg, category, site, origin @ _) => s"[${category.name} @ $site] $msg" // origin text is obvious at caret
case Message.Plain(_, msg, category, site) => s"[${category.name} @ $site] $msg"
}
wconf.action(warning) match {
Expand Down Expand Up @@ -192,7 +193,7 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
impl(sym)
} else ""

def deprecationWarning(pos: Position, msg: String, since: String, site: String, origin: String): Unit =
override def deprecationWarning(pos: Position, msg: String, since: String, site: String, origin: String): Unit =
issueIfNotSuppressed(Message.Deprecation(pos, msg, site, origin, Version.fromString(since)))

def deprecationWarning(pos: Position, origin: Symbol, site: Symbol, msg: String, since: String): Unit =
Expand Down Expand Up @@ -262,6 +263,10 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
def warning(pos: Position, msg: String, category: WarningCategory, site: Symbol): Unit =
warning(pos, msg, category, siteName(site))

// Provide an origin for the warning.
def warning(pos: Position, msg: String, category: WarningCategory, site: Symbol, origin: String): Unit =
issueIfNotSuppressed(Message.Origin(pos, msg, category, siteName(site), origin))

// used by Global.deprecationWarnings, which is used by sbt
def deprecationWarnings: List[(Position, String)] = summaryMap(Action.WarningSummary, WarningCategory.Deprecation).toList.map(p => (p._1, p._2.msg))
def uncheckedWarnings: List[(Position, String)] = summaryMap(Action.WarningSummary, WarningCategory.Unchecked).toList.map(p => (p._1, p._2.msg))
Expand Down Expand Up @@ -305,8 +310,12 @@ object Reporting {
}

object Message {
// an ordinary Message has a `category` for filtering and the `site` where it was issued
final case class Plain(pos: Position, msg: String, category: WarningCategory, site: String) extends Message

// a Plain message with an `origin` which should not be empty. For example, the origin of an unused import is the fully-qualified selection
final case class Origin(pos: Position, msg: String, category: WarningCategory, site: String, origin: String) extends Message

// `site` and `origin` may be empty
final case class Deprecation(pos: Position, msg: String, site: String, origin: String, since: Version) extends Message {
def category: WarningCategory = WarningCategory.Deprecation
Expand Down Expand Up @@ -502,6 +511,7 @@ object Reporting {
final case class DeprecatedOrigin(pattern: Regex) extends MessageFilter {
def matches(message: Message): Boolean = message match {
case m: Message.Deprecation => pattern.matches(m.origin)
case m: Message.Origin => pattern.matches(m.origin)
case _ => false
}
}
Expand Down Expand Up @@ -558,7 +568,7 @@ object Reporting {
regex(s.substring(5)).map(SitePattern)
} else if (s.startsWith("origin=")) {
regex(s.substring(7)).map(DeprecatedOrigin)
} else if(s.startsWith("since")) {
} else if (s.startsWith("since")) {
def fail = Left(s"invalid since filter: `$s`; required shape: `since<1.2.3`, `since=3.2`, `since>2`")
if (s.length < 6) fail
else {
Expand Down
32 changes: 12 additions & 20 deletions src/compiler/scala/tools/nsc/typechecker/Contexts.scala
Expand Up @@ -65,45 +65,34 @@ trait Contexts { self: Analyzer =>
)

private lazy val allUsedSelectors =
mutable.Map[ImportInfo, Set[ImportSelector]]() withDefaultValue Set()
mutable.Map.empty[ImportInfo, Set[ImportSelector]].withDefaultValue(Set.empty)
private lazy val allImportInfos =
mutable.Map[CompilationUnit, List[(ImportInfo, Symbol)]]() withDefaultValue Nil
mutable.Map.empty[CompilationUnit, List[(ImportInfo, Symbol)]].withDefaultValue(Nil)

def warnUnusedImports(unit: CompilationUnit) = {
def cullUnusedSelections(infos0: List[(ImportInfo, Symbol)]): List[(Position, Symbol)] = {
var unused = List.empty[(Position, Symbol)]
def warnUnusedImports(unit: CompilationUnit) = if (!unit.isJava) {
def warnUnusedSelections(infos0: List[(ImportInfo, Symbol)]): Unit = {
type Culled = (Position, Symbol, String)
var unused = List.empty[Culled]
@tailrec def loop(infos: List[(ImportInfo, Symbol)]): Unit =
infos match {
case (info, owner) :: rest =>
val used = allUsedSelectors.remove(info).getOrElse(Set.empty)
// since we are going in reverse order, add unused selectors from right to left, last to first
def checkSelectors(selectors: List[ImportSelector]): Unit =
selectors match {
case selector :: rest =>
checkSelectors(rest)
if (!selector.isMask && !used(selector))
unused ::= ((info.posOf(selector), owner))
unused ::= ((info.posOf(selector), owner, info.fullSelectorString(selector)))
case _ =>
}
checkSelectors(info.tree.selectors)
loop(rest)
case _ =>
}
loop(infos0)
unused
unused.foreach { case (pos, owner, origin) => runReporting.warning(pos, "Unused import", WarningCategory.UnusedImports, owner, origin) }
}
@tailrec def warnUnusedSelections(unused: List[(Position, Symbol)]): Unit =
unused match {
case (pos, site) :: rest =>
runReporting.warning(pos, "Unused import", WarningCategory.UnusedImports, site = site)
warnUnusedSelections(rest)
case _ =>
}
if (!unit.isJava)
allImportInfos.remove(unit) match {
case Some(importInfos) => warnUnusedSelections(cullUnusedSelections(importInfos))
case _ => ()
}
allImportInfos.remove(unit).foreach(warnUnusedSelections)
}

var lastAccessCheckDetails: String = ""
Expand Down Expand Up @@ -1868,6 +1857,9 @@ trait Contexts { self: Analyzer =>
}
}

def fullSelectorString(s: ImportSelector): String =
s"${if (qual.tpe.isError) tree.toString else qual.tpe.typeSymbol.fullName}.${selectorString(s)}"

private def selectorString(s: ImportSelector): String =
if (s.isWildcard) "_"
else if (s.isRename) s"${s.name} => ${s.rename}"
Expand Down
6 changes: 6 additions & 0 deletions test/files/neg/t12326.check
@@ -0,0 +1,6 @@
t12326.scala:5: warning: Unused import
import scala.collection.mutable._
^
error: No warnings can be incurred under -Werror.
1 warning
1 error
9 changes: 9 additions & 0 deletions test/files/neg/t12326.scala
@@ -0,0 +1,9 @@
// scalac: -Werror -Wunused:imports -Wconf:site=p.T:s

package p

import scala.collection.mutable._

trait T {
import scala.concurrent.ExecutionContext.Implicits._
}
6 changes: 6 additions & 0 deletions test/files/neg/t12326b.check
@@ -0,0 +1,6 @@
t12326b.scala:5: warning: Unused import
import scala.collection.mutable.{ListBuffer, Map, Set}
^
error: No warnings can be incurred under -Werror.
1 warning
1 error
9 changes: 9 additions & 0 deletions test/files/neg/t12326b.scala
@@ -0,0 +1,9 @@
// scalac: -Werror -Wunused:imports -Wconf:site=p.T:s,origin=scala.collection.mutable.Map:s,origin=scala.collection.mutable.Set:s

package p

import scala.collection.mutable.{ListBuffer, Map, Set}

trait T {
import scala.concurrent.ExecutionContext.Implicits._
}
6 changes: 6 additions & 0 deletions test/files/neg/t12326c.check
@@ -0,0 +1,6 @@
t12326c.scala:8: warning: Unused import
import scala.concurrent.ExecutionContext.Implicits._
^
error: No warnings can be incurred under -Werror.
1 warning
1 error
9 changes: 9 additions & 0 deletions test/files/neg/t12326c.scala
@@ -0,0 +1,9 @@
// scalac: -Werror -Wunused:imports -Wconf:origin=scala.collection.mutable.*:s

package p

import scala.collection.mutable.{ListBuffer, Map, Set}

trait T {
import scala.concurrent.ExecutionContext.Implicits._
}
7 changes: 7 additions & 0 deletions test/files/pos/t12326.scala
@@ -0,0 +1,7 @@
// scalac: -Werror -Wunused:imports -Wconf:origin=scala.collection.mutable._:s,origin=scala.concurrent.ExecutionContext.Implicits._:s

import scala.collection.mutable._

trait T {
import scala.concurrent.ExecutionContext.Implicits._
}
8 changes: 8 additions & 0 deletions test/files/pos/t12326b.scala
@@ -0,0 +1,8 @@
// scalac: -Werror -Wunused:imports

import annotation._

@nowarn("origin=scala.concurrent.ExecutionContext.Implicits._")
trait T {
import scala.concurrent.ExecutionContext.Implicits._
}