Discourage multi-argument infix syntax: lint applications (x op (a, b)), also lint operator-name definitions

src/compiler/scala/tools/nsc/Reporting.scala

@@ -382,6 +382,7 @@ object Reporting {
     object LintBynameImplicit extends Lint; add(LintBynameImplicit)
     object LintRecurseWithDefault extends Lint; add(LintRecurseWithDefault)
     object LintUnitSpecialization extends Lint; add(LintUnitSpecialization)
+    object LintMultiargInfix extends Lint; add(LintMultiargInfix)
 
     sealed trait Feature extends WarningCategory { override def summaryCategory: WarningCategory = Feature }
     object Feature extends Feature { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[Feature] }; add(Feature)

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

@@ -16,12 +16,11 @@
 package scala.tools.nsc
 package ast.parser
 
-import scala.collection.mutable
-import mutable.ListBuffer
-import scala.reflect.internal.{Precedence, ModifierFlags => Flags}
+import scala.annotation.tailrec
+import scala.collection.mutable, mutable.ListBuffer
+import scala.reflect.internal.{Chars, ModifierFlags => Flags, Precedence}
 import scala.reflect.internal.util.{FreshNameCreator, ListOfNil, Position, SourceFile}
 import Tokens._
-import scala.annotation.tailrec
 import scala.tools.nsc.Reporting.WarningCategory
 
 /** Historical note: JavaParsers started life as a direct copy of Parsers
@@ -868,11 +867,18 @@ self =>
           }
         }
       }
-      def mkNamed(args: List[Tree]) = if (isExpr) args map treeInfo.assignmentToMaybeNamedArg else args
+      def mkNamed(args: List[Tree]) = if (isExpr) args.map(treeInfo.assignmentToMaybeNamedArg) else args
+      var isMultiarg = false
       val arguments = right match {
-        case Parens(Nil)  => literalUnit :: Nil
-        case Parens(args) => mkNamed(args)
-        case _            => right :: Nil
+        case Parens(Nil)               => literalUnit :: Nil
+        case Parens(args @ (_ :: Nil)) => mkNamed(args)
+        case Parens(args)              => isMultiarg = true ; mkNamed(args)
+        case _                         => right :: Nil
+      }
+      def mkApply(fun: Tree, args: List[Tree]) = {
+        val apply = Apply(fun, args)
+        if (isMultiarg) apply.updateAttachment(MultiargInfixAttachment)
+        apply
       }
       if (isExpr) {
         if (rightAssoc) {
@@ -881,15 +887,12 @@ self =>
           val liftedArg = atPos(left.pos) {
             ValDef(Modifiers(FINAL | SYNTHETIC | ARTIFACT), x, TypeTree(), stripParens(left))
           }
-          Block(
-            liftedArg :: Nil,
-            Apply(mkSelection(right), List(Ident(x) setPos left.pos.focus)))
-        } else {
-          Apply(mkSelection(left), arguments)
-        }
-      } else {
-        Apply(Ident(op.encode), stripParens(left) :: arguments)
-      }
+          val apply = mkApply(mkSelection(right), List(Ident(x) setPos left.pos.focus))
+          Block(liftedArg :: Nil, apply)
+        } else
+          mkApply(mkSelection(left), arguments)
+      } else
+        mkApply(Ident(op.encode), stripParens(left) :: arguments)
     }
 
     /* --------- OPERAND/OPERATOR STACK --------------------------------------- */
@@ -2796,6 +2799,12 @@ self =>
             }
             expr()
           }
+        if (settings.multiargInfix)
+          vparamss match {
+            case (_ :: _ :: _) :: Nil if settings.multiargInfix && Chars.isOperatorPart(name.decoded.last) =>
+              warning(nameOffset, "multiarg infix syntax looks like a tuple and will be deprecated", WarningCategory.LintMultiargInfix)
+            case _ =>
+          }
         DefDef(newmods, name.toTermName, tparams, vparamss, restype, rhs)
       }
       signalParseProgress(result.pos)

src/compiler/scala/tools/nsc/settings/Warnings.scala

