From 8ee1dcd4c2f06cf9c3ca464a086b3f3da01a49c2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 15 Feb 2022 10:47:11 -0800 Subject: [PATCH 1/4] 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 | 74 +++++++++++++-- .../dotc/interactive/InteractiveDriver.scala | 2 +- .../dotty/tools/dotc/parsing/Parsers.scala | 12 ++- .../tools/dotc/printing/PlainPrinter.scala | 14 ++- .../dotty/tools/dotc/typer/Implicits.scala | 21 ++++- .../dotty/tools/dotc/typer/ImportInfo.scala | 39 ++++++-- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 24 +++-- .../dotty/tools/dotc/typer/TyperPhase.scala | 91 +++++++++++++++++++ .../dotty/tools/dotc/util/SourceFile.scala | 7 +- .../tools/dotc/util/SourcePosition.scala | 6 ++ .../src/dotty/tools/dotc/util/Spans.scala | 4 + .../dotty/tools/dotc/CompilationTests.scala | 2 + .../dotty/tools/vulpix/ParallelTesting.scala | 4 +- project/Build.scala | 7 +- tests/neg/i13558.check | 8 +- tests/neg/unused-imports.check | 40 ++++++++ tests/neg/unused-imports.scala | 76 ++++++++++++++++ tests/rewrites/unused-imports-stylized.check | 84 +++++++++++++++++ tests/rewrites/unused-imports-stylized.scala | 88 ++++++++++++++++++ tests/rewrites/unused-imports.check | 74 +++++++++++++++ tests/rewrites/unused-imports.scala | 76 ++++++++++++++++ 24 files changed, 710 insertions(+), 56 deletions(-) create mode 100644 tests/neg/unused-imports.check create mode 100644 tests/neg/unused-imports.scala create mode 100644 tests/rewrites/unused-imports-stylized.check create mode 100644 tests/rewrites/unused-imports-stylized.scala create mode 100644 tests/rewrites/unused-imports.check create mode 100644 tests/rewrites/unused-imports.scala diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index f72cafd4205d..3bfee17f8f3d 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 f7743dddda4e..f80127fa0ded 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 @@ -162,12 +162,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", @@ -343,5 +344,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 a6c1a24ebf96..bdcffecdbf9b 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._ @@ -20,14 +21,17 @@ 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 printing._ -import config.{JavaPlatform, SJSPlatform, Platform, ScalaSettings} +import config.{JavaPlatform, SJSPlatform, Platform, ScalaSettings, ScalaRelease} import classfile.ReusableDataReader import StdNames.nme +import parsing.Parsers.EnclosingSpan +import util.Spans.NoSpan import scala.annotation.internal.sharable @@ -40,7 +44,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]() @@ -52,8 +58,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 @@ -239,6 +246,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 = { @@ -247,9 +257,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 @@ -475,8 +483,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, imp.attachmentOrElse(EnclosingSpan, NoSpan)).tap(importInfo => if enteringSyms && ctx.settings.WunusedHas.imports then usages += importInfo)) /** Is the debug option set? */ def debug: Boolean = base.settings.Ydebug.value @@ -812,6 +820,7 @@ object Contexts { store = initialStore .updated(settingsStateLoc, settingsGroup.defaultState) .updated(notNullInfosLoc, Nil) + .updated(usagesLoc, Usages()) .updated(compilationUnitLoc, NoCompilationUnit) searchHistory = new SearchRoot gadt = GadtConstraint.empty @@ -939,7 +948,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 @@ -996,4 +1005,49 @@ 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: + 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, List[untpd.ImportSelector])] = + if ctx.settings.WunusedHas.imports && !ctx.compilationUnit.isJava then + var unusages = List.empty[(ImportInfo, Symbol, List[untpd.ImportSelector])] + def checkUsed(info: ImportInfo, owner: Symbol): Unit = + val usedSelectors = selectors.remove(info).getOrElse(Set.empty) + var unusedSelectors = List.empty[untpd.ImportSelector] + def cull(toCheck: List[untpd.ImportSelector]): Unit = + toCheck match + case selector :: rest => + cull(rest) // reverse + if !selector.isMask && !usedSelectors(selector) then + unusedSelectors ::= selector + case _ => + cull(info.selectors) + if unusedSelectors.nonEmpty then unusages ::= (info, owner, unusedSelectors) + end checkUsed + importInfos.remove(ctx.compilationUnit).foreach(_.foreach(checkUsed)) + unusages + else + Nil + end unused + + 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/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index fcaa51dff483..ac4641bd084b 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -7,7 +7,7 @@ import scala.language.unsafeNulls import scala.annotation.internal.sharable import scala.collection.mutable.ListBuffer import scala.collection.immutable.BitSet -import util.{ SourceFile, SourcePosition, NoSourcePosition } +import util.{Property, SourceFile, SourcePosition, NoSourcePosition} import Tokens._ import Scanners._ import xml.MarkupParsers.MarkupParser @@ -64,6 +64,8 @@ object Parsers { val QuotedPattern = 1 << 2 } + val EnclosingSpan: Property.Key[Span] = Property.Key() + extension (buf: ListBuffer[Tree]) def +++=(x: Tree) = x match { case x: Thicket => buf ++= x.trees @@ -3283,7 +3285,7 @@ object Parsers { /** Import ::= `import' ImportExpr {‘,’ ImportExpr} * Export ::= `export' ImportExpr {‘,’ ImportExpr} */ - def importOrExportClause(leading: Token, mkTree: ImportConstr): List[Tree] = { + def importOrExportClause(leading: Token, mkTree: ImportConstr): List[Tree] = val offset = accept(leading) commaSeparated(importExpr(mkTree)) match { case t :: rest => @@ -3291,10 +3293,12 @@ object Parsers { val firstPos = if (t.span.exists) t.span.withStart(offset) else Span(offset, in.lastOffset) - t.withSpan(firstPos) :: rest + val imports = t.withSpan(firstPos) :: rest + val enclosing = imports.head.span union imports.last.span + imports.foreach(_.putAttachment(EnclosingSpan, enclosing)) + imports case nil => nil } - } def exportClause() = importOrExportClause(EXPORT, Export(_,_)) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index c9c64f458ad0..9f6b8a89082c 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 @@ -648,12 +649,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 exprStr = siteStr.stripSuffix(".type") 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 0400d241e367..9218d69a9ac4 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 @@ -1150,8 +1157,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): @@ -1272,6 +1283,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 @@ -1341,6 +1353,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..5815e9e6890a 100644 --- a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala +++ b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala @@ -6,12 +6,15 @@ import ast.{tpd, untpd} import core._ import printing.{Printer, Showable} import util.SimpleIdentityMap +import util.Spans.* 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) @@ -30,7 +33,7 @@ object ImportInfo { val expr = tpd.Ident(ref.refFn()) // refFn must be called in the context of ImportInfo.sym tpd.Import(expr, selectors).symbol - ImportInfo(sym, selectors, untpd.EmptyTree, isRootImport = true) + ImportInfo(sym, selectors, untpd.EmptyTree, NoSpan, isRootImport = true) extension (c: Context) def withRootImports(rootRefs: List[RootRef])(using Context): Context = @@ -46,12 +49,14 @@ object ImportInfo { * @param selectors The selector clauses * @param qualifier The import qualifier, or EmptyTree for root imports. * Defined for all explicit imports from ident or select nodes. + * @param enclosingSpan Span of the enclosing import statement * @param isRootImport true if this is one of the implicit imports of scala, java.lang, * scala.Predef in the start context, false otherwise. */ class ImportInfo(symf: Context ?=> Symbol, val selectors: List[untpd.ImportSelector], val qualifier: untpd.Tree, + val enclosingSpan: Span, val isRootImport: Boolean = false) extends Showable { private def symNameOpt = qualifier match { @@ -125,6 +130,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 +150,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 +163,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 +189,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 +205,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 +239,11 @@ class ImportInfo(symf: Context ?=> Symbol, featureCache(feature).nn def toText(printer: Printer): Text = printer.toText(this) + + def copy(selectors: List[untpd.ImportSelector]): ImportInfo = ImportInfo(symf, selectors = selectors, qualifier, enclosingSpan, isRootImport) + + 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 ad8d0e50d348..4600d10fb932 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -393,7 +393,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 33638df54fb1..566a69e57835 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -53,6 +53,7 @@ import cc.CheckCaptures import config.Config import scala.annotation.constructorOnly +import scala.util.chaining.given object Typer { @@ -210,7 +211,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer * @param prevCtx The context of the previous denotation, * or else `NoContext` if nothing was found yet. */ - def findRefRecur(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type = { + def findRefRecur(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type = import BindingPrec._ /** Check that any previously found result from an inner context @@ -272,11 +273,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 => @@ -291,7 +293,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) @@ -299,14 +301,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` @@ -430,9 +432,10 @@ 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 + // result or find import + result orElse { val outer = ctx.outer val curImport = ctx.importInfo def updateUnimported() = @@ -460,10 +463,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else loop(ctx)(using outer) } } + end loop // begin findRefRecur loop(NoContext) - } + end findRefRecur findRefRecur(NoType, BindingPrec.NothingBound, NoContext) } @@ -937,7 +941,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 60f0c043b435..ae02100d3879 100644 --- a/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala +++ b/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala @@ -59,6 +59,94 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase { JavaChecks.check(unit.tpdTree) } + /** Report unused imports. + * + * If `-Yrewrite-imports`, emit patches instead. + * Patches are applied if `-rewrite` and no errors. + */ + def emitDiagnostics(using Context): Unit = + import ast.Trees.* + import ast.untpd + import parsing.Parsers + import rewrites.Rewrites.patch + import util.SourceFile + import Decorators.* + def reportSelectors(sels: List[untpd.ImportSelector]) = sels.foreach(sel => report.warning(s"Unused import", pos = sel.srcPos)) + // format the import clause, qual.{ selectors } + def toText(qual: untpd.Tree, sels: List[untpd.ImportSelector]): String = + def selected(sel: untpd.ImportSelector) = + if sel.isGiven then + if sel.bound.isEmpty then "given" else "given " + sel.bound.show + 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 selections = sels.map(selected) + if sels.length > 1 then selections.mkString(i"$qual.{", ", ", "}") else i"$qual.${selections.head}" + end toText + // begin + val unused = ctx.usages.unused //ImportInfo, owner Symbol, unused List[untpd.ImportSelector] + if !ctx.settings.YrewriteImports.value then + unused.foreach { (info, owner, selectors) => reportSelectors(selectors) } + else if unused.nonEmpty then + val byLocation = unused.groupBy((info, owner, selectors) => info.qualifier.sourcePos.withSpan(info.enclosingSpan)) + byLocation.foreach { (enclosingPos, grouped) => + val importText = enclosingPos.spanText + val lineSource = SourceFile.virtual(name = "import-line.scala", content = importText) + val PackageDef(_, clauses) = Parsers.Parser(lineSource).parse(): @unchecked + val edited = + clauses.map { + case importTree @ Import(pqual, pselectors) => + //val importPrefix = raw"import\s+".r + //val prologue = importPrefix.findPrefixMatchOf(importText).map(_.end).getOrElse(0) + // uncorrupt span of first clause, which starts at keyword but really starts at point + val span = + if importTree.span.point != importTree.span.start then importTree.span.withStart(importTree.span.point) + else importTree.span + val zone = span.shift(enclosingPos.span.start) + grouped.find { (info, _, _) => zone.contains(info.qualifier.span) } match + case Some((_, _, selectors)) => + val retained = pselectors.filterNot(psel => selectors.exists(_.sameTree(psel))) + (zone, pqual, retained, true) + case _ => + (zone, pqual, pselectors, false) + } + val src = ctx.compilationUnit.source + // Simple deletion, or replace import statement with nonempty clauses; or if multiline, patch the emended bits + if edited.forall(_._3.isEmpty) then + val spanToDelete = + val lines = enclosingPos.lineContent.linesIterator + val line = lines.next() + val (gutter, text) = line.splitAt(enclosingPos.startColumn) + if !lines.hasNext && gutter.forall(Character.isWhitespace) && text == importText then + enclosingPos.source.lineSpan(enclosingPos.point) + else enclosingPos.span + patch(src, spanToDelete, "") + else if !importText.contains('\n') then + val patched = edited.flatMap { + case (_, qual, retained, _) if retained.nonEmpty => Some(toText(qual, retained)) + case _ => None + }.mkString("import ", ", ", "") + patch(src, enclosingPos.span, patched) + else + edited.zipWithIndex.foreach { + case ((span, qual, retained, emended), index) => + if emended then + if retained.isEmpty then + // adjust span to include a comma + val adjustedSpan = + if index == edited.length - 1 then span.withStart(edited(index - 1)._1.end) // TODO don't overlap + else span.withEnd(edited(1)._1.start) + patch(src, adjustedSpan, "") + else + patch(src, span, toText(qual, retained)) + } + end if + } + end emitDiagnostics + + def clearDiagnostics()(using Context): Unit = + ctx.usages.clear() + protected def discardAfterTyper(unit: CompilationUnit)(using Context): Boolean = unit.isJava || unit.suspended @@ -89,6 +177,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/SourcePosition.scala b/compiler/src/dotty/tools/dotc/util/SourcePosition.scala index 29f9a34d2292..d6f273a1661c 100644 --- a/compiler/src/dotty/tools/dotc/util/SourcePosition.scala +++ b/compiler/src/dotty/tools/dotc/util/SourcePosition.scala @@ -33,6 +33,12 @@ extends SrcPos, interfaces.SourcePosition, Showable { def linesSlice: Array[Char] = source.content.slice(source.startOfLine(start), source.nextLine(end)) + /** Extract exactly the span from the source file. */ + def spanSlice: Array[Char] = source.content.slice(start, end) + + /** Extract exactly the span from the source file as a String. */ + def spanText: String = String(spanSlice) + /** The lines of the position */ def lines: Range = { val startOffset = source.offsetToLine(start) 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/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index c1b465ad4a88..a472e94867aa 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -83,6 +83,8 @@ class CompilationTests { compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")), compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")), compileFile("tests/rewrites/i12340.scala", unindentOptions.and("-rewrite")), + compileFile("tests/rewrites/unused-imports.scala", defaultOptions), + compileFile("tests/rewrites/unused-imports-stylized.scala", defaultOptions), ).checkRewrites() } diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index b64142c0021f..ad873219ede0 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -774,8 +774,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 d735db6a8a5b..9fcced73f519 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -193,7 +193,7 @@ object Build { "-deprecation", "-unchecked", //"-Wconf:cat=deprecation&msg=Unsafe:s", // example usage - "-Xfatal-warnings", // -Werror in modern usage + "-Werror", "-encoding", "UTF8", "-language:implicitConversions", ), @@ -791,6 +791,11 @@ object Build { "-Ddotty.tests.classes.dottyTastyInspector=" + jars("scala3-tasty-inspector"), ) }, + scalacOptions ++= Seq( + //"-Wunused:imports", + //"-rewrite", + //"-Yrewrite-imports", + ), 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 ab10a42cdd32..6f608254a066 100644 --- a/tests/neg/i13558.check +++ b/tests/neg/i13558.check @@ -9,8 +9,8 @@ | 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 | ^^^^ @@ -22,5 +22,5 @@ | 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..e0a59bd1c3d4 --- /dev/null +++ b/tests/neg/unused-imports.check @@ -0,0 +1,40 @@ +-- 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 +-- Error: tests/neg/unused-imports.scala:51:38 ------------------------------------------------------------------------- +51 | import Givens.{given Ordering[C], *}, E.{given, *} // error // error // error + | ^ + | Unused import +-- Error: tests/neg/unused-imports.scala:51:45 ------------------------------------------------------------------------- +51 | import Givens.{given Ordering[C], *}, E.{given, *} // error // error // error + | ^^^^^ + | Unused import +-- Error: tests/neg/unused-imports.scala:51:52 ------------------------------------------------------------------------- +51 | import Givens.{given Ordering[C], *}, E.{given, *} // error // error // error + | ^ + | Unused import +-- Error: tests/neg/unused-imports.scala:57:14 ------------------------------------------------------------------------- +57 | import E.{given, *}, Givens.{given Ordering[C], *} // error // error // error + | ^^^^^ + | Unused import +-- Error: tests/neg/unused-imports.scala:57:21 ------------------------------------------------------------------------- +57 | import E.{given, *}, Givens.{given Ordering[C], *} // error // error // error + | ^ + | Unused import +-- Error: tests/neg/unused-imports.scala:57:52 ------------------------------------------------------------------------- +57 | import E.{given, *}, Givens.{given Ordering[C], *} // error // error // error + | ^ + | Unused import diff --git a/tests/neg/unused-imports.scala b/tests/neg/unused-imports.scala new file mode 100644 index 000000000000..f41f75b1f6d8 --- /dev/null +++ b/tests/neg/unused-imports.scala @@ -0,0 +1,76 @@ +// 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) + } + locally { + import Givens.{given Ordering[C], *}, E.{given, *} // error // error // error + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } + locally { + import E.{given, *}, Givens.{given Ordering[C], *} // error // error // error + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } +end C + +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 = () diff --git a/tests/rewrites/unused-imports-stylized.check b/tests/rewrites/unused-imports-stylized.check new file mode 100644 index 000000000000..be74b34a1f4f --- /dev/null +++ b/tests/rewrites/unused-imports-stylized.check @@ -0,0 +1,84 @@ +// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports + +import language.{ future, + implicitConversions, + postfixOps, + } +import scala.concurrent.*, ExecutionContext.Implicits.* + +class C: + def c = 42 + import collection.mutable, mutable.ListBuffer, mutable.{Seq as _, Set as OK} + + 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) + } + locally { + import Givens.given Ordering[C] + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } + locally { + import + Givens.given Ordering[C] + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } +end C + +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 = () diff --git a/tests/rewrites/unused-imports-stylized.scala b/tests/rewrites/unused-imports-stylized.scala new file mode 100644 index 000000000000..095eadc29f7f --- /dev/null +++ b/tests/rewrites/unused-imports-stylized.scala @@ -0,0 +1,88 @@ +// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports + +import language.{ future, + implicitConversions, + postfixOps, + } +import scala.concurrent.*, ExecutionContext.Implicits.* + +class C: + def c = 42 + import collection.mutable, mutable.ListBuffer, mutable.{HashMap as GoodMap, Seq as _, Buffer, Set as OK} + + 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) + } + locally { + import Givens + .{given Ordering[C], *}, + E + .{given, *} + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } + locally { + import + E.{given, *}, + Givens.{given Ordering[C], *} + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } +end C + +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 = () diff --git a/tests/rewrites/unused-imports.check b/tests/rewrites/unused-imports.check new file mode 100644 index 000000000000..a0a3025827fb --- /dev/null +++ b/tests/rewrites/unused-imports.check @@ -0,0 +1,74 @@ +// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports + +import language.future +import language.implicitConversions +import scala.concurrent.ExecutionContext.Implicits.* + +class C: + def c = 42 + import scala.collection.mutable.{Seq as _, ListBuffer, 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) + } + locally { + import Givens.given Ordering[C] + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } + locally { + import Givens.given Ordering[C] + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } +end C + +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 = () diff --git a/tests/rewrites/unused-imports.scala b/tests/rewrites/unused-imports.scala new file mode 100644 index 000000000000..66f022f476b3 --- /dev/null +++ b/tests/rewrites/unused-imports.scala @@ -0,0 +1,76 @@ +// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports + +import language.future +import language.implicitConversions +import language.postfixOps +import scala.concurrent.* +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) + } + locally { + import Givens.{given Ordering[C], *}, E.{given, *} + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } + locally { + import E.{given, *}, Givens.{given Ordering[C], *} + + def g(s: String)(using Ordering[C]) = ??? + println(g("")) + } +end C + +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 = () From 1072aa960e9ecf7ba0b2ec4ba671a9b3da4512bb Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 10 Nov 2022 09:00:58 -0800 Subject: [PATCH 2/4] Fix rewrite check --- tests/rewrites/unused-imports-stylized.check | 12 ++++-------- tests/rewrites/unused-imports.check | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/tests/rewrites/unused-imports-stylized.check b/tests/rewrites/unused-imports-stylized.check index be74b34a1f4f..711d32913aee 100644 --- a/tests/rewrites/unused-imports-stylized.check +++ b/tests/rewrites/unused-imports-stylized.check @@ -37,7 +37,7 @@ class C: case E.E0 => "e0" case E.E1 => "e1" - locally { + locally: import Givens.{ *, given @@ -45,28 +45,24 @@ class C: def g(s: String)(using Ordering[C]) = ??? def ordered = g(greeting) - } - locally { + locally: import Givens.{ cOrdering, *} def g(s: String)(using Ordering[C]) = ??? def ordered = g(greeting) - } - locally { + locally: import Givens.given Ordering[C] def g(s: String)(using Ordering[C]) = ??? println(g("")) - } - locally { + locally: import Givens.given Ordering[C] def g(s: String)(using Ordering[C]) = ??? println(g("")) - } end C object Givens: diff --git a/tests/rewrites/unused-imports.check b/tests/rewrites/unused-imports.check index a0a3025827fb..83597f7889f4 100644 --- a/tests/rewrites/unused-imports.check +++ b/tests/rewrites/unused-imports.check @@ -33,30 +33,26 @@ class C: case E.E0 => "e0" case E.E1 => "e1" - locally { + locally: import Givens.{*, given} def g(s: String)(using Ordering[C]) = ??? def ordered = g(greeting) - } - locally { + locally: import Givens.{cOrdering, *} def g(s: String)(using Ordering[C]) = ??? def ordered = g(greeting) - } - locally { + locally: import Givens.given Ordering[C] def g(s: String)(using Ordering[C]) = ??? println(g("")) - } - locally { + locally: import Givens.given Ordering[C] def g(s: String)(using Ordering[C]) = ??? println(g("")) - } end C object Givens: From 3bbd9e3d97c37b8d239446a24e71b1a96ef587b0 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 10 Nov 2022 11:36:49 -0800 Subject: [PATCH 3/4] Mark import used after implicit ranking chooses it --- .../src/dotty/tools/dotc/core/Types.scala | 4 ++-- .../dotty/tools/dotc/typer/Implicits.scala | 20 +++++++++++++++---- .../dotty/tools/dotc/typer/ImportInfo.scala | 2 ++ tests/neg/t12690a.scala | 16 +++++++++++++++ tests/neg/t12690b.scala | 16 +++++++++++++++ 5 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 tests/neg/t12690a.scala create mode 100644 tests/neg/t12690b.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 9a8e2a051ad4..3eda3ad7f690 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2736,8 +2736,8 @@ object Types { override def eql(that: Type): Boolean = this eq that // safe because named types are hash-consed separately } - /** A reference to an implicit definition. This can be either a TermRef or a - * Implicits.RenamedImplicitRef. + /** A reference to an implicit definition. This can be either a TermRef or + * an Implicits.{ImportedImplicitRef, RenamedImplicitRef}. */ trait ImplicitRef { def implicitName(using Context): TermName diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 9218d69a9ac4..73f6fdf6283b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -44,6 +44,7 @@ object Implicits: */ class ImportedImplicitRef(val underlyingRef: TermRef, val importInfo: ImportInfo, val selector: Int) extends ImplicitRef: def implicitName(using Context): TermName = underlyingRef.implicitName + override def toString = s"ImportedImplicitRef($underlyingRef, $importInfo, selector=$selector)" /** An implicit definition `ImplicitRef` that is visible under a different name, `alias`. * Gets generated if an implicit ref is imported via a renaming import. @@ -51,6 +52,7 @@ object Implicits: class RenamedImplicitRef(underlyingRef: TermRef, importInfo: ImportInfo, selector: Int, val alias: TermName) extends ImportedImplicitRef(underlyingRef, importInfo, selector): override def implicitName(using Context): TermName = alias + override def toString = s"ImportedImplicitRef($underlyingRef, $importInfo, selector=$selector, alias=$alias)" /** Both search candidates and successes are references with a specific nesting level. */ sealed trait RefAndLevel { @@ -1157,9 +1159,6 @@ 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 @@ -1485,7 +1484,20 @@ trait Implicits: validateOrdering(ord) throw ex - rank(sort(eligible), NoMatchingImplicitsFailure, Nil) + @tailrec + def markSelector(allEligible: List[Candidate], foundRef: ImplicitRef): Unit = + allEligible match + case Candidate(k: ImportedImplicitRef, _, _) :: _ if k.underlyingRef == foundRef => + ctx.usages.use(k.importInfo, k.importInfo.selectors(k.selector)) + case _ :: rest => markSelector(rest, foundRef) + case nil => () + + rank(sort(eligible), NoMatchingImplicitsFailure, Nil) match { + case res: SearchSuccess if !ctx.mode.is(Mode.ImplicitExploration) => + markSelector(eligible, res.ref) + res + case res => res + } end searchImplicit def isUnderSpecifiedArgument(tp: Type): Boolean = diff --git a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala index 5815e9e6890a..a222009205d7 100644 --- a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala +++ b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala @@ -59,6 +59,8 @@ class ImportInfo(symf: Context ?=> Symbol, val enclosingSpan: Span, val isRootImport: Boolean = false) extends Showable { + override def toString = selectors.mkString(s"ImportInfo#$hashCode(", ",", s") at $enclosingSpan") + private def symNameOpt = qualifier match { case ref: untpd.RefTree => Some(ref.name.asTermName) case _ => None diff --git a/tests/neg/t12690a.scala b/tests/neg/t12690a.scala new file mode 100644 index 000000000000..1e41727d3333 --- /dev/null +++ b/tests/neg/t12690a.scala @@ -0,0 +1,16 @@ + +// scalac: -Werror -Wunused:imports + +class X +class Y extends X +object A { implicit val x: X = new X } +object B { implicit val y: Y = new Y } +class C { + import B._ + import A._ // error: unused + def t = implicitly[X] +} + +object Test extends App { + println(new C().t) +} diff --git a/tests/neg/t12690b.scala b/tests/neg/t12690b.scala new file mode 100644 index 000000000000..8ab16c56539b --- /dev/null +++ b/tests/neg/t12690b.scala @@ -0,0 +1,16 @@ + +// scalac: -Werror -Wunused:imports + +class X +class Y extends X +object A { implicit val x: X = new X } +object B { implicit val y: Y = new Y } +class C { + import A._ // error: unused + import B._ + def t = implicitly[X] +} + +object Test extends App { + println(new C().t) +} From e37012de9730e3906308267ca2c14358fcdd336f Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 10 Nov 2022 16:12:27 -0800 Subject: [PATCH 4/4] cleanup --- project/Build.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/project/Build.scala b/project/Build.scala index 9fcced73f519..d735db6a8a5b 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -193,7 +193,7 @@ object Build { "-deprecation", "-unchecked", //"-Wconf:cat=deprecation&msg=Unsafe:s", // example usage - "-Werror", + "-Xfatal-warnings", // -Werror in modern usage "-encoding", "UTF8", "-language:implicitConversions", ), @@ -791,11 +791,6 @@ object Build { "-Ddotty.tests.classes.dottyTastyInspector=" + jars("scala3-tasty-inspector"), ) }, - scalacOptions ++= Seq( - //"-Wunused:imports", - //"-rewrite", - //"-Yrewrite-imports", - ), packageAll := { (`scala3-compiler` / packageAll).value ++ Seq( "scala3-compiler" -> (Compile / packageBin).value.getAbsolutePath,