Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration rewrites for infix arguments interpreted as named tuples #21949

Merged
merged 6 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1120,16 +1120,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."""
8 changes: 7 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,13 @@ 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:
case NamedArg(_, _) => true
case _ => false
WojciechMazur marked this conversation as resolved.
Show resolved Hide resolved
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
Loading