From b8ff3d6e9e40f4f0a9df212ceb5b0b4c19a539ab Mon Sep 17 00:00:00 2001 From: Jared Crawford Date: Mon, 17 Oct 2022 09:45:35 -0500 Subject: [PATCH] Fix: Scalafix rename that leads to a name collision breaks compilation #1305 --- .../internal/patch/ReplaceSymbolOps.scala | 31 +++++++++++++++++-- .../src/main/scala-2/test/ReplaceSymbol.scala | 7 ++++- .../com/geirsson/mutable/immutable.scala | 11 +++++++ .../src/main/scala-2/test/ReplaceSymbol.scala | 4 ++- 4 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 scalafix-tests/output/src/main/scala-2/com/geirsson/mutable/immutable.scala diff --git a/scalafix-core/src/main/scala/scalafix/internal/patch/ReplaceSymbolOps.scala b/scalafix-core/src/main/scala/scalafix/internal/patch/ReplaceSymbolOps.scala index bdc10bb79..17082a4c7 100644 --- a/scalafix-core/src/main/scala/scalafix/internal/patch/ReplaceSymbolOps.scala +++ b/scalafix-core/src/main/scala/scalafix/internal/patch/ReplaceSymbolOps.scala @@ -10,6 +10,8 @@ import scalafix.patch.Patch import scalafix.patch.Patch.internal.ReplaceSymbol import scalafix.syntax._ import scalafix.v0._ +import scala.annotation.tailrec + object ReplaceSymbolOps { private object Select { @@ -19,6 +21,20 @@ object ReplaceSymbolOps { case _ => None } } + private def extractImports(stats: Seq[Stat]): Seq[Import] = { + stats + .takeWhile(_.is[Import]) + .collect { case i: Import => i } + } + + @tailrec private final def getGlobalImports(ast: Tree): Seq[Import] = + ast match { + case Pkg(_, Seq(pkg: Pkg)) => getGlobalImports(pkg) + case Source(Seq(pkg: Pkg)) => getGlobalImports(pkg) + case Pkg(_, stats) => extractImports(stats) + case Source(stats) => extractImports(stats) + case _ => Nil + } def naiveMoveSymbolPatch( moveSymbols: Seq[ReplaceSymbol] @@ -92,6 +108,7 @@ object ReplaceSymbolOps { case _ => None } } + val patches = ctx.tree.collect { case n @ Move(to) => // was this written as `to = "blah"` instead of `to = _root_.blah` val isSelected = to match { @@ -126,10 +143,20 @@ object ReplaceSymbolOps { if sig.name != parent.value => Patch.empty // do nothing because it was a renamed symbol case Some(parent) => + val causesCollision = getGlobalImports(ctx.tree).exists { importStmt => + importStmt.importers.flatMap(_.importees).exists { + case Importee.Name(name) => name.value == to.signature.name + case Importee.Rename(_, rename) => rename.value == to.signature.name + case _ => false + } + } val addImport = - if (n.isDefinition) Patch.empty + if (n.isDefinition || causesCollision) Patch.empty else ctx.addGlobalImport(to) - addImport + ctx.replaceTree(n, to.signature.name) + if(causesCollision) + addImport + ctx.replaceTree(n, to.owner.syntax + to.signature.name) + else + addImport + ctx.replaceTree(n, to.signature.name) case _ => Patch.empty } diff --git a/scalafix-tests/input/src/main/scala-2/test/ReplaceSymbol.scala b/scalafix-tests/input/src/main/scala-2/test/ReplaceSymbol.scala index 5281f1dca..d9c07ea32 100644 --- a/scalafix-tests/input/src/main/scala-2/test/ReplaceSymbol.scala +++ b/scalafix-tests/input/src/main/scala-2/test/ReplaceSymbol.scala @@ -12,6 +12,8 @@ patches.replaceSymbols = [ to = "com.geirsson.mutable.CoolBuffer" } { from = "scala.collection.mutable.HashMap" to = "com.geirsson.mutable.unsafe.CoolMap" } + { from = "scala.collection.immutable.TreeMap" + to = "com.geirsson.immutable.SortedMap" } { from = "scala.math.sqrt" to = "com.geirsson.fastmath.sqrt" } // normalized symbol renames all overloaded methods @@ -29,6 +31,8 @@ patches.replaceSymbols = [ */ package fix +import scala.collection.immutable.SortedMap +import scala.collection.immutable.TreeMap import scala.collection.mutable.HashMap import scala.collection.mutable.ListBuffer import scala.collection.mutable @@ -42,8 +46,9 @@ object ReplaceSymbol { "blah".substring(1) "blah".substring(1, 2) val u: mutable.HashMap[Int, Int] = HashMap.empty[Int, Int] + val v: SortedMap[Int, Int] = TreeMap.empty[Int, Int] val x: ListBuffer[Int] = ListBuffer.empty[Int] val y: mutable.ListBuffer[Int] = mutable.ListBuffer.empty[Int] val z: scala.collection.mutable.ListBuffer[Int] = scala.collection.mutable.ListBuffer.empty[Int] -} +} \ No newline at end of file diff --git a/scalafix-tests/output/src/main/scala-2/com/geirsson/mutable/immutable.scala b/scalafix-tests/output/src/main/scala-2/com/geirsson/mutable/immutable.scala new file mode 100644 index 000000000..ae065c477 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-2/com/geirsson/mutable/immutable.scala @@ -0,0 +1,11 @@ +package com.geirsson + +import scala.collection.immutable.TreeMap + +package object immutable { + type SortedMap[A, B] = TreeMap[A, B] + + object SortedMap { + def empty[A : Ordering, B]: SortedMap[A, B] = TreeMap.empty[A, B] + } +} diff --git a/scalafix-tests/output/src/main/scala-2/test/ReplaceSymbol.scala b/scalafix-tests/output/src/main/scala-2/test/ReplaceSymbol.scala index 55fc14b58..8bd9c19c6 100644 --- a/scalafix-tests/output/src/main/scala-2/test/ReplaceSymbol.scala +++ b/scalafix-tests/output/src/main/scala-2/test/ReplaceSymbol.scala @@ -1,5 +1,6 @@ package fix +import scala.collection.immutable.SortedMap import com.geirsson.Future import com.geirsson.{ fastmath, mutable } import com.geirsson.mutable.{ CoolBuffer, unsafe } @@ -13,8 +14,9 @@ object ReplaceSymbol { "blah".substringFrom(1) "blah".substringBetween(1, 2) val u: unsafe.CoolMap[Int, Int] = CoolMap.empty[Int, Int] + val v: SortedMap[Int, Int] = com.geirsson.immutable.SortedMap.empty[Int, Int] val x: CoolBuffer[Int] = CoolBuffer.empty[Int] val y: mutable.CoolBuffer[Int] = mutable.CoolBuffer.empty[Int] val z: com.geirsson.mutable.CoolBuffer[Int] = com.geirsson.mutable.CoolBuffer.empty[Int] -} +} \ No newline at end of file