@@ -192,6 +192,7 @@ trait Warnings {
     val ByNameImplicit         = LintWarning("byname-implicit",           "Block adapted by implicit with by-name parameter.")
     val RecurseWithDefault     = LintWarning("recurse-with-default",      "Recursive call used default argument.")
     val UnitSpecialization     = LintWarning("unit-special",              "Warn for specialization of Unit in parameter position.")
+    val MultiargInfix          = LintWarning("multiarg-infix",            "Infix operator was defined or used with multiarg operand.")
 
     def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
   }
@@ -223,6 +224,7 @@ trait Warnings {
   def warnByNameImplicit         = lint contains ByNameImplicit
   def warnRecurseWithDefault     = lint contains RecurseWithDefault
   def unitSpecialization         = lint contains UnitSpecialization
+  def multiargInfix              = lint contains MultiargInfix
 
   // The Xlint warning group.
   val lint = MultiChoiceSetting(

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -5127,6 +5127,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
               // The enclosing context may be case c @ C(_) => or val c @ C(_) = v.
               tree1 modifyType (_.finalResultType)
               tree1
+            case tree1 @ Apply(_, args1) if tree.hasAttachment[MultiargInfixAttachment.type] && args1.lengthCompare(1) > 0 =>
+              context.warning(tree1.pos, "multiarg infix syntax looks like a tuple and will be deprecated", WarningCategory.LintMultiargInfix)
+              tree1
             case tree1                                                               => tree1
           }
       }

src/reflect/scala/reflect/internal/StdAttachments.scala

@@ -119,4 +119,7 @@ trait StdAttachments {
   class QualTypeSymAttachment(val sym: Symbol)
 
   case object ConstructorNeedsFence extends PlainAttachment
+
+  /** Mark the syntax for linting purposes. */
+  case object MultiargInfixAttachment extends PlainAttachment
 }

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -64,6 +64,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.TypeParamVarargsAttachment
     this.KnownDirectSubclassesCalled
     this.ConstructorNeedsFence
+    this.MultiargInfixAttachment
     this.noPrint
     this.typeDebug
     // inaccessible: this.posAssigner

test/files/jvm/stringbuilder.scala

@@ -30,7 +30,7 @@ Scala is a general purpose programming language designed to express common progr
     println("j2="+j2+", s2="+s2)
     println("s2.toString equals j2.toString = " + (s2.toString equals j2.toString))
 
-    val j3 = j2; j3 setCharAt (0, j3 charAt 2)
+    val j3 = j2; j3.setCharAt(0, j3 charAt 2)
     val s3 = s2; s3(0) = s3(2)
     println("j3="+j3+", s3="+s3)
     println("s3.toString equals j3.toString = " + (s3.toString equals j3.toString))
