Remove Importee.Rename if it does not shadow anything

scalafix-rules/src/main/scala/scalafix/internal/rule/RemoveUnused.scala

@@ -1,10 +1,11 @@
 package scalafix.internal.rule
 
-import java.util.regex.Pattern
 import metaconfig.Configured
+import scalafix.v1._
+
 import scala.collection.mutable
 import scala.meta._
-import scalafix.v1._
+import scala.meta.transversers.SimpleTraverser
 
 class RemoveUnused(config: RemoveUnusedConfig)
     extends SemanticRule(
@@ -39,55 +40,102 @@ class RemoveUnused(config: RemoveUnusedConfig)
     }
 
   override def fix(implicit doc: SemanticDocument): Patch = {
-    val isUnusedTerm = mutable.Set.empty[Position]
-    val isUnusedImport = mutable.Set.empty[Position]
+    val unusedTermsPosition = mutable.Set.empty[Position]
+    val unusedImportsPosition = mutable.Set.empty[Position]
 
     doc.diagnostics.foreach { diagnostic =>
       if (config.imports && diagnostic.message == "Unused import") {
-        isUnusedImport += diagnostic.position
+        unusedImportsPosition += diagnostic.position
       } else if (config.privates &&
         diagnostic.message.startsWith("private") &&
         diagnostic.message.endsWith("is never used")) {
-        isUnusedTerm += diagnostic.position
+        unusedTermsPosition += diagnostic.position
       } else if (config.locals &&
         diagnostic.message.startsWith("local") &&
         diagnostic.message.endsWith("is never used")) {
-        isUnusedTerm += diagnostic.position
+        unusedTermsPosition += diagnostic.position
       }
     }
 
-    if (isUnusedImport.isEmpty && isUnusedTerm.isEmpty) {
+    if (unusedImportsPosition.isEmpty && unusedTermsPosition.isEmpty) {
       // Optimization: don't traverse if there are no diagnostics to act on.
       Patch.empty
     } else {
-      doc.tree.collect {
-        case i: Importee if isUnusedImport(importPosition(i)) =>
-          i match {
-            case Importee.Rename(_, to) =>
+      fix(doc.tree, unusedTermsPosition.toSet, unusedImportsPosition.toSet)
+    }
+  }
+
+  private def fix(
+      tree: Tree,
+      unusedTermsPosition: Set[Position],
+      unusedImportsPosition: Set[Position])(
+      implicit doc: SemanticDocument): Patch = {
+    val patches = mutable.ListBuffer.empty[Patch]
+    var wildcardImportsInScope = Set.empty[Symbol]
+    var unusedRenameImports = Map.empty[Importee.Rename, Symbol]
+    def isUnusedImport(i: Importee) = unusedImportsPosition(importPosition(i))
+
+    object traverser extends SimpleTraverser {
+      override def apply(current: Tree): Unit = {
+        current match {
+          case Importer(ref, importees) =>
+            importees.foreach {
+              case i: Importee.Wildcard if !isUnusedImport(i) =>
+                wildcardImportsInScope += ref.symbol
+              case i: Importee.Rename if isUnusedImport(i) =>
+                unusedRenameImports += (i -> ref.symbol)
+              case i: Importee if isUnusedImport(i) =>
+                patches += Patch.removeImportee(i).atomic
+              case _ =>
+            }
+            super.apply(current)
+
+          case i: Defn if unusedTermsPosition.contains(i.pos) =>
+            patches += defnTokensToRemove(i)
+            super.apply(current)
+
+          case i @ Defn.Val(_, List(pat), _, _)
+              if unusedTermsPosition.exists(_.start == pat.pos.start) =>
+            patches += defnTokensToRemove(i)
+            super.apply(current)
+
+          case i @ Defn.Var(_, List(pat), _, _)
+              if unusedTermsPosition.exists(_.start == pat.pos.start) =>
+            patches += defnTokensToRemove(i)
+            super.apply(current)
+
+          case EnclosingScope() =>
+            val prevWildcards = wildcardImportsInScope
+            val prevUnusedRenames = unusedRenameImports
+            super.apply(current)
+            val (potentiallyShadowing, notShadowing) =
+              unusedRenameImports.partition {
+                case (_, refSymbol) =>
+                  wildcardImportsInScope.contains(refSymbol)
+              }
+            patches ++= potentiallyShadowing.keys.map {
               // Unimport the identifier instead of removing the importee since
               // unused renamed may still impact compilation by shadowing an identifier.
               // See https://github.com/scalacenter/scalafix/issues/614
-              Patch.replaceTree(to, "_").atomic
-            case _ =>
-              Patch.removeImportee(i).atomic
-          }
-        case i: Defn if isUnusedTerm(i.pos) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
-        case i @ Defn.Val(_, List(pat), _, _)
-            if isUnusedTerm.exists(p => p.start == pat.pos.start) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
-        case i @ Defn.Var(_, List(pat), _, _)
-            if isUnusedTerm.exists(p => p.start == pat.pos.start) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
-      }.asPatch
+              case Importee.Rename(_, to) => Patch.replaceTree(to, "_").atomic
+            }
+            val (unusedRenamesInCurrentScope, remaining) =
+              notShadowing.partition {
+                case (i, _) => !prevUnusedRenames.contains(i)
+              }
+            patches ++= unusedRenamesInCurrentScope.keys
+              .map(Patch.removeImportee(_).atomic)
+            unusedRenameImports = remaining
+            wildcardImportsInScope = prevWildcards
+
+          case _ => super.apply(current)
+        }
+      }
     }
+    traverser(tree)
+    patches.asPatch
   }
 
-  private val unusedPrivateLocalVal: Pattern =
-    Pattern.compile("""(local|private) (.*) is never used""")
-  def isUnusedPrivateDiagnostic(message: Diagnostic): Boolean =
-    unusedPrivateLocalVal.matcher(message.message).matches()
-
   private def importPosition(importee: Importee): Position = importee match {
     case Importee.Rename(from, _) => from.pos
     case _ => importee.pos
@@ -100,13 +148,24 @@ class RemoveUnused(config: RemoveUnusedConfig)
     defn.tokens.take(startBody - startDef)
   }
 
-  private def defnTokensToRemove(defn: Defn): Option[Tokens] = defn match {
-    case i @ Defn.Val(_, _, _, Lit(_)) => Some(i.tokens)
-    case i @ Defn.Val(_, _, _, rhs) => Some(binderTokens(i, rhs))
-    case i @ Defn.Var(_, _, _, Some(Lit(_))) => Some(i.tokens)
-    case i @ Defn.Var(_, _, _, rhs) => rhs.map(binderTokens(i, _))
-    case i: Defn.Def => Some(i.tokens)
-    case _ => None
+  private def defnTokensToRemove(defn: Defn): Patch = {
+    val maybeTokens = defn match {
+      case i @ Defn.Val(_, _, _, Lit(_)) => Some(i.tokens)
+      case i @ Defn.Val(_, _, _, rhs) => Some(binderTokens(i, rhs))
+      case i @ Defn.Var(_, _, _, Some(Lit(_))) => Some(i.tokens)
+      case i @ Defn.Var(_, _, _, rhs) => rhs.map(binderTokens(i, _))
+      case i: Defn.Def => Some(i.tokens)
+      case _ => None
+    }
+    maybeTokens.map(Patch.removeTokens).asPatch.atomic
   }
 
+  private case object EnclosingScope {
+    def unapply(tree: Tree): Boolean = tree match {
+      case _: Source | _: Pkg | _: Template | _: Term.Block |
+          _: Ctor.Secondary =>
+        true
+      case _ => false
+    }
+  }
 }

scalafix-tests/input/src/main/scala/test/RemoveUnusedImportsRename.scala

@@ -3,9 +3,170 @@ rule = RemoveUnused
  */
 package test
 
-import scala.util.{Success => UnusedSuccess, _}
-import scala.concurrent.{Future => UnusedSuccess, _}
+package t1 {
+  import scala.collection.immutable.{List => IList}
 
-class RemoveUnusedImportsRename {
-  Failure(new Exception())
+  object RenameUsed {
+    IList.empty[String]
+  }
 }
+
+package t2 {
+  import scala.collection.immutable.{Set => ISet}
+
+  object RenameUnused
+}
+
+package t3 {
+  import scala.util.{Success => UnusedSuccess, _}
+
+  object RenameUnusedWithUsedWildcard {
+    Failure(new Exception())
+  }
+}
+
+package t4 {
+  import scala.concurrent.{Future => UnusedSuccess, _}
+
+  object RenameUnusedWithUnusedWildcard
+}
+
+package t5 {
+  import scala.collection.mutable.{Map => MutableMap, HashMap => MutableHashMap, HashSet => MutableHashSet}
+  import scala.collection.mutable.{Map => MMap, HashMap => MHashMap, HashSet => MHashSet}
+
+  object MultipleUnusedRenames
+}
+
+package t6 {
+  import scala.concurrent.duration.{FiniteDuration => FDuration}
+  import scala.concurrent.duration._
+
+  object UnusedRenameFollowedByUsedWildcardInDifferentImports {
+    Duration("10s")
+  }
+}
+
+package t7 {
+  import scala.io.StdIn._
+  import scala.io.StdIn.{readByte => readL}
+
+  object UnusedRenameFollowingUsedWildcardInDifferentImports {
+    lazy val l = readLine()
+  }
+}
+
+package t8 {
+  import scala.io.{Source => S}, scala.io._
+
+  object UnusedRenameFollowedByUsedWildcardBetweenCommas {
+    val c = Codec.UTF8
+  }
+}
+
+package t9 {
+  import java.util.concurrent.atomic.{AtomicInteger => A}
+  import java.util
+  import util.concurrent
+  import concurrent.atomic
+  import atomic._
+
+  object UnusedRenameWithUsedWildcardWhosePackagesAreImportedIndividually {
+    new AtomicBoolean()
+  }
+}
+
+package t10 {
+  import java.util.concurrent.locks.{ReentrantLock => RL}
+  import java.{util => jutil}
+  import jutil.{concurrent => jconcurrent}
+  import jconcurrent.{locks => jlocks}
+  import jlocks._
+
+  object UnusedRenameWithUsedWildcardWhosePackagesAreImportedIndividuallyAndRenamed {
+    new StampedLock()
+  }
+}
+
+package t11 {
+  import java.time.{Clock => jClock}
+
+  object UnusedRenameFollowedByUsedWildcardInNestedScope {
+    def foo: Int = { 1 + 1 }
+
+    import java.time._
+    Instant.now()
+  }
+}
+
+package t12 {
+  import java.util.function._
+
+  object UsedWildcardFollowedByUnusedRenameInNestedScope {
+    import java.util.function.{Predicate => Pred}
+    new Consumer[String] {
+      override def accept(t: String): Unit = ???
+    }
+  }
+}
+
+package t13 {
+
+  object UnusedRenameAndUsedWildcardInNonOverlappingScopes {
+    def a: Unit = {
+      import scala.collection.concurrent._
+      TrieMap.empty[Int, Int]
+    }
+
+    def b: Unit = {
+      import scala.collection.concurrent.{Map => CMap}
+      println("b")
+    }
+
+    def c: Unit = {
+      import scala.collection.concurrent._
+      TrieMap.empty[Int, Int]
+    }
+  }
+}
+
+package t14 {
+
+  class UnusedRenameInInnerScopesFollowedByUsedWildcardInOuterScope {
+    def a: Unit = {
+      import scala.math.{BigDecimal => BigDec}
+    }
+
+    val b = {
+      import scala.math.{BigInt => BigInteger}
+      0
+    }
+
+    def this(x: Int) = {
+      this()
+      import scala.math.{E => e}
+    }
+
+    {
+      import scala.math.{random => randomize}
+    }
+
+    trait Foo {
+      import scala.math.{Pi => PI}
+    }
+
+    object L1 {
+      import scala.math.{Pi => PI}
+      object L2 {
+        import scala.math.{Pi => PI}
+        object L3 {
+          import scala.math.{Pi => PI}
+        }
+      }
+    }
+
+    import scala.math._
+    val r = random
+  }
+}
+