Skip to content

Commit

Permalink
Warn unused imports
Browse files Browse the repository at this point in the history
Handle language feature imports.
Ignore language version imports.
Skip Java sources.
Support rewrite.
  • Loading branch information
som-snytt committed Jun 23, 2022
1 parent f92ab11 commit e400c61
Show file tree
Hide file tree
Showing 17 changed files with 291 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 @@ -135,6 +135,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
9 changes: 6 additions & 3 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import scala.language.unsafeNulls
import dotty.tools.dotc.config.PathResolver.Defaults
import dotty.tools.dotc.config.Settings.{Setting, SettingGroup}
import dotty.tools.dotc.config.SourceVersion
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 @@ -161,12 +161,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 Expand Up @@ -337,5 +338,7 @@ private sealed trait YSettings:
val YinstrumentDefs: Setting[Boolean] = BooleanSetting("-Yinstrument-defs", "Add instrumentation code that counts method calls; needs -Yinstrument to be set, too.")

val YforceInlineWhileTyping: Setting[Boolean] = BooleanSetting("-Yforce-inline-while-typing", "Make non-transparent inline methods inline when typing. Emulates the old inlining behavior of 3.0.0-M3.")

val YrewriteImports: Setting[Boolean] = BooleanSetting("-Yrewrite-imports", "Rewrite unused imports. Requires -Wunused:imports.")
end YSettings

122 changes: 112 additions & 10 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 @@ -19,12 +20,14 @@ import Nullables._
import Implicits.ContextualImplicits
import config.Settings._
import config.Config
import config.SourceVersion.allSourceVersionNames
import reporting._
import io.{AbstractFile, NoAbstractFile, PlainFile, Path}
import scala.io.Codec
import collection.mutable
import parsing.Parsers
import printing._
import config.{JavaPlatform, SJSPlatform, Platform, ScalaSettings}
import config.{JavaPlatform, SJSPlatform, Platform, ScalaSettings, ScalaRelease}
import classfile.ReusableDataReader
import StdNames.nme

Expand All @@ -39,7 +42,9 @@ import plugins._
import java.util.concurrent.atomic.AtomicInteger
import java.nio.file.InvalidPathException

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 @@ -51,8 +56,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]()

private val initialStore = store10
private val initialStore = store11

/** The current context */
inline def ctx(using ctx: Context): Context = ctx
Expand Down Expand Up @@ -238,6 +244,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 @@ -246,9 +255,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 @@ -474,8 +481,24 @@ 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
if releaseName.nonEmpty then ScalaRelease.parse(releaseName).get else ScalaRelease.latest

def tastyVersion: TastyVersion =
import math.Ordered.orderingToOrdered
val latestRelease = ScalaRelease.latest
val specifiedRelease = scalaRelease
if specifiedRelease < latestRelease then
// This is needed to make -scala-output-version a no-op when set to the latest release for unstable versions of the compiler
// (which might have the tasty format version numbers set to higher values before they're decreased during a release)
TastyVersion.fromStableScalaRelease(specifiedRelease.majorVersion, specifiedRelease.minorVersion)
else
TastyVersion.compilerVersion
>>>>>>> Warn unused imports

/** Is the debug option set? */
def debug: Boolean = base.settings.Ydebug.value
Expand Down Expand Up @@ -811,6 +834,7 @@ object Contexts {
store = initialStore
.updated(settingsStateLoc, settingsGroup.defaultState)
.updated(notNullInfosLoc, Nil)
.updated(usagesLoc, Usages())
.updated(compilationUnitLoc, NoCompilationUnit)
searchHistory = new SearchRoot
gadt = EmptyGadtConstraint
Expand Down Expand Up @@ -938,7 +962,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 @@ -995,4 +1019,82 @@ 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 =
def isLanguageImport = info.isLanguageImport && allSourceVersionNames.exists(info.forwardMapping.contains)
if ctx.settings.WunusedHas.imports && !isLanguageImport && !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)
if needsPatch && ctx.settings.YrewriteImports.value then
val src = ctx.compilationUnit.source
val infoPos = info.qualifier.sourcePos
val lineSource = SourceFile.virtual(name = "import-line.scala", content = infoPos.lineContent)
val PackageDef(_, pieces) = Parsers.Parser(lineSource).parse(): @unchecked
// patch if there's just one import on the line, i.e., not import a.b, c.d
if pieces.length == 1 then
val retained = info.selectors.filter(sel => sel.isMask || used(sel))
val selectorSpan = info.selectors.map(_.span).reduce(_ union _)
val lineSpan = src.lineSpan(infoPos.start)
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
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 @@ -1136,8 +1143,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 @@ -1258,6 +1269,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 @@ -1327,6 +1339,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 e400c61

Please sign in to comment.