@@ -57,15 +57,15 @@ object Test3 {
   def run(): Unit = {
     val j0 = new java.lang.StringBuilder("abc")
     val s0 = new StringBuilder("abc")
-    j0 insert (0, true) insert (0, 1.toByte) insert (0, 'a') insert (0, 88.toShort) insert (0, 9) insert (0, -1L)
-    s0 insert (0, true) insert (0, 1.toByte) insert (0, 'a') insert (0, 88.toShort) insert (0, 9) insert (0, -1L)
+    j0.insert(0, true).insert(0, 1.toByte).insert(0, 'a').insert(0, 88.toShort).insert(0, 9).insert(0, -1L)
+    s0.insert(0, true).insert(0, 1.toByte).insert(0, 'a').insert(0, 88.toShort).insert(0, 9).insert(0, -1L)
     println("j0="+j0+", s0="+s0)
     println("s0.toString equals j0.toString = " + (s0.toString equals j0.toString))
 
     val j1 = new java.lang.StringBuilder
     val s1 = new StringBuilder
-    j1 insert (0, "###") insert (0, Array('0', '1', '2')) insert (0, "xyz".subSequence(0, 3))
-    s1 insert (0, "###") insertAll (0, Array('0', '1', '2')) insertAll (0, List('x', 'y', 'z'))
+    j1.insert(0, "###").insert(0, Array('0', '1', '2')).insert(0, "xyz".subSequence(0, 3))
+    s1.insert(0, "###").insertAll(0, Array('0', '1', '2')).insertAll(0, List('x', 'y', 'z'))
     println("j1="+j1+", s1="+s1)
     println("s1.toString equals j1.toString = " + (s1.toString equals j1.toString))
   }
@@ -76,13 +76,13 @@ object Test4 {
     val j0 = new java.lang.StringBuilder("abc") // Java 1.5+
     val s0 = new StringBuilder("abc")
 
-    val j1 = j0 indexOf("c")
-    val s1 = s0 indexOf("c")
+    val j1 = j0.indexOf("c")
+    val s1 = s0.indexOf("c")
     println("j1="+j1+", s1="+s1)
     println("s1 == j1 = " + (s1 == j1))
 
-    val j2 = j0 append "123abc" lastIndexOf("c")
-    val s2 = s0 append "123abc" lastIndexOf("c")
+    val j2 = j0.append("123abc").lastIndexOf("c")
+    val s2 = s0.append("123abc").lastIndexOf("c")
     println("j2="+j2+", s2="+s2)
     println("s2 == j2 = " + (s2 == j2))
  }

test/files/neg/sd503.check

@@ -0,0 +1,35 @@
+sd503.scala:18: error: not enough arguments for method x_=: (i: Int, j: Int): Unit.
+Unspecified value parameter j.
+  def f4() = c.x = (42, 27)           // missing arg
+                 ^
+sd503.scala:23: error: type mismatch;
+ found   : (Int, Int)
+ required: Int
+  def f7() = d.x = (42, 27)           // type mismatch (same as doti)
+                   ^
+sd503.scala:9: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def % (i: Int, j: Int) = i + j      // operator, warn
+      ^
+sd503.scala:29: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def x_=(i: Int, j: Int): Unit = value = i + j           // multiarg, warn
+      ^
+sd503.scala:34: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def x_=(i: Int, j: Int = 1): Unit = devalue = i + j     // multiarg, warn
+      ^
+sd503.scala:47: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def +=(x: A, y: A, zs: A*): this.type = addAll(x +: y +: zs)                // very multiarg, warn
+      ^
+sd503.scala:13: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def f1(t: T) = t m (1, 2)           // multiarg, warn
+                   ^
+sd503.scala:15: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def f3(t: T) = t % (1, 2)           // multiarg, warn
+                   ^
+sd503.scala:19: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def f5() = c x_= (42, 27)           // multiarg, warn
+               ^
+sd503.scala:52: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  def f[A](as: Embiggen[A], x: A, y: A, z: A): as.type = as += (x, y, z)      // very multiarg, warn
+                                                            ^
+8 warnings
+2 errors

test/files/neg/sd503.scala

@@ -0,0 +1,53 @@
+// scalac: -Xlint:multiarg-infix
+//
+// lint multiarg infix syntax, e.g., vs += (1, 2, 3)
+// Since infix is encouraged by symbolic operator names, discourage defining def += (x: A, y: A, zs: A*)
+
+trait T {
+  def m(i: Int, j: Int) = i + j
+
+  def % (i: Int, j: Int) = i + j      // operator, warn
+
+
+  def f0(t: T) = t.m(1, 2)            // ok
+  def f1(t: T) = t m (1, 2)           // multiarg, warn
+  def f2(t: T) = t.%(1, 2)            // ok
+  def f3(t: T) = t % (1, 2)           // multiarg, warn
+
+  def c = new C
+  def f4() = c.x = (42, 27)           // missing arg
+  def f5() = c x_= (42, 27)           // multiarg, warn
+
+  def d = new D
+  def f6() = d.x = 42                 // ok!
+  def f7() = d.x = (42, 27)           // type mismatch (same as doti)
+}
+
+class C {
+  private var value: Int = _
+  def x: Int = value
+  def x_=(i: Int, j: Int): Unit = value = i + j           // multiarg, warn
+}
+class D {
+  private var devalue: Int = _                            // d.value
+  def x: Int = devalue
+  def x_=(i: Int, j: Int = 1): Unit = devalue = i + j     // multiarg, warn
+}
+
+// If the application is adapted such that what looks like a tuple is taken as a tuple,
+// then don't warn; eventually this will be normal behavior.
+trait OK {
+  def f(p: (Int, Int)) = p == (42, 17)                    // nowarn!
+  def g(ps: Embiggen[(Int, Int)]) = ps :+ (42, 17)        // nowarn!
+}
+
+// Growable is the paradigmatic example
+trait Embiggen[A] {
+  def addAll(as: Seq[A]): this.type
+  def +=(x: A, y: A, zs: A*): this.type = addAll(x +: y +: zs)                // very multiarg, warn
+  def :+(a: A): Embiggen[A]
+}
+
+trait NotOK {
+  def f[A](as: Embiggen[A], x: A, y: A, z: A): as.type = as += (x, y, z)      // very multiarg, warn
+}

test/files/neg/t2641.check

@@ -2,6 +2,6 @@ t2641.scala:22: error: wrong number of type arguments for ManagedSeq, should be
         with TraversableViewLike[A, ManagedSeqStrict[A], ManagedSeq[A]]
                                                          ^
 t2641.scala:28: error: value managedIterator is not a member of ManagedSeq[A,Coll]
-    override def managedIterator = self.managedIterator slice (0, 0)
+    override def managedIterator = self.managedIterator.slice(0, 0)
                                         ^
 2 errors

test/files/neg/t2641.scala

@@ -25,7 +25,7 @@ trait ManagedSeq[+A, +Coll]
   trait Transformed[+B] extends ManagedSeq[B, Coll] with super.Transformed[B]
 
   trait Sliced extends Transformed[A] with super.Sliced {
-    override def managedIterator = self.managedIterator slice (0, 0)
+    override def managedIterator = self.managedIterator.slice(0, 0)
   }
 
 }

