Migration rewrites for infix arguments interpreted as named tuples (#21949)

Best effort migration rewrites based on prior work in #21565  

At this point, it's too late to deprecate named infix arguments. Let's
produce warnings instead. We accept infix arguments that might be an
argument of a named tuple, eg. `zip`, `++` or `==` - each of these takes
a single argument with NamedTuple.

---------

Co-authored-by: Som Snytt <[email protected]>
Co-authored-by: Matt Bovel <[email protected]>

Author: 3 people

compiler/src/dotty/tools/dotc/config/MigrationVersion.scala

@@ -26,6 +26,7 @@ enum MigrationVersion(val warnFrom: SourceVersion, val errorFrom: SourceVersion)
   case WithOperator extends MigrationVersion(`3.4`, future)
   case FunctionUnderscore extends MigrationVersion(`3.4`, future)
   case NonNamedArgumentInJavaAnnotation extends MigrationVersion(`3.6`, `3.6`)
+  case AmbiguousNamedTupleInfixApply extends MigrationVersion(`3.6`, never)
   case ImportWildcard extends MigrationVersion(future, future)
   case ImportRename extends MigrationVersion(future, future)
   case ParameterEnclosedByParenthesis extends MigrationVersion(future, future)

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -668,6 +668,7 @@ object StdNames {
     val readResolve: N          = "readResolve"
     val zero: N                 = "zero"
     val zip: N                  = "zip"
+    val `++` : N                = "++"
     val nothingRuntimeClass: N  = "scala.runtime.Nothing$"
     val nullRuntimeClass: N     = "scala.runtime.Null$"
 

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1132,16 +1132,32 @@ object Parsers {
           if (prec < opPrec || leftAssoc && prec == opPrec) {
             opStack = opStack.tail
             recur {
-              atSpan(opInfo.operator.span union opInfo.operand.span union top.span) {
-                InfixOp(opInfo.operand, opInfo.operator, top)
-              }
+              migrateInfixOp(opInfo, isType):
+                atSpan(opInfo.operator.span union opInfo.operand.span union top.span):
+                  InfixOp(opInfo.operand, opInfo.operator, top)
             }
           }
           else top
         }
       recur(top)
     }
 
+    private def migrateInfixOp(opInfo: OpInfo, isType: Boolean)(infixOp: InfixOp): Tree = {
+      def isNamedTupleOperator = opInfo.operator.name match
+        case nme.EQ | nme.NE |  nme.eq | nme.ne | nme.`++` | nme.zip => true
+        case _ => false
+      if isType then infixOp
+      else infixOp.right match
+        case Tuple(args) if args.exists(_.isInstanceOf[NamedArg]) && !isNamedTupleOperator =>
+          report.errorOrMigrationWarning(AmbiguousNamedTupleInfixApply(), infixOp.right.srcPos, MigrationVersion.AmbiguousNamedTupleInfixApply)
+          if MigrationVersion.AmbiguousNamedTupleInfixApply.needsPatch then
+            val asApply = cpy.Apply(infixOp)(Select(opInfo.operand, opInfo.operator.name), args)
+            patch(source, infixOp.span, asApply.show(using ctx.withoutColors))
+            asApply // allow to use pre-3.6 syntax in migration mode
+          else infixOp
+        case _ => infixOp
+    }
+
     /** True if we are seeing a lambda argument after a colon of the form:
      *    : (params) =>
      *      body

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -217,6 +217,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
   case QuotedTypeMissingID // errorNumber: 202
   case AmbiguousNamedTupleAssignmentID // errorNumber: 203
+  case AmbiguousNamedTupleInfixApplyID // errorNumber: 204
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3352,3 +3352,13 @@ final class AmbiguousNamedTupleAssignment(key: Name, value: untpd.Tree)(using Co
       |To assign a value, use curly braces: `{${key} = ${value}}`."""
 
   override protected def explain(using Context): String = ""
+
+class AmbiguousNamedTupleInfixApply()(using Context) extends SyntaxMsg(AmbiguousNamedTupleInfixApplyID):
+  def msg(using Context) =
+    "Ambigious syntax: this infix call argument list is interpreted as single named tuple argument, not as an named arguments list."
+    + Message.rewriteNotice("This", version = SourceVersion.`3.6-migration`)
+
+  def explain(using Context) =
+    i"""Starting with Scala 3.6 infix named arguments are interpretted as Named Tuple.
+        |
+        |To avoid this warning, either remove the argument names or use dotted selection."""

compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala

@@ -110,7 +110,11 @@ object ErrorReporting {
             case tp => i" and expected result type $tp"
           }
           i"(${tp.typedArgs().tpes}%, %)$result"
-        s"arguments ${argStr(tp)}"
+        def hasNames = tp.args.exists:
+          case tree: untpd.Tuple => tree.trees.exists(_.isInstanceOf[NamedArg])
+          case _ => false
+        val addendum = if hasNames then " (a named tuple)" else ""
+        s"arguments ${argStr(tp)}$addendum"
       case _ =>
         i"expected type $tp"
     }

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -79,6 +79,7 @@ class CompilationTests {
       compileFile("tests/rewrites/i20002.scala", defaultOptions.and("-indent", "-rewrite")),
       compileDir("tests/rewrites/annotation-named-pararamters", defaultOptions.and("-rewrite", "-source:3.6-migration")),
       compileFile("tests/rewrites/i21418.scala", unindentOptions.and("-rewrite", "-source:3.5-migration")),
+      compileFile("tests/rewrites/infix-named-args.scala", defaultOptions.and("-rewrite", "-source:3.6-migration")),
     ).checkRewrites()
   }
 

docs/_docs/reference/other-new-features/named-tuples.md

@@ -17,8 +17,9 @@ val persons: List[Person] = ...
 val minors = persons.filter: p =>
   p.age < 18
 ```
-Named bindings in tuples are similar to function parameters and arguments. We use `name: Type` for element types and `name = value` for element values. It is illegal to mix named and unnamed elements in a tuple, or to use the same same
-name for two different elements.
+Named bindings in tuples are similar to function parameters and arguments.
+We use `name: Type` for element types and `name = value` for element values.
+It is illegal to mix named and unnamed elements in a tuple, or to use the same name for two different elements.
 
 Fields of named tuples can be selected by their name, as in the line `p.age < 18` above.
 

tests/neg/infix-named-args.check

@@ -0,0 +1,41 @@
+-- [E134] Type Error: tests/neg/infix-named-args.scala:2:13 ------------------------------------------------------------
+2 |  def f = 42 + (x = 1) // error // werror
+  |          ^^^^
+  |          None of the overloaded alternatives of method + in class Int with types
+  |           (x: Double): Double
+  |           (x: Float): Float
+  |           (x: Long): Long
+  |           (x: Int): Int
+  |           (x: Char): Int
+  |           (x: Short): Int
+  |           (x: Byte): Int
+  |           (x: String): String
+  |          match arguments ((x : Int)) (a named tuple)
+-- [E204] Syntax Warning: tests/neg/infix-named-args.scala:2:15 --------------------------------------------------------
+2 |  def f = 42 + (x = 1) // error // werror
+  |               ^^^^^^^
+  |Ambigious syntax: this infix call argument list is interpreted as single named tuple argument, not as an named arguments list.
+  |This can be rewritten automatically under -rewrite -source 3.6-migration.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E204] Syntax Warning: tests/neg/infix-named-args.scala:5:26 --------------------------------------------------------
+5 |  def g = new C() `multi` (x = 42, y = 27) // werror
+  |                          ^^^^^^^^^^^^^^^^
+  |Ambigious syntax: this infix call argument list is interpreted as single named tuple argument, not as an named arguments list.
+  |This can be rewritten automatically under -rewrite -source 3.6-migration.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E204] Syntax Warning: tests/neg/infix-named-args.scala:6:21 --------------------------------------------------------
+6 |  def h = new C() ** (x = 42, y = 27) // werror
+  |                     ^^^^^^^^^^^^^^^^
+  |Ambigious syntax: this infix call argument list is interpreted as single named tuple argument, not as an named arguments list.
+  |This can be rewritten automatically under -rewrite -source 3.6-migration.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E204] Syntax Warning: tests/neg/infix-named-args.scala:13:18 -------------------------------------------------------
+13 |  def f = this ** (x = 2) // werror
+   |                  ^^^^^^^
+   |Ambigious syntax: this infix call argument list is interpreted as single named tuple argument, not as an named arguments list.
+   |This can be rewritten automatically under -rewrite -source 3.6-migration.
+   |
+   | longer explanation available when compiling with `-explain`

tests/neg/infix-named-args.scala

@@ -0,0 +1,14 @@
+class C:
+  def f = 42 + (x = 1) // error // werror
+  def multi(x: Int, y: Int): Int = x + y
+  def **(x: Int, y: Int): Int = x + y
+  def g = new C() `multi` (x = 42, y = 27) // werror
+  def h = new C() ** (x = 42, y = 27) // werror
+
+type X = (x: Int)
+
+class D(d: Int):
+  def **(x: Int): Int = d * x
+  def **(x: X): Int = d * x.x
+  def f = this ** (x = 2) // werror
+  def g = this ** 2

tests/rewrites/infix-named-args.check

@@ -0,0 +1,15 @@
+class C:
+  def multi(x: Int, y: Int): Int = x + y
+  def **(x: Int, y: Int): Int = x + y
+  def g = new C().multi(x = 42, y = 27)
+  def h = new C().**(x = 42, y = 27)
+
+type X = (x: Int)
+
+class D(d: Int):
+  def **(x: Int): Int = d * x
+  def **(x: X): Int = d * x.x
+  def f = this.**(x = 2)
+  def g = this ** 2
+  def h = this ** ((x = 2))
+  def i = this.**(x = (1 + 1))

tests/rewrites/infix-named-args.scala

@@ -0,0 +1,15 @@
+class C:
+  def multi(x: Int, y: Int): Int = x + y
+  def **(x: Int, y: Int): Int = x + y
+  def g = new C() `multi` (x = 42, y = 27)
+  def h = new C() ** (x = 42, y = 27)
+
+type X = (x: Int)
+
+class D(d: Int):
+  def **(x: Int): Int = d * x
+  def **(x: X): Int = d * x.x
+  def f = this ** (x = 2)
+  def g = this ** 2
+  def h = this ** ((x = 2))
+  def i = this ** (x = (1 + 1))

tests/warn/infix-named-args-migration.scala

@@ -0,0 +1,14 @@
+//> using options -source:3.6-migration
+class C:
+  def f = 42 + (x = 1) // warn // interpreted as 42.+(x = 1) under migration, x is a valid synthetic parameter name
+  def multi(x: Int, y: Int): Int = x + y
+  def **(x: Int, y: Int): Int = x + y
+  def g = new C() `multi` (x = 42, y = 27) // warn
+  def h = new C() ** (x = 42, y = 27) // warn
+
+type X = (x: Int)
+
+class D(d: Int):
+  def **(x: Int): Int = d * x
+  def f = this ** (x = 2) // warn
+  def g = this ** 2