From 20fc59bbc8fa2cf91e391431acc217fa9cdd35b2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 6 May 2022 12:09:48 -0700 Subject: [PATCH] Clean up rewrite unused imports --- .../src/dotty/tools/dotc/core/Contexts.scala | 54 +++------------- .../dotty/tools/dotc/typer/TyperPhase.scala | 43 ++++++++++++- .../dotty/tools/dotc/CompilationTests.scala | 1 + tests/rewrites/unused-imports.check | 60 ++++++++++++++++++ tests/rewrites/unused-imports.scala | 62 +++++++++++++++++++ 5 files changed, 175 insertions(+), 45 deletions(-) create mode 100644 tests/rewrites/unused-imports.check create mode 100644 tests/rewrites/unused-imports.scala diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 9bc1ef1190e7..c494e1510a2c 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -25,7 +25,6 @@ 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, ScalaRelease} import classfile.ReusableDataReader @@ -1024,7 +1023,6 @@ object Contexts: /** 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) @@ -1040,60 +1038,28 @@ object Contexts: 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)] + def unused(using Context): List[(ImportInfo, Symbol, List[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", "")) + var unusages = List.empty[(ImportInfo, Symbol, List[untpd.ImportSelector])] def checkUsed(info: ImportInfo, owner: Symbol): Unit = - val used = selectors(info) - var needsPatch = false + val usedSelectors = selectors(info) + var unusedSelectors = List.empty[untpd.ImportSelector] 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 + if !selector.isMask && !usedSelectors(selector) then + unusedSelectors ::= selector 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)) + if unusedSelectors.nonEmpty then unusages ::= (info, owner, unusedSelectors) end checkUsed importInfos.remove(ctx.compilationUnit).foreach(_.foreach(checkUsed)) - unusages + unusages + else + Nil 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() diff --git a/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala b/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala index fa6e6ef3488d..435734a0373c 100644 --- a/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala +++ b/compiler/src/dotty/tools/dotc/typer/TyperPhase.scala @@ -2,6 +2,7 @@ package dotty.tools package dotc package typer +import ast.untpd import core._ import Phases._ import Contexts._ @@ -56,7 +57,47 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase { } def emitDiagnostics(using Context): Unit = - ctx.usages.unused.foreach((importInfo, owner, selector) => report.warning(s"Unused import", pos = selector.srcPos)) + ctx.usages.unused.foreach { (info, owner, selectors) => + import rewrites.Rewrites.patch + import parsing.Parsers + import util.SourceFile + import ast.Trees.* + def reportSelectors() = selectors.foreach(selector => report.warning(s"Unused import", pos = selector.srcPos)) + if 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.filterNot(selectors.contains) + 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)) + else + reportSelectors() + else + reportSelectors() + } + // 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 clearDiagnostics()(using Context): Unit = ctx.usages.clear() diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 5b18b6c81fe9..657e146ab7ce 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -79,6 +79,7 @@ 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), ).checkRewrites() } diff --git a/tests/rewrites/unused-imports.check b/tests/rewrites/unused-imports.check new file mode 100644 index 000000000000..885dee2d20b5 --- /dev/null +++ b/tests/rewrites/unused-imports.check @@ -0,0 +1,60 @@ +// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports + +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) + } + +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..d499a4837496 --- /dev/null +++ b/tests/rewrites/unused-imports.scala @@ -0,0 +1,62 @@ +// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports + +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 = ()