Skip to content

Commit

Permalink
Warn unused imports
Browse files Browse the repository at this point in the history
Handle language feature imports.
Skip Java sources.
Support rewrite.
Disable test.
  • Loading branch information
som-snytt committed Mar 7, 2022
1 parent fb618ad commit d40f892
Show file tree
Hide file tree
Showing 18 changed files with 269 additions and 45 deletions.
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
val rename: TermName = renamed match
case Ident(rename: TermName) => rename
case _ => name

def isMask: Boolean = !isWildcard && (rename == nme.WILDCARD)
}

case class Number(digits: String, kind: NumberKind)(implicit @constructorOnly src: SourceFile) extends TermTree
Expand Down
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import scala.language.unsafeNulls

import dotty.tools.dotc.config.PathResolver.Defaults
import dotty.tools.dotc.config.Settings.{Setting, SettingGroup}
import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.rewrites.Rewrites
import dotty.tools.io.{AbstractFile, Directory, JDK9Reflectors, PlainDirectory}

import scala.util.chaining._
import scala.util.chaining.*

class ScalaSettings extends SettingGroup with AllScalaSettings

Expand Down Expand Up @@ -151,12 +151,13 @@ private sealed trait WarningSettings:
name = "-Wunused",
helpArg = "warning",
descr = "Enable or disable specific `unused` warnings",
choices = List("nowarn", "all"),
choices = List("nowarn", "all", "imports"),
default = Nil
)
object WunusedHas:
def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s))
def nowarn(using Context) = allOr("nowarn")
def imports(using Context) = allOr("imports")

