Skip to content

Commit

Permalink
Deprecate scala.Predef.any2stringadd
Browse files Browse the repository at this point in the history
Ref scala/bug#194

1. This deprecates scala.Predef.any2stringadd.
2. Another thing it does is to remove the conversion altogether under `-Xsource:2.14` flag.

### Background

Scala up till 2.12 has had two kinds of String concatenation operator `+` (also known as PLUS or ADD).

a. When the receiver is a non-String, it uses implicit `Predef.any2addstring` to inject `+`.
b. When the receiver is a String, the compiler injects a special method `String_+`.

In other words, `true + "what"` and `"what" + true` use different mechanisms.
This change addresses the first variant that introduces `+` operator into `Any`.
The rationale discussed in scala/bug#194 is that this implicit injection removes too much type safety, leading to confusing results.

Here some examples listed:

```
scala> var v = Vector[Int]()
v: scala.collection.immutable.Vector[Int] = Vector()

scala> v += 3
<console>:13: error: value += is not a member of scala.collection.immutable.Vector[Int]
  Expression does not convert to assignment because:
    type mismatch;
     found   : Int(3)
     required: String
    expansion: v = v.$plus(3)
       v += 3
         ^

scala> val xs = List(1, 2, 3)
xs: List[Int] = List(1, 2, 3)

scala> xs foreach { println _ + 1 }
<console>:13: error: type mismatch;
 found   : Int(1)
 required: String
       xs foreach { println _ + 1 }
                                ^
```

Note that in both examples the person who wrote the code were not trying to concatenate String.

### Implementation note

Initially I've attempted to implement what Adriaan proposed in 2014 (scala/bug#194 (comment)) and remove all deprecated implicits under `-Xfuture` flag. A simple implementation

```scala
val fd = settings.future && sym.isDeprecated
```

did not work, because the implicit caching is unware of the annotations?
  • Loading branch information
eed3si9n committed Feb 22, 2018
1 parent ec5e241 commit 48076cb
Show file tree
Hide file tree
Showing 43 changed files with 125 additions and 131 deletions.
7 changes: 7 additions & 0 deletions src/compiler/scala/tools/nsc/typechecker/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,13 @@ trait Contexts { self: Analyzer =>
private def isQualifyingImplicit(name: Name, sym: Symbol, pre: Type, imported: Boolean) =
sym.isImplicit &&
isAccessible(sym, pre) &&
!({
// [eed3si9n] ideally I'd like to do this: val fd = settings.isScala214 && sym.isDeprecated
// but implicit caching currently does not report sym.isDeprecated correctly.
val fd = settings.isScala214 && (name ne null) && definitions.isPredefMemberNamed(sym, TermName("any2stringadd"))
if (settings.XlogImplicits && fd) echo(sym.pos, sym + " is not a valid implicit value because:\n" + "-Xsource:2.14 removes scala.Predef.any2stringadd")
fd
}) &&
!(imported && {
val e = scope.lookupEntry(name)
(e ne null) && (e.owner == scope) && (!settings.isScala212 || e.sym.exists)
Expand Down
2 changes: 2 additions & 0 deletions src/library/scala/Predef.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import mutable.ArrayOps
import generic.CanBuildFrom
import scala.annotation.{ elidable, implicitNotFound }
import scala.annotation.elidable.ASSERTION
import scala.annotation.meta.companionMethod
import scala.io.StdIn

/** The `Predef` object provides definitions that are accessible in all Scala
Expand Down Expand Up @@ -326,6 +327,7 @@ object Predef extends LowPriorityImplicits {

// scala/bug#8229 retaining the pre 2.11 name for source compatibility in shadowing this implicit
/** @group implicit-classes-any */
@(deprecated @companionMethod)("Implicit injection of + is deprecated. Convert to String to call +", "2.13.0")
implicit final class any2stringadd[A](private val self: A) extends AnyVal {
def +(other: String): String = String.valueOf(self) + other
}
Expand Down
8 changes: 4 additions & 4 deletions test/files/jvm/future-spec/FutureTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,13 @@ class FutureTests extends MinimalScalaTest {
a <- future0.mapTo[Int] // returns 5
b <- async(a) // returns "10"
c <- async(7) // returns "14"
} yield b + "-" + c
} yield s"$b-$c"

val future2 = for {
a <- future0.mapTo[Int]
b <- (Future { (a * 2).toString }).mapTo[Int]
c <- Future { (7 * 2).toString }
} yield b + "-" + c
} yield s"$b-$c"

Await.result(future1, defaultTimeout) mustBe ("10-14")
assert(checkType(future1, manifest[String]))
Expand All @@ -248,13 +248,13 @@ class FutureTests extends MinimalScalaTest {
Res(a: Int) <- async(Req("Hello"))
Res(b: String) <- async(Req(a))
Res(c: String) <- async(Req(7))
} yield b + "-" + c
} yield s"$b-$c"

