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

Implement the Any2StringAdd rewrite #10

Merged
merged 4 commits into from
Jun 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions input/src/main/scala/fix/scala213/Any2StringAdd.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
rule = fix.scala213.Any2StringAdd
*/
package fix.scala213

abstract class Any2StringAdd {
// Strings: leave as-is, both literal strings and non-literal
def s = "bob"
def str1 = s + s
def str2 = s + "bob"
def str3 = "bob" + s
def str4 = "bob" + "fred"

// Non-strings: add toString
def nil = Nil + s

// Non-string, generic type: add toString
type A
def x: A
def generic = x + "bob"

// Primitives: add toString
def unit = ()
def bool = true
def byte = 1.toByte
def short = 1.toShort
def char = 'a'
def int = 1
def long = 1L
def float = 1.0F
def double = 1.0
//
def unit1 = unit + s
def bool1 = bool + s
def byte1 = byte + s
def byte2 = byte + byte
def short1 = short + s
def short2 = short + short
def char1 = char + s
def char2 = char + char
def int1 = int + s
def int2 = int + int
def long1 = long + s
def long2 = long + long
def float1 = float + s
def float2 = float + float
def double1 = double + s
def double2 = double + double

// With infix operators, make sure to use parens
def parens1 = Nil ++ Nil + s
def parens2 = int + int + s
def parens3 = {Nil ++ Nil} + s
}
51 changes: 51 additions & 0 deletions output/src/main/scala/fix/scala213/Any2StringAdd.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package fix.scala213

abstract class Any2StringAdd {
// Strings: leave as-is, both literal strings and non-literal
def s = "bob"
def str1 = s + s
def str2 = s + "bob"
def str3 = "bob" + s
def str4 = "bob" + "fred"

// Non-strings: add toString
def nil = Nil.toString + s

// Non-string, generic type: add toString
type A
def x: A
def generic = x.toString + "bob"

// Primitives: add toString
def unit = ()
def bool = true
def byte = 1.toByte
def short = 1.toShort
def char = 'a'
def int = 1
def long = 1L
def float = 1.0F
def double = 1.0
//
def unit1 = unit.toString + s
def bool1 = bool.toString + s
def byte1 = "" + byte + s
def byte2 = byte + byte
def short1 = "" + short + s
def short2 = short + short
def char1 = "" + char + s
def char2 = char + char
def int1 = "" + int + s
def int2 = int + int
def long1 = "" + long + s
def long2 = long + long
def float1 = "" + float + s
def float2 = float + float
def double1 = "" + double + s
def double2 = double + double

// With infix operators, make sure to use parens
def parens1 = (Nil ++ Nil).toString + s
def parens2 = "" + (int + int) + s
def parens3 = {Nil ++ Nil}.toString + s
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
fix.scala213.Any2StringAdd
fix.scala213.Core
fix.scala213.ScalaSeq
fix.scala213.Varargs
39 changes: 39 additions & 0 deletions rewrites/src/main/scala/fix/scala213/Any2StringAdd.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package fix.scala213

import scalafix.v1._

import scala.meta._

object Any2StringAdd {
val any2stringaddPlusString = SymbolMatcher.exact("scala/Predef.any2stringadd#`+`().")
val primitivePlusString = SymbolMatcher.exact(
"scala/Byte#`+`().",
"scala/Short#`+`().",
"scala/Char#`+`().",
"scala/Int#`+`().",
"scala/Long#`+`().",
"scala/Float#`+`().",
"scala/Double#`+`().",
)
}

final class Any2StringAdd extends SemanticRule("fix.scala213.Any2StringAdd") {
import Any2StringAdd._

override def fix(implicit doc: SemanticDocument): Patch = {
doc.tree.collect {
case any2stringaddPlusString(Term.ApplyInfix(lhs, _, _, _)) => addToString(lhs)
case primitivePlusString(Term.ApplyInfix(lhs, _, _, _)) => blankStringPlus(lhs)
}.asPatch
}

private def addToString(term: Term) = term match {
case _: Term.Name | _: Term.Select | _: Term.Block => Patch.addRight(term, ".toString")
case _ => Patch.addLeft(term, "(") + Patch.addRight(term, ").toString")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my only concern here is that it unnecessarily boxes primitives.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that must be why the deprecation message for these is to use string interpolation.

Should we do "" + x for these? (And "" + (...) for non-Name/Select/Block.) . Or should we try to rewrite it to use string interpolation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. IIRC, string interpolation is special-cased by the compiler in 2.13, so it's probably at least as efficient as other options, if not more so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's the right choice. But for mechanical rewrites you could get some weird rewrites, so maybe this is best?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that must be why the deprecation message for these is to use string interpolation.

IIRC, string interpolation is special-cased by the compiler in 2.13

Yes. see scala/scala#7201 scala/bug#11025

}

private def blankStringPlus(term: Term) = term match {
case _: Term.Name | _: Term.Select | _: Term.Block => Patch.addLeft(term, """"" + """)
case _ => Patch.addLeft(term, """"" + (""") + Patch.addRight(term, ")")
}
}