Skip to content

Commit

Permalink
Migration rewrites for infix arguments interpreted as named tuples (#…
Browse files Browse the repository at this point in the history
…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]>
[Cherry-picked 5d5a9e6]
  • Loading branch information
WojciechMazur committed Nov 18, 2024
1 parent 10c13da commit 3e4d48d
Show file tree
Hide file tree
Showing 13 changed files with 140 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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$"

Expand Down
22 changes: 19 additions & 3 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1116,16 +1116,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 10 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
5 changes: 3 additions & 2 deletions docs/_docs/reference/other-new-features/named-tuples.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
41 changes: 41 additions & 0 deletions tests/neg/infix-named-args.check
Original file line number Diff line number Diff line change
@@ -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`
14 changes: 14 additions & 0 deletions tests/neg/infix-named-args.scala
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions tests/rewrites/infix-named-args.check
Original file line number Diff line number Diff line change
@@ -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))
15 changes: 15 additions & 0 deletions tests/rewrites/infix-named-args.scala
Original file line number Diff line number Diff line change
@@ -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))
14 changes: 14 additions & 0 deletions tests/warn/infix-named-args-migration.scala
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 3e4d48d

Please sign in to comment.