-
Couldn't load subscription status.
- Fork 189
Fix compilation breaking due to collisions in Scalafix symbol replacements #2260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0d96c1e
d960708
125fbc5
a8e39db
2de7be1
12569bb
100b6d4
26b2f88
e0aefe8
97dd899
aac4d5d
3374091
85cd8bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||
| package scalafix.internal.patch | ||||||
|
|
||||||
| import scala.annotation.tailrec | ||||||
|
|
||||||
| import scala.meta._ | ||||||
| import scala.meta.internal.trees._ | ||||||
|
|
||||||
|
|
@@ -12,6 +14,11 @@ import scalafix.syntax._ | |||||
| import scalafix.v0._ | ||||||
|
|
||||||
| object ReplaceSymbolOps { | ||||||
| private case class ImportInfo( | ||||||
| globalImports: Seq[Import], | ||||||
| globalImportedSymbols: Map[String, Symbol] | ||||||
| ) | ||||||
|
|
||||||
| private object Select { | ||||||
| def unapply(arg: Ref): Option[(Ref, Name)] = arg match { | ||||||
| case Term.Select(a: Ref, b) => Some(a -> b) | ||||||
|
|
@@ -20,10 +27,47 @@ object ReplaceSymbolOps { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| private def extractImports(stats: Seq[Stat]): Seq[Import] = { | ||||||
| stats.collect { case i: Import => i } | ||||||
| } | ||||||
|
|
||||||
| private def extractImportInfo( | ||||||
| tree: Tree | ||||||
| )(implicit index: SemanticdbIndex): ImportInfo = { | ||||||
| @tailrec | ||||||
| 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 | ||||||
| } | ||||||
|
|
||||||
| val globalImports = getGlobalImports(tree) | ||||||
|
|
||||||
| // pre-compute global imported symbols for O(1) collision detection | ||||||
| // since ctx.addGlobalImport adds imports at global scope | ||||||
| val globalImportedSymbols = globalImports.flatMap { importStat => | ||||||
| importStat.importers.flatMap { importer => | ||||||
| importer.importees.collect { | ||||||
| case Importee.Name(name) => | ||||||
| name.value -> name.symbol.getOrElse(Symbol.None) | ||||||
| case Importee.Rename(_, rename) => | ||||||
| rename.value -> rename.symbol.getOrElse(Symbol.None) | ||||||
| } | ||||||
| } | ||||||
| }.toMap | ||||||
|
|
||||||
| ImportInfo(globalImports, globalImportedSymbols) | ||||||
| } | ||||||
|
|
||||||
| def naiveMoveSymbolPatch( | ||||||
| moveSymbols: Seq[ReplaceSymbol] | ||||||
| )(implicit ctx: RuleCtx, index: SemanticdbIndex): Patch = { | ||||||
| if (moveSymbols.isEmpty) return Patch.empty | ||||||
|
|
||||||
| val importInfo = extractImportInfo(ctx.tree)(index) | ||||||
|
|
||||||
| val moves: Map[String, Symbol.Global] = | ||||||
| moveSymbols.iterator.flatMap { | ||||||
| case ReplaceSymbol( | ||||||
|
|
@@ -126,10 +170,15 @@ object ReplaceSymbolOps { | |||||
| if sig.name != parent.value => | ||||||
| Patch.empty // do nothing because it was a renamed symbol | ||||||
| case Some(_) => | ||||||
| val causesCollision = | ||||||
| importInfo.globalImportedSymbols.contains(to.signature.name) | ||||||
| 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) | ||||||
|
||||||
| addImport + ctx.replaceTree(n, to.signature.name) | |
| patchForCollision(n, to, addImport, causesCollision) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.geirsson | ||
|
|
||
| import scala.collection.immutable.TreeMap | ||
|
|
||
| object immutable { | ||
| type SortedMap[A, B] = TreeMap[A, B] | ||
|
|
||
| object SortedMap { | ||
| def empty[A : Ordering, B]: SortedMap[A, B] = TreeMap.empty[A, B] | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String concatenation for building qualified names could be error-prone. Consider using a more robust method like
to.owner.syntax + "." + to.signature.nameor a dedicated method to ensure proper formatting.