val future2 = for {
Res(a: Int) <- async(Req("Hello"))
Res(b: Int) <- async(Req(a))
Res(c: Int) <- async(Req(7))
} yield b + "-" + c
} yield s"$b-$c"

Await.result(future1, defaultTimeout) mustBe ("10-14")
intercept[NoSuchElementException] { Await.result(future2, defaultTimeout) }
Expand Down
2 changes: 1 addition & 1 deletion test/files/jvm/future-spec/main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ trait MinimalScalaTest extends Output with Features {

implicit def objectops(obj: Any) = new {

def mustBe(other: Any) = assert(obj == other, obj + " is not " + other)
def mustBe(other: Any) = assert(obj == other, s"$obj is not $other")
def mustEqual(other: Any) = mustBe(other)

}
Expand Down
6 changes: 6 additions & 0 deletions test/files/neg/implicit-any2stringadd-warning.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
implicit-any2stringadd-warning.scala:2: warning: method any2stringadd in object Predef is deprecated (since 2.13.0): Implicit injection of + is deprecated. Convert to String to call +
true + "what"
^
error: No warnings can be incurred under -Xfatal-warnings.
one warning found
one error found
1 change: 1 addition & 0 deletions test/files/neg/implicit-any2stringadd-warning.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Xfatal-warnings -deprecation
3 changes: 3 additions & 0 deletions test/files/neg/implicit-any2stringadd-warning.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object Test {
true + "what"
}
6 changes: 6 additions & 0 deletions test/files/neg/implicit-any2stringadd.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
method any2stringadd is not a valid implicit value because:
-Xsource:2.14 removes scala.Predef.any2stringadd
implicit-any2stringadd.scala:2: error: value + is not a member of Boolean
true + "what"
^
one error found
1 change: 1 addition & 0 deletions test/files/neg/implicit-any2stringadd.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Xsource:2.14 -Xlog-implicits
3 changes: 3 additions & 0 deletions test/files/neg/implicit-any2stringadd.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object Test {
true + "what"
}
2 changes: 1 addition & 1 deletion test/files/run/collection-stacks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ object Test extends App {
}

def check[T](expected: T, got: T) {
println(got + ": " + (expected == got))
println(s"$got: ${expected == got}")
}

// check #957
Expand Down
1 change: 1 addition & 0 deletions test/files/run/colltest1.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-deprecation
8 changes: 4 additions & 4 deletions test/files/run/colltest1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ object Test extends App {
println("new test starting with "+empty)
assert(empty.isEmpty)
val ten = empty ++ List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
println(ten.size+": "+ten)
println(ten.tail.size+": "+ten.tail)
println(s"${ten.size}: $ten")
println(s"${ten.tail.size}: ${ten.tail}")
assert(ten == empty ++ (1 to 10))
assert(ten.size == 10)
assert(ten forall (_ <= 10))
Expand Down Expand Up @@ -43,7 +43,7 @@ object Test extends App {
assert(ten.last == 10)
assert(List(ten.head) ++ ten.tail == ten)
assert(ten.init ++ List(ten.last) == ten, ten.init)
assert(vs1 == vs2, vs1+"!="+vs2)
assert(vs1 == vs2, s"$vs1!=$vs2")
assert(vs1 == ten)
assert((ten take 5) == firstFive)
assert((ten drop 5) == secondFive)
Expand Down Expand Up @@ -185,7 +185,7 @@ object Test extends App {
assert(m.keySet.size == 26)
assert(m.size == 26)
assert(m.keySet == Set() ++ m.keysIterator)
assert(m.keySet == m.keysIterator.toList.toSet, m.keySet.toList+"!="+m.keysIterator.toList.toSet)
assert(m.keySet == m.keysIterator.toList.toSet, s"${m.keySet.toList}!=${m.keysIterator.toList.toSet}")
val m1 = empty ++ m
val mm = m -- m.keySet.toList
assert(mm.isEmpty, mm)
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/compiler-asSeenFrom.scala
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ package ll {
for (x <- classes ++ terms) {
afterEachPhase(signaturesIn(x.tpe)) collect {
case (ph, sigs) if sigs.nonEmpty =>
println(sigs.mkString(x + " { // after " + ph + "\n ", "\n ", "\n}\n"))
println(sigs.mkString(x.toString + " { // after " + ph + "\n ", "\n ", "\n}\n"))
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions test/files/run/equality.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ object Test
def main(args: Array[String]): Unit = {
var xs = makeFromInt(5)
for (x <- xs ; y <- xs) {
assert(x == y, x + " == " + y)
assert(x == y, s"$x == $y")
assert(hash(x) == hash(y), "hash(%s) == hash(%s)".format(x, y))
}

xs = makeFromInt(-5)
for (x <- xs ; y <- xs) {
assert(x == y, x + " == " + y)
assert(x == y, s"$x == $y")
assert(hash(x) == hash(y), "hash(%s) == hash(%s)".format(x, y))
}

xs = makeFromDouble(500.0)
for (x <- xs ; y <- xs) {
assert(x == y, x + " == " + y)
assert(x == y, s"$x == $y")
assert(hash(x) == hash(y), "hash(%s) == hash(%s)".format(x, y))
}

Expand Down
2 changes: 1 addition & 1 deletion test/files/run/fail-non-value-types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class CompletelyIndependentList[+A] {
object Test {
var failed = false
def expectFailure[T](body: => T): Boolean = {
try { val res = body ; failed = true ; println(res + " failed to fail.") ; false }
try { val res = body ; failed = true ; println(s"$res failed to fail.") ; false }
catch { case _: AssertionError => true }
}

Expand Down
1 change: 1 addition & 0 deletions test/files/run/macroPlugins-enterStats.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-deprecation
2 changes: 1 addition & 1 deletion test/files/run/macroPlugins-enterStats.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ object Test extends DirectTest {
def logEnterStat(pluginName: String, stat: Tree): Unit = log(s"$pluginName:enterStat($stat)")
def deriveStat(pluginName: String, typer: Typer, stat: Tree): List[Tree] = stat match {
case DefDef(mods, name, Nil, Nil, TypeTree(), body) =>
val derived = DefDef(NoMods, TermName(name + pluginName), Nil, Nil, TypeTree(), Ident(TermName("$qmark$qmark$qmark")))
val derived = DefDef(NoMods, TermName(name.toString + pluginName), Nil, Nil, TypeTree(), Ident(TermName("$qmark$qmark$qmark")))
newNamer(typer.context).enterSym(derived)
List(derived)
case _ =>
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/mixin-signatures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ object Test {
}

def show(clazz: Class[_]) {
print(clazz + " {")
print(clazz.toString + " {")
clazz.getMethods.sortBy(x => (x.getName, x.isBridge, x.toString)) filter (_.getName.length == 1) foreach { m =>
print("\n " + m + flagsString(m))
if ("" + m != "" + m.toGenericString) {
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/no-pickle-skolems/Test_2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ object Test {
}
loop(m)

buf.reverse.distinct map (s => s.name + "#" + id(s))
buf.reverse.distinct map (s => s"${s.name}#${id(s)}")
}

def main(args: Array[String]): Unit = {
Expand Down
1 change: 1 addition & 0 deletions test/files/run/patmat-exprs.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-deprecation
2 changes: 1 addition & 1 deletion test/files/run/patmat-exprs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ trait Pattern {
def derivative(v: Var[T]) = Mul(Mul(Const(num.two), expr), expr.derivative(v))
def eval(f: Any => Any) = num.sqr(expr.eval(f))
def mapArgs(f: EndoFunction[Expr[_]]) = Sqr(f(expr))
override def toString = expr + " ^ 2"
override def toString = expr.toString + " ^ 2"
override lazy val hashCode = ScalaRunTime._hashCode(this);
}

Expand Down
2 changes: 1 addition & 1 deletion test/files/run/range.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ object Test {
def rangeForeach(range : Range) = {
val buffer = new scala.collection.mutable.ListBuffer[Int];
range.foreach(buffer += _);
assert(buffer.toList == range.iterator.toList, buffer.toList+"/"+range.iterator.toList)
assert(buffer.toList == range.iterator.toList, buffer.toList.toString + "/" + range.iterator.toList)
}

def boundaryTests() = {
Expand Down
4 changes: 2 additions & 2 deletions test/files/run/reflection-magicsymbols-invoke.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ package scala {
}

object Test extends App {
def key(sym: Symbol) = sym + ": " + sym.info
def key(sym: Symbol) = s"$sym: ${sym.info}"
def test(tpe: Type, receiver: Any, method: String, args: Any*) {
def wrap[T](op: => T) =
try {
Expand All @@ -20,7 +20,7 @@ object Test extends App {
} catch {
case ex: Throwable =>
val realex = scala.ExceptionUtils.unwrapThrowable(ex)
println(realex.getClass + ": " + realex.getMessage)
println(s"${realex.getClass}: ${realex.getMessage}")
}
print(s"testing ${tpe.typeSymbol.name}.$method: ")
wrap({
Expand Down
1 change: 1 addition & 0 deletions test/files/run/reflection-valueclasses-magic.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-deprecation
6 changes: 3 additions & 3 deletions test/files/run/reflection-valueclasses-magic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ object Test extends App {
// initialize parameter symbols
case meth: MethodSymbol => meth.paramLists.flatten.map(_.info)
}
sym + ": " + sym.info
s"$sym: ${sym.info}"
}

def convert(value: Any, tpe: Type) = {
Expand All @@ -42,13 +42,13 @@ object Test extends App {
} catch {
case ex: Throwable =>
val realex = scala.ExceptionUtils.unwrapThrowable(ex)
println(realex.getClass + ": " + realex.getMessage)
println(s"${realex.getClass}: ${realex.getMessage}")
}
val meth = tpe.decl(TermName(method).encodedName.toTermName)
val testees = if (meth.isMethod) List(meth.asMethod) else meth.asTerm.alternatives.map(_.asMethod)
testees foreach (testee => {
val convertedArgs = args.zipWithIndex.map { case (arg, i) => convert(arg, testee.paramLists.flatten.apply(i).info) }
print(s"testing ${tpe.typeSymbol.name}.$method(${testee.paramLists.flatten.map(_.info).mkString(','.toString)}) with receiver = $receiver and args = ${convertedArgs.map(arg => arg + ' '.toString + arg.getClass).toList}: ")
print(s"testing ${tpe.typeSymbol.name}.$method(${testee.paramLists.flatten.map(_.info).mkString(','.toString)}) with receiver = $receiver and args = ${convertedArgs.map(arg => s"$arg ${arg.getClass}").toList}: ")
wrap(cm.reflect(receiver).reflectMethod(testee)(convertedArgs: _*))
})
}
Expand Down

0 comments on commit 48076cb

Please sign in to comment.