val Wconf: Setting[List[String]] = MultiStringSetting(
"-Wconf",
Expand Down
106 changes: 97 additions & 9 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import interfaces.CompilerCallback
import Decorators._
import Periods._
import Names._
import Flags.*
import Phases._
import Types._
import Symbols._
Expand All @@ -23,7 +24,9 @@ import reporting._
import io.{AbstractFile, NoAbstractFile, PlainFile, Path}
import scala.io.Codec
import collection.mutable
import parsing.Parsers
import printing._
import config.Printers.{implicits, implicitsDetailed}
import config.{JavaPlatform, SJSPlatform, Platform, ScalaSettings, ScalaRelease}
import classfile.ReusableDataReader
import StdNames.nme
Expand All @@ -42,7 +45,9 @@ import dotty.tools.tasty.TastyFormat
import dotty.tools.dotc.config.{ NoScalaVersion, SpecificScalaVersion, AnyScalaVersion, ScalaBuild }
import dotty.tools.dotc.core.tasty.TastyVersion

object Contexts {
import scala.util.chaining.given

object Contexts:

private val (compilerCallbackLoc, store1) = Store.empty.newLocation[CompilerCallback]()
private val (sbtCallbackLoc, store2) = store1.newLocation[AnalysisCallback]()
Expand All @@ -54,8 +59,9 @@ object Contexts {
private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]()
private val (importInfoLoc, store9) = store8.newLocation[ImportInfo | Null]()
private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner)
private val (usagesLoc, store11) = store10.newLocation[Usages](Usages())

private val initialStore = store10
private val initialStore = store11

/** The current context */
inline def ctx(using ctx: Context): Context = ctx
Expand Down Expand Up @@ -241,6 +247,9 @@ object Contexts {
/** The current type assigner or typer */
def typeAssigner: TypeAssigner = store(typeAssignerLoc)

/** Tracker for usages of elements such as import selectors. */
def usages: Usages = store(usagesLoc)

/** The new implicit references that are introduced by this scope */
protected var implicitsCache: ContextualImplicits | Null = null
def implicits: ContextualImplicits = {
Expand All @@ -249,9 +258,7 @@ object Contexts {
val implicitRefs: List[ImplicitRef] =
if (isClassDefContext)
try owner.thisType.implicitMembers
catch {
case ex: CyclicReference => Nil
}
catch case ex: CyclicReference => Nil
else if (isImportContext) importInfo.nn.importedImplicits
else if (isNonEmptyScopeContext) scope.implicitDecls
else Nil
Expand Down Expand Up @@ -477,8 +484,8 @@ object Contexts {
else fresh.setOwner(exprOwner)

/** A new context that summarizes an import statement */
def importContext(imp: Import[?], sym: Symbol): FreshContext =
fresh.setImportInfo(ImportInfo(sym, imp.selectors, imp.expr))
def importContext(imp: Import[?], sym: Symbol, enteringSyms: Boolean = false): FreshContext =
fresh.setImportInfo(ImportInfo(sym, imp.selectors, imp.expr).tap(ii => if enteringSyms && ctx.settings.WunusedHas.imports then usages += ii))

def scalaRelease: ScalaRelease =
val releaseName = base.settings.scalaOutputVersion.value
Expand Down Expand Up @@ -956,7 +963,7 @@ object Contexts {
private[dotc] var stopInlining: Boolean = false

/** A variable that records that some error was reported in a globally committable context.
* The error will not necessarlily be emitted, since it could still be that
* The error will not necessarily be emitted, since it could still be that
* the enclosing context will be aborted. The variable is used as a smoke test
* to turn off assertions that might be wrong if the program is erroneous. To
* just test for `ctx.reporter.errorsReported` is not always enough, since it
Expand Down Expand Up @@ -1013,4 +1020,85 @@ object Contexts {
if (thread == null) thread = Thread.currentThread()
else assert(thread == Thread.currentThread(), "illegal multithreaded access to ContextBase")
}
}
end ContextState

/** Collect information about the run for purposes of additional diagnostics.
*/
class Usages:
import rewrites.Rewrites.patch
private val selectors = mutable.Map.empty[ImportInfo, Set[untpd.ImportSelector]].withDefaultValue(Set.empty)
private val importInfos = mutable.Map.empty[CompilationUnit, List[(ImportInfo, Symbol)]].withDefaultValue(Nil)

// register an import
def +=(info: ImportInfo)(using Context): Unit =
if ctx.settings.WunusedHas.imports && !ctx.owner.is(Enum) && !ctx.compilationUnit.isJava then
importInfos(ctx.compilationUnit) ::= ((info, ctx.owner))

// mark a selector as used
def use(info: ImportInfo, selector: untpd.ImportSelector)(using Context): Unit =
if ctx.settings.WunusedHas.imports && !info.isRootImport then
selectors(info) += selector

// unused import, owner, which selector
def unused(using Context): List[(ImportInfo, Symbol, untpd.ImportSelector)] =
var unusages = List.empty[(ImportInfo, Symbol, untpd.ImportSelector)]
if ctx.settings.WunusedHas.imports && !ctx.compilationUnit.isJava then
//if ctx.settings.Ydebug.value then
// println(importInfos.get(ctx.compilationUnit).map(iss => iss.map((ii, s) => s"${ii.show} ($ii)")).getOrElse(Nil).mkString("Registered ImportInfos\n", "\n", ""))
// println(selectors.toList.flatMap((k,v) => v.toList.map(sel => s"${k.show} -> $sel")).mkString("Used selectors\n", "\n", ""))
def checkUsed(info: ImportInfo, owner: Symbol): Unit =
val used = selectors(info)
var needsPatch = false
def cull(toCheck: List[untpd.ImportSelector]): Unit =
toCheck match
case selector :: rest =>
cull(rest) // reverse
if !selector.isMask && !used(selector) then
unusages ::= ((info, owner, selector))
needsPatch = true
case _ =>
cull(info.selectors)
val decidingFactor = false // whether to apply patches
if decidingFactor && needsPatch && !ctx.settings.rewrite.value.isEmpty then
val retained = info.selectors.filter(sel => sel.isMask || used(sel))
val infoPos = info.qualifier.sourcePos
val lineText = infoPos.lineContent
val src = ctx.compilationUnit.source
val lineSpan = src.lineSpan(infoPos.start)
val selectorSpan = info.selectors.map(_.span).reduce(_ union _)
val lineSource = SourceFile.virtual(name = "import-line.scala", content = lineText)
val parser = Parsers.Parser(lineSource)
val lineTree = parser.parse()
val PackageDef(_, pieces) = lineTree: @unchecked
// patch if there's just one import on the line, i.e., not import a.b, c.d
if pieces.length == 1 then
if retained.isEmpty then
patch(src, lineSpan, "") // line deletion
else if retained.size == 1 && info.selectors.size > 1 then
var starting = info.selectors.head.span.start
while starting > lineSpan.start && src.content()(starting) != '{' do starting -= 1
var ending = info.selectors.last.span.end
while ending <= lineSpan.end && src.content()(ending) != '}' do ending += 1
if ending < lineSpan.end then ending += 1 // past the close brace
val widened = selectorSpan.withStart(starting).withEnd(ending)
patch(src, widened, toText(retained)) // try to remove braces
else
patch(src, selectorSpan, toText(retained))
end checkUsed
importInfos.remove(ctx.compilationUnit).foreach(_.foreach(checkUsed))
unusages
end unused

// just the selectors, no need to add braces
private def toText(retained: List[untpd.ImportSelector])(using Context): String =
def selected(sel: untpd.ImportSelector) =
if sel.isGiven then "given"
else if sel.isWildcard then "*"
else if sel.name == sel.rename then sel.name.show
else s"${sel.name.show} as ${sel.rename.show}"
retained.map(selected).mkString(", ")

def clear()(using Context): Unit =
importInfos.clear()
selectors.clear()
end Usages
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
def run(uri: URI, sourceCode: String): List[Diagnostic] = run(uri, toSource(uri, sourceCode))

def run(uri: URI, source: SourceFile): List[Diagnostic] = {
import typer.ImportInfo._
import typer.ImportInfo.withRootImports

val previousCtx = myCtx
try {
Expand Down
12 changes: 9 additions & 3 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Contexts._
import Scopes.Scope, Denotations.Denotation, Annotations.Annotation
import StdNames.nme
import ast.Trees._
import ast.untpd
import typer.Implicits._
import typer.ImportInfo
import Variances.varianceSign
Expand Down Expand Up @@ -611,12 +612,17 @@ class PlainPrinter(_ctx: Context) extends Printer {
}

def toText(importInfo: ImportInfo): Text =
def selected(sel: untpd.ImportSelector) =
if sel.isGiven then "given"
else if sel.isWildcard then "*"
else if sel.name == sel.rename then sel.name.show
else s"${sel.name.show} as ${sel.rename.show}"
val siteStr = importInfo.site.show
val exprStr = if siteStr.endsWith(".type") then siteStr.dropRight(5) else siteStr
val selectorStr = importInfo.selectors match
case sel :: Nil if sel.renamed.isEmpty && sel.bound.isEmpty =>
if sel.isGiven then "given" else sel.name.show
case _ => "{...}"
case sel :: Nil if sel.renamed.isEmpty && sel.bound.isEmpty => selected(sel)
case sels => sels.map(selected).mkString("{", ", ", "}")
//case _ => "{...}"
s"import $exprStr.$selectorStr"

def toText(c: OrderingConstraint): Text =
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/reporting/WConf.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ object WConf:
case ErrorId(num) =>
ErrorMessageID.fromErrorNumber(num.toInt) match
case Some(errId) => Right(MessageID(errId))
case _ => Left(s"unknonw error message number: E$num")
case _ => Left(s"unknown error message number: E$num")
case _ =>
Left(s"invalid error message id: $conf")
case "name" =>
Expand Down
21 changes: 17 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,17 @@ import scala.annotation.threadUnsafe
object Implicits:
import tpd._

/** An implicit definition `implicitRef` that is visible under a different name, `alias`.
/** Pairs an imported `ImplicitRef` with its `ImportInfo` for diagnostic bookkeeping.
*/
class ImportedImplicitRef(val underlyingRef: TermRef, val importInfo: ImportInfo, val selector: Int) extends ImplicitRef:
def implicitName(using Context): TermName = underlyingRef.implicitName

/** An implicit definition `ImplicitRef` that is visible under a different name, `alias`.
* Gets generated if an implicit ref is imported via a renaming import.
*/
class RenamedImplicitRef(val underlyingRef: TermRef, val alias: TermName) extends ImplicitRef {
def implicitName(using Context): TermName = alias
}
class RenamedImplicitRef(underlyingRef: TermRef, importInfo: ImportInfo, selector: Int, val alias: TermName)
extends ImportedImplicitRef(underlyingRef, importInfo, selector):
override def implicitName(using Context): TermName = alias

/** Both search candidates and successes are references with a specific nesting level. */
sealed trait RefAndLevel {
Expand Down Expand Up @@ -260,7 +265,9 @@ object Implicits:
refs.foreach(tryCandidate(extensionOnly = false))
candidates.toList
}
end filterMatching
}
end ImplicitRefs

/** The implicit references coming from the implicit scope of a type.
* @param tp the type determining the implicit scope
Expand Down Expand Up @@ -1117,8 +1124,12 @@ trait Implicits:
SearchFailure(adapted.withType(new MismatchedImplicit(ref, pt, argument)))
}
else
cand match
case Candidate(k: ImportedImplicitRef, _, _) => ctx.usages.use(k.importInfo, k.importInfo.selectors(k.selector))
case _ =>
SearchSuccess(adapted, ref, cand.level, cand.isExtension)(ctx.typerState, ctx.gadt)
}
end typedImplicit

/** An implicit search; parameters as in `inferImplicit` */
class ImplicitSearch(protected val pt: Type, protected val argument: Tree, span: Span)(using Context):
Expand Down Expand Up @@ -1237,6 +1248,7 @@ trait Implicits:
else if diff > 0 then alt1
else SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument), span)
case _: SearchFailure => alt2
end disambiguate

/** Try to find a best matching implicit term among all the candidates in `pending`.
* @param pending The list of candidates that remain to be tested
Expand Down Expand Up @@ -1306,6 +1318,7 @@ trait Implicits:
if (rfailures.isEmpty) found
else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize))
}
end rank

def negateIfNot(result: SearchResult) =
if (isNotGiven)
Expand Down
Loading

0 comments on commit d40f892

Please sign in to comment.