test/files/run/collections.scala

@@ -35,7 +35,7 @@ object Test extends App {
     println("***** "+msg+":")
     var s = s0
     s = s.clone() += 2
-    s = s.clone += (3, 4000, 10000)
+    s = s.clone.+=(3, 4000, 10000)
     println("test1: "+sum(s))
     time {
       s = s ++ (List.range(0, iters) map (2*))
@@ -88,7 +88,7 @@ object Test extends App {
     println("***** "+msg+":")
     var s = s0
     s = s.clone() += (2 -> 2)
-    s = s.clone() += (3 -> 3, 4000 -> 4000, 10000 -> 10000)
+    s = s.clone().+=(3 -> 3, 4000 -> 4000, 10000 -> 10000)
     println("test1: "+sum(s map (_._2)))
     time {
       s = s ++ (List.range(0, iters) map (x => x * 2 -> x * 2))

test/files/run/colltest1.scala

@@ -47,8 +47,8 @@ object Test extends App {
     assert(vs1 == ten)
     assert((ten take 5) == firstFive)
     assert((ten drop 5) == secondFive)
-    assert(ten slice (3, 3) isEmpty)
-    assert((ten slice (3, 6)) == List(4, 5, 6), ten slice (3, 6))
+    assert(ten.slice(3, 3).isEmpty)
+    assert((ten.slice(3, 6)) == List(4, 5, 6), ten.slice(3, 6))
     assert((ten takeWhile (_ <= 5)) == firstFive)
     assert((ten dropWhile (_ <= 5)) == secondFive)
     assert((ten span (_ <= 5)) == (firstFive, secondFive))

test/files/run/indyLambdaKinds/Test_2.scala

@@ -49,7 +49,7 @@ object Test extends DirectTest {
 
   override def show(): Unit = {
     compile()
-    ScalaClassLoader(getClass.getClassLoader) run ("Main", Nil)
+    ScalaClassLoader(getClass.getClassLoader).run("Main", Nil)
 
   }
 }

test/files/run/infix.check

@@ -1,2 +1,17 @@
+infix.scala:6: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  val xs = new op(null, 0, 0) op (1, 1) op (2, 2)
+                              ^
+infix.scala:6: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  val xs = new op(null, 0, 0) op (1, 1) op (2, 2)
+                                        ^
+infix.scala:9: warning: multiarg infix syntax looks like a tuple and will be deprecated
+    case null op (0, 0) op (1, 1) op (2, 2) => Console.println("OK")
+              ^
+infix.scala:9: warning: multiarg infix syntax looks like a tuple and will be deprecated
+    case null op (0, 0) op (1, 1) op (2, 2) => Console.println("OK")
+                        ^
+infix.scala:9: warning: multiarg infix syntax looks like a tuple and will be deprecated
+    case null op (0, 0) op (1, 1) op (2, 2) => Console.println("OK")
+                                  ^
 op(op(op(null,0,0),1,1),2,2)
 OK

test/files/run/synchronized.scala

@@ -328,9 +328,7 @@ class C2 extends T
 object O2 extends T
 
 object Test extends App {
-  def check(name: String, result: Boolean): Unit = {
-    println("%-10s %s" format (name +":", if (result) "OK" else "FAILED"))
-  }
+  def check(name: String, result: Boolean): Unit = println("%-10s %s".format(name +":", if (result) "OK" else "FAILED"))
 
   val c1 = new C1
   check("c1.f1",   c1.f1)

test/files/run/t2544.scala

@@ -11,10 +11,10 @@ object Test {
   )
 
   def main(args: Array[String]) = {
-    println(Foo indexWhere(_ >= 2,1))
-    println(Foo.toList indexWhere(_ >= 2,1))
-    println(Foo segmentLength(_ <= 3,1))
-    println(Foo.toList segmentLength(_ <= 3,1))
+    println(Foo.indexWhere(_ >= 2,1))
+    println(Foo.toList.indexWhere(_ >= 2,1))
+    println(Foo.segmentLength(_ <= 3,1))
+    println(Foo.toList.segmentLength(_ <= 3,1))
     lengthEquiv(Foo lengthCompare 7)
     lengthEquiv(Foo.toList lengthCompare 7)
     lengthEquiv(Foo lengthCompare 2)

test/files/run/t4813.scala

@@ -26,6 +26,6 @@ object Test extends App {
   runTest(TreeSet(1,2,3))(_.clone) { buf => buf add 4 }
 
   // Maps
-  runTest(HashMap(1->1,2->2,3->3))(_.clone) { buf => buf put (4,4) }
-  runTest(WeakHashMap(1->1,2->2,3->3))(_.clone) { buf => buf put (4,4) }
+  runTest(HashMap(1->1,2->2,3->3))(_.clone)(_.put(4,4))
+  runTest(WeakHashMap(1->1,2->2,3->3))(_.clone)(_.put(4,4))
 }

test/files/run/t5045.scala

@@ -1,12 +1,10 @@
 
-import scala.language.postfixOps
-
 object Test extends App {
 
  import scala.util.matching.{ Regex, UnanchoredRegex }
 
  val dateP1 = """(\d\d\d\d)-(\d\d)-(\d\d)""".r.unanchored
- val dateP2 = """(\d\d\d\d)-(\d\d)-(\d\d)""" r ("year", "month", "day") unanchored
+ val dateP2 = """(\d\d\d\d)-(\d\d)-(\d\d)""".r("year", "month", "day").unanchored
  val dateP3 =  new Regex("""(\d\d\d\d)-(\d\d)-(\d\d)""", "year", "month", "day") with UnanchoredRegex
 
  val yearStr = "2011"

test/files/run/t5565.check

@@ -0,0 +1,3 @@
+t5565.scala:10: warning: multiarg infix syntax looks like a tuple and will be deprecated
+  assert(math.abs(-4.0) ~== (4.0, 0.001))
+                        ^

test/files/run/t6271.scala

@@ -21,7 +21,7 @@ object Test extends App {
   }
   def slicedIssue = {
     val viewed : Iterable[Iterable[Int]] = List(List(0).view).view
-    val filtered = viewed flatMap { x => List( x slice (2,3) ) }
+    val filtered = viewed.flatMap(x => List(x.slice(2,3)))
     filtered.iterator.to(Iterable).flatten
   }
   filterIssue