From 3e4d48d67f41a806f78b78f5aeeb4a28c1b727cd Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Mon, 18 Nov 2024 10:35:15 +0100 Subject: [PATCH] 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 Co-authored-by: Matt Bovel [Cherry-picked 5d5a9e6ff17a5d1980653e6fcb622dc5a2e4d082] --- .../tools/dotc/config/MigrationVersion.scala | 1 + .../src/dotty/tools/dotc/core/StdNames.scala | 1 + .../dotty/tools/dotc/parsing/Parsers.scala | 22 ++++++++-- .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 10 +++++ .../tools/dotc/typer/ErrorReporting.scala | 6 ++- .../dotty/tools/dotc/CompilationTests.scala | 1 + .../other-new-features/named-tuples.md | 5 ++- tests/neg/infix-named-args.check | 41 +++++++++++++++++++ tests/neg/infix-named-args.scala | 14 +++++++ tests/rewrites/infix-named-args.check | 15 +++++++ tests/rewrites/infix-named-args.scala | 15 +++++++ tests/warn/infix-named-args-migration.scala | 14 +++++++ 13 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 tests/neg/infix-named-args.check create mode 100644 tests/neg/infix-named-args.scala create mode 100644 tests/rewrites/infix-named-args.check create mode 100644 tests/rewrites/infix-named-args.scala create mode 100644 tests/warn/infix-named-args-migration.scala diff --git a/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala b/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala index 3da716abbc40..247e3f62a98d 100644 --- a/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala +++ b/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) diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index d3e198a7e7a7..56d71c7fb57e 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/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$" diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 807755af7ab0..53fe9934d64b 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1116,9 +1116,9 @@ 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 @@ -1126,6 +1126,22 @@ object Parsers { 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 diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 6d0a85b3ef0f..35c170858bbf 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/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 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 3b7fba1cb52d..328ec122f848 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/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.""" diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 68143dfd2ba0..13e75be75838 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/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" } diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index d7ef7f6f6085..ee2dd587c4fd 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/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() } diff --git a/docs/_docs/reference/other-new-features/named-tuples.md b/docs/_docs/reference/other-new-features/named-tuples.md index df96a91fe182..5483c5cc255b 100644 --- a/docs/_docs/reference/other-new-features/named-tuples.md +++ b/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. diff --git a/tests/neg/infix-named-args.check b/tests/neg/infix-named-args.check new file mode 100644 index 000000000000..0cfbbaef73a3 --- /dev/null +++ b/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` diff --git a/tests/neg/infix-named-args.scala b/tests/neg/infix-named-args.scala new file mode 100644 index 000000000000..d8616899540c --- /dev/null +++ b/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 diff --git a/tests/rewrites/infix-named-args.check b/tests/rewrites/infix-named-args.check new file mode 100644 index 000000000000..a50593ef18a8 --- /dev/null +++ b/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)) \ No newline at end of file diff --git a/tests/rewrites/infix-named-args.scala b/tests/rewrites/infix-named-args.scala new file mode 100644 index 000000000000..bcdf4a21a9d2 --- /dev/null +++ b/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)) diff --git a/tests/warn/infix-named-args-migration.scala b/tests/warn/infix-named-args-migration.scala new file mode 100644 index 000000000000..df4bfb50271c --- /dev/null +++ b/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