From e400c61c8b6e86172100700e9ed4f2aaa61fdb38 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 15 Feb 2022 10:47:11 -0800 Subject: [PATCH] Warn unused imports Handle language feature imports. Ignore language version imports. Skip Java sources. Support rewrite. --- compiler/src/dotty/tools/dotc/ast/untpd.scala | 2 + .../tools/dotc/config/ScalaSettings.scala | 9 +- .../src/dotty/tools/dotc/core/Contexts.scala | 122 ++++++++++++++++-- .../dotc/interactive/InteractiveDriver.scala | 2 +- .../tools/dotc/printing/PlainPrinter.scala | 12 +- .../dotty/tools/dotc/typer/Implicits.scala | 21 ++- .../dotty/tools/dotc/typer/ImportInfo.scala | 32 +++-- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 18 ++- .../dotty/tools/dotc/typer/TyperPhase.scala | 8 ++ .../dotty/tools/dotc/util/SourceFile.scala | 7 +- .../src/dotty/tools/dotc/util/Spans.scala | 4 + .../dotty/tools/vulpix/ParallelTesting.scala | 4 +- project/Build.scala | 6 + tests/neg/i13558.check | 8 +- tests/neg/unused-imports.check | 16 +++ tests/neg/unused-imports.scala | 63 +++++++++ 17 files changed, 291 insertions(+), 45 deletions(-) create mode 100644 tests/neg/unused-imports.check create mode 100644 tests/neg/unused-imports.scala diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index a769df537f1a..bf2599901f79 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 8a66b5abca8a..4880ec6373c5 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -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 @@ -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", @@ -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 diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 44446080c106..9bc1ef1190e7 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -6,6 +6,7 @@ import interfaces.CompilerCallback import Decorators._ import Periods._ import Names._ +import Flags.* import Phases._ import Types._ import Symbols._ @@ -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 @@ -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]() @@ -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 @@ -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 = { @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index 132ff162be61..622d497085b6 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -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 { diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index d3545f09b0e7..9e9e8a86094c 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -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 @@ -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 = diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 9a42babcd38e..be1d130a15db 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -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 { @@ -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 @@ -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): @@ -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 @@ -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) diff --git a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala index b5be2daf873b..2868155d0013 100644 --- a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala +++ b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala @@ -7,11 +7,13 @@ import core._ import printing.{Printer, Showable} import util.SimpleIdentityMap import Symbols._, Names._, Types._, Contexts._, StdNames._, Flags._ -import Implicits.RenamedImplicitRef +import Implicits.{ImportedImplicitRef, RenamedImplicitRef} import StdNames.nme import printing.Texts.Text import NameKinds.QualifiedName +import annotation.* + object ImportInfo { case class RootRef(refFn: () => TermRef, isPredef: Boolean = false) @@ -125,6 +127,11 @@ class ImportInfo(symf: Context ?=> Symbol, myWildcardBound = ctx.typer.importBound(selectors, isGiven = false) myWildcardBound + private def givenSelector = selectors.indexWhere(_.isGiven) + private def wildSelector = selectors.indexWhere(_.isWildcard) + private def selectorOf(name: Name) = selectors.indexWhere(_.name == name) + private def selectorOf(original: Name, rename: Name) = selectors.indexWhere(s => s.name == original && s.rename == rename) + /** The implicit references imported by this import clause */ def importedImplicits(using Context): List[ImplicitRef] = val pre = site @@ -140,10 +147,11 @@ class ImportInfo(symf: Context ?=> Symbol, || ctx.mode.is(Mode.FindHiddenImplicits) // consider both implicits and givens for error reporting || ref.symbol.is(Implicit) // a wildcard `_` import only pulls in implicits val bound = if isGivenImport then givenBound else wildcardBound - if isEligible && ref.denot.asSingleDenotation.matchesImportBound(bound) then ref :: Nil + val selector = if isGivenImport then givenSelector else wildSelector + if isEligible && ref.denot.asSingleDenotation.matchesImportBound(bound) then ImportedImplicitRef(ref, this, selector) :: Nil else Nil - else if renamed == ref.name then ref :: Nil - else RenamedImplicitRef(ref, renamed) :: Nil + else if renamed == ref.name then ImportedImplicitRef(ref, this, selectorOf(name)) :: Nil + else RenamedImplicitRef(ref, this, selectorOf(name, renamed), renamed) :: Nil } else for @@ -152,8 +160,8 @@ class ImportInfo(symf: Context ?=> Symbol, yield val original = reverseMapping(renamed).nn val ref = TermRef(pre, original, denot) - if renamed == original then ref - else RenamedImplicitRef(ref, renamed) + if renamed == original then ImportedImplicitRef(ref, this, selectorOf(original)) + else RenamedImplicitRef(ref, this, selectorOf(original, renamed), renamed) /** The root import symbol hidden by this symbol, or NoSymbol if no such symbol is hidden. * Note: this computation needs to work even for un-initialized import infos, and @@ -178,7 +186,7 @@ class ImportInfo(symf: Context ?=> Symbol, assert(myUnimported != null) myUnimported.uncheckedNN - private val isLanguageImport: Boolean = untpd.languageImport(qualifier).isDefined + val isLanguageImport: Boolean = untpd.languageImport(qualifier).isDefined private var myUnimported: Symbol | Null = _ @@ -194,11 +202,14 @@ class ImportInfo(symf: Context ?=> Symbol, def test(prefix: TermName, feature: TermName): Option[Boolean] = untpd.languageImport(qualifier) match case Some(`prefix`) => - if forwardMapping.contains(feature) then Some(true) + if forwardMapping.contains(feature) then + ctx.usages.use(this, selectors(selectorOf(feature))) + Some(true) else if excluded.contains(feature) then Some(false) else None case _ => None feature match + case _ if !isLanguageImport => None case QualifiedName(prefix, name) => test(prefix, name) case _ => test(EmptyTermName, feature) @@ -225,4 +236,9 @@ class ImportInfo(symf: Context ?=> Symbol, featureCache(feature).nn def toText(printer: Printer): Text = printer.toText(this) + + override def hashCode = qualifier.## + override def equals(other: Any) = other match + case that: ImportInfo => qualifier == that.qualifier + case _ => false } diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 3b9a9e1ac382..1dc505c773c6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -392,7 +392,7 @@ class Namer { typer: Typer => setDocstring(pkg, stat) ctx case imp: Import => - ctx.importContext(imp, createSymbol(imp)) + ctx.importContext(imp, createSymbol(imp), enteringSyms = true) case mdef: DefTree => val sym = createSymbol(mdef) enterSymbol(sym) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index f18bf47cfd53..f2918d382a51 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -50,6 +50,7 @@ import Nullables._ import NullOpsDecorator._ import scala.annotation.constructorOnly +import scala.util.chaining.given object Typer { @@ -268,11 +269,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if imp.importSym.isCompleting then report.warning(i"cyclic ${imp.importSym}, ignored", pos) NoType + end selection /** The type representing a named import with enclosing name when imported * from given `site` and `selectors`. */ - def namedImportRef(imp: ImportInfo)(using Context): Type = { + def namedImportRef(imp: ImportInfo)(using Context): Type = val termName = name.toTermName def recur(selectors: List[untpd.ImportSelector]): Type = selectors match case selector :: rest => @@ -287,7 +289,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if selector.name == termName then name else if name.isTypeName then selector.name.toTypeName else selector.name - checkUnambiguous(selection(imp, memberName, checkBounds = false)) + checkUnambiguous(selection(imp, memberName, checkBounds = false)).tap(res => if res.exists then ctx.usages.use(imp, selector)) else recur(rest) @@ -295,14 +297,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer NoType recur(imp.selectors) - } + end namedImportRef /** The type representing a wildcard import with enclosing name when imported * from given import info */ def wildImportRef(imp: ImportInfo)(using Context): Type = - if (imp.isWildcardImport && !imp.excluded.contains(name.toTermName) && name != nme.CONSTRUCTOR) - selection(imp, name, checkBounds = true) + if imp.isWildcardImport && !imp.excluded.contains(name.toTermName) && name != nme.CONSTRUCTOR then + selection(imp, name, checkBounds = true).tap(res => if res.exists then ctx.usages.use(imp, imp.selectors.find(_.isWildcard).get)) else NoType /** Is (some alternative of) the given predenotation `denot` @@ -426,6 +428,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else if (prevPrec.ordinal < PackageClause.ordinal) result = findRefRecur(found, PackageClause, ctx)(using ctx.outer) } + end if // isNewDefScope if result.exists then result else { // find import @@ -455,11 +458,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer } else loop(ctx)(using outer) } + end if // result.exists } + end loop // begin findRefRecur loop(NoContext) } + end findRefRecur findRefRecur(NoType, BindingPrec.NothingBound, NoContext) } @@ -930,7 +936,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val tag = withTag(defn.TypeTestClass.typeRef.appliedTo(pt, tpe)) .orElse(withTag(defn.ClassTagClass.typeRef.appliedTo(tpe))) .getOrElse(tree) - if tag.symbol.maybeOwner == defn.ClassTagClass && config.Feature.sourceVersion.isAtLeast(config.SourceVersion.future) then + if tag.symbol.maybeOwner == defn.ClassTagClass && sourceVersion.isAtLeast(future) then report.warning("Use of `scala.reflect.ClassTag` for type testing may be unsound. Consider using `scala.reflect.TypeTest` instead.", tree.srcPos) tag } diff --git a/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala b/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala index 71e1dc961c00..fa6e6ef3488d 100644 --- a/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala +++ b/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala @@ -55,6 +55,11 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase { JavaChecks.check(unit.tpdTree) } + def emitDiagnostics(using Context): Unit = + ctx.usages.unused.foreach((importInfo, owner, selector) => report.warning(s"Unused import", pos = selector.srcPos)) + def clearDiagnostics()(using Context): Unit = + ctx.usages.clear() + protected def discardAfterTyper(unit: CompilationUnit)(using Context): Boolean = unit.isJava || unit.suspended @@ -85,6 +90,9 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase { record("total trees after typer", ast.Trees.ntrees) unitContexts.foreach(javaCheck(using _)) // after typechecking to avoid cycles + unitContexts.foreach(emitDiagnostics(using _)) + clearDiagnostics() + val newUnits = unitContexts.map(_.compilationUnit).filterNot(discardAfterTyper) ctx.run.nn.checkSuspendedUnits(newUnits) newUnits diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala index 42d07869f74e..d9078745f8cb 100644 --- a/compiler/src/dotty/tools/dotc/util/SourceFile.scala +++ b/compiler/src/dotty/tools/dotc/util/SourceFile.scala @@ -11,7 +11,6 @@ import core.Contexts._ import scala.io.Codec import Chars._ import scala.annotation.internal.sharable -import scala.collection.mutable import scala.collection.mutable.ArrayBuffer import scala.util.chaining.given @@ -19,7 +18,6 @@ import java.io.File.separator import java.nio.charset.StandardCharsets import java.nio.file.{FileSystemException, NoSuchFileException} import java.util.Optional -import java.util.concurrent.atomic.AtomicInteger import java.util.regex.Pattern object ScriptSourceFile { @@ -60,7 +58,6 @@ object ScriptSourceFile { } class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends interfaces.SourceFile { - import SourceFile._ private var myContent: Array[Char] | Null = null @@ -191,6 +188,10 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends def lineContent(offset: Int): String = content.slice(startOfLine(offset), nextLine(offset)).mkString + /** The span of the line containing position `offset` */ + def lineSpan(offset: Int): Span = + Spans.Span(startOfLine(offset), nextLine(offset)) + /** The column corresponding to `offset`, starting at 0 */ def column(offset: Int): Int = { var idx = startOfLine(offset) diff --git a/compiler/src/dotty/tools/dotc/util/Spans.scala b/compiler/src/dotty/tools/dotc/util/Spans.scala index baf2cfa121b0..74621b2d77af 100644 --- a/compiler/src/dotty/tools/dotc/util/Spans.scala +++ b/compiler/src/dotty/tools/dotc/util/Spans.scala @@ -144,6 +144,10 @@ object Spans { def ==(that: Span): Boolean = this.coords == that.coords def !=(that: Span): Boolean = this.coords != that.coords } + object Span: + extension (span: Span) + def spread: Int = span.end - span.start + end Span private def fromOffsets(start: Int, end: Int, pointDelta: Int) = //assert(start <= end || start == 1 && end == 0, s"$start..$end") diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 48175f5236ab..5727cd7396ce 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -768,8 +768,8 @@ trait ParallelTesting extends RunnerOrchestration { self => lazy val actualErrors = reporters.foldLeft(0)(_ + _.errorCount) lazy val (expected, unexpected) = getMissingExpectedErrors(errorMap, reporters.iterator.flatMap(_.errors)) def hasMissingAnnotations = expected.nonEmpty || unexpected.nonEmpty - def showErrors = "-> following the errors:\n" + - reporters.flatMap(_.allErrors.sortBy(_.pos.line).map(e => s"${e.pos.line + 1}: ${e.message}")).mkString(" at ", "\n at ", "") + def showErrors = "Reported errors:\n" + + reporters.flatMap(_.allErrors.sortBy(_.pos.line).map(e => s"${e.pos.line + 1}: ${e.message.linesIterator.mkString("\\")}")).mkString(" at ", "\n at ", "") Option { if compilerCrashed then s"Compiler crashed when compiling: ${testSource.title}" diff --git a/project/Build.scala b/project/Build.scala index 30aa05b9ee5d..935157c95042 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -170,6 +170,8 @@ object Build { "-deprecation", "-unchecked", "-Xfatal-warnings", + //"-Werror", + //"-Wunused:imports", "-encoding", "UTF8", "-language:implicitConversions" ), @@ -764,6 +766,10 @@ object Build { "-Ddotty.tests.classes.dottyTastyInspector=" + jars("scala3-tasty-inspector"), ) }, + //scalacOptions ++= Seq( + // "-Wunused:imports", + // "-rewrite", + //), packageAll := { (`scala3-compiler` / packageAll).value ++ Seq( "scala3-compiler" -> (Compile / packageBin).value.getAbsolutePath, diff --git a/tests/neg/i13558.check b/tests/neg/i13558.check index 4c468a854781..6e25587902d5 100644 --- a/tests/neg/i13558.check +++ b/tests/neg/i13558.check @@ -7,8 +7,8 @@ | testcode.ExtensionA.id(a) failed with | | Reference to id is ambiguous, - | it is both imported by import testcode.ExtensionB._ - | and imported subsequently by import testcode.ExtensionA._ + | it is both imported by import testcode.ExtensionB.* + | and imported subsequently by import testcode.ExtensionA.* -- [E008] Not Found Error: tests/neg/i13558.scala:29:14 ---------------------------------------------------------------- 29 | println(a.id) // error | ^^^^ @@ -18,5 +18,5 @@ | testcode.ExtensionB.id(a) failed with | | Reference to id is ambiguous, - | it is both imported by import testcode.ExtensionA._ - | and imported subsequently by import testcode.ExtensionB._ + | it is both imported by import testcode.ExtensionA.* + | and imported subsequently by import testcode.ExtensionB.* diff --git a/tests/neg/unused-imports.check b/tests/neg/unused-imports.check new file mode 100644 index 000000000000..2ac1495f5011 --- /dev/null +++ b/tests/neg/unused-imports.check @@ -0,0 +1,16 @@ +-- Error: tests/neg/unused-imports.scala:5:16 -------------------------------------------------------------------------- +5 |import language.postfixOps // error + | ^^^^^^^^^^ + | Unused import +-- Error: tests/neg/unused-imports.scala:6:24 -------------------------------------------------------------------------- +6 |import scala.concurrent.* // error + | ^ + | Unused import +-- Error: tests/neg/unused-imports.scala:11:43 ------------------------------------------------------------------------- +11 | import scala.collection.mutable.{HashMap as GoodMap, Seq as _, ListBuffer, Buffer, Set as OK} // error // error + | ^^^^^^^^^^^^^^^^^^ + | Unused import +-- Error: tests/neg/unused-imports.scala:11:77 ------------------------------------------------------------------------- +11 | import scala.collection.mutable.{HashMap as GoodMap, Seq as _, ListBuffer, Buffer, Set as OK} // error // error + | ^^^^^^ + | Unused import diff --git a/tests/neg/unused-imports.scala b/tests/neg/unused-imports.scala new file mode 100644 index 000000000000..1e5c2418df87 --- /dev/null +++ b/tests/neg/unused-imports.scala @@ -0,0 +1,63 @@ +// scalac: -Wunused:imports -Werror -feature + +import language.future +import language.implicitConversions +import language.postfixOps // error +import scala.concurrent.* // error +import scala.concurrent.ExecutionContext.Implicits.* + +class C: + def c = 42 + import scala.collection.mutable.{HashMap as GoodMap, Seq as _, ListBuffer, Buffer, Set as OK} // error // error + + def buf = ListBuffer.empty[String] + def ok: OK[Int] = ??? + def f = concurrent.Future(42) // implicit usage + + import Thread.* + import State.{NEW, BLOCKED} + + def state(t: Thread) = + t.getState match + case NEW => + import State.RUNNABLE + t.getState match + case RUNNABLE => 0 + case BLOCKED => 1 + case _ => -1 + case _ => -1 + + enum E: + case E0, E1 + + def e(x: E) = + x match + case E.E0 => "e0" + case E.E1 => "e1" + + locally { + import Givens.{*, given} + + def g(s: String)(using Ordering[C]) = ??? + def ordered = g(greeting) + } + locally { + import Givens.{cOrdering, *} + + def g(s: String)(using Ordering[C]) = ??? + def ordered = g(greeting) + } + +object Givens: + given cOrdering: Ordering[C] with + override def compare(c0: C, c1: C) = 0 + val greeting = "we love Givens" + +class A: + def f(b: B): Unit = b.f(this) + +object A: + implicit val a2b: Conversion[A, B] = _ => B() + +class B: + def f(b: B): Unit = ()