Skip to content
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

make undocumented replace rules safe to use #1168

Open
bjaglin opened this issue Jun 18, 2020 · 0 comments
Open

make undocumented replace rules safe to use #1168

bjaglin opened this issue Jun 18, 2020 · 0 comments

Comments

@bjaglin
Copy link
Collaborator

bjaglin commented Jun 18, 2020

/*
rules = [
"replace:scala.concurrent/com.geirsson"
]
patches.removeGlobalImports = [
"scala.collection.mutable"
]
patches.replaceSymbols = [
{ from = "fix.ReplaceSymbol"
to = "fix.ReplaceSymbol" }
{ from = "scala.collection.mutable.ListBuffer"
to = "com.geirsson.mutable.CoolBuffer" }
{ from = "scala.collection.mutable.HashMap"
to = "com.geirsson.mutable.unsafe.CoolMap" }
{ from = "scala.math.sqrt"
to = "com.geirsson.fastmath.sqrt" }
// normalized symbol renames all overloaded methods
{ from = "scala.collection.IterableOnceOps.mkString"
to = "unsafeMkString" }
// for prior to scala 2.12
{ from = "scala.collection.TraversableOnce.mkString."
to = "unsafeMkString" }
// non-normalized symbol renames single method overload
{ from = "java/lang/String#substring()."
to = "substringFrom" }
{ from = "java/lang/String#substring(+1)."
to = "substringBetween" }
]
*/
demonstrates usage of simple symbol replacements via --rules=replace:from/to and via the patches.replaceSymbols configuration key, backed by
/**
* Replace occurrences of fromSymbol to reference toSymbol instead.
*
* `toSymbol` must be a global symbol such as an object/class or a static method.
*
* May produce broken code in some cases, works best when toSymbol has the same depth
* as fromSymbol, example:
* - good: replace:com.foo.Bar/org.qux.Buz
* - bad: replace:com.Bar/org.qux.Buz
*/
def replaceSymbols(
toReplace: (String, String)*
)(implicit c: SemanticContext): Patch =
toReplace.foldLeft(Patch.empty) {
case (a, (from, to)) =>
val (fromSymbol, toSymbol) =
ScalafixMetaconfigReaders.parseReplaceSymbol(from, to).get
a + ReplaceSymbol(fromSymbol, toSymbol)
}

As of 0.10.4, this is not documented publicly in tutorials (but does appear in the scaladoc) as it doesn’t handle some important cases leading to generating broken code.

From https://gitter.im/scalacenter/scalafix?at=5eeb039a405be935cda9fe27

At the top of my head, renaming a method won’t rename overrides/supermethods. The rename functionality in metals and IntelliJ are more reliable since they handle such cases. There are some cases the replace rule can do stuff I haven’t seen in IntelliJ or metals (for example replace usage of com.foo.Bar with org.foo.Bar, not definition site) but it’s not obvious for users to evaluate what cases will generate valid rewrites and what cases generate broken code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant