Skip to content

Commit

Permalink
Warn when named tuples resemble assignments (#21823)
Browse files Browse the repository at this point in the history
This PR adds a warning for named tuples that look like assignment, such
as `(x = 1)`.

This is the first half to implement #21681. The second will be to add
warnings for named arguments to infix method calls (as a separate PR?).

Started during the Spree of October 21st.

Closes #21770.
  • Loading branch information
mbovel authored Oct 22, 2024
2 parents 658edd7 + d1e68f1 commit 276d0a3
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 3 deletions.
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1605,9 +1605,10 @@ object desugar {

/** Translate tuple expressions
*
* () ==> ()
* (t) ==> t
* (t1, ..., tN) ==> TupleN(t1, ..., tN)
* () ==> ()
* (t) ==> t
* (t1, ..., tN) ==> TupleN(t1, ..., tN)
* (n1 = t1, ..., nN = tN) ==> NamedTuple.build[(n1, ..., nN)]()(TupleN(t1, ..., tN))
*/
def tuple(tree: Tuple, pt: Type)(using Context): Tree =
var elems = checkWellFormedTupleElems(tree.trees)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case FinalLocalDefID // errorNumber: 200
case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
case QuotedTypeMissingID // errorNumber: 202
case AmbiguousNamedTupleAssignmentID // errorNumber: 203

def errorNumber = ordinal - 1

Expand Down
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3343,3 +3343,12 @@ final class QuotedTypeMissing(tpe: Type)(using Context) extends StagingMessage(Q
|"""

end QuotedTypeMissing

final class AmbiguousNamedTupleAssignment(key: Name, value: untpd.Tree)(using Context) extends SyntaxMsg(AmbiguousNamedTupleAssignmentID):
override protected def msg(using Context): String =
i"""Ambiguous syntax: this is interpreted as a named tuple with one element,
|not as an assignment.
|
|To assign a value, use curly braces: `{${key} = ${value}}`."""

override protected def explain(using Context): String = ""
15 changes: 15 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3398,6 +3398,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
/** Translate tuples of all arities */
def typedTuple(tree: untpd.Tuple, pt: Type)(using Context): Tree =
val tree1 = desugar.tuple(tree, pt)
checkAmbiguousNamedTupleAssignment(tree)
if tree1 ne tree then typed(tree1, pt)
else
val arity = tree.trees.length
Expand All @@ -3423,6 +3424,20 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
val resTpe = TypeOps.nestedPairs(elemTpes)
app1.cast(resTpe)

/** Checks if `tree` is a named tuple with one element that could be
* interpreted as an assignment, such as `(x = 1)`. If so, issues a warning.
*/
def checkAmbiguousNamedTupleAssignment(tree: untpd.Tuple)(using Context): Unit =
tree.trees match
case List(NamedArg(name, value)) =>
val tmpCtx = ctx.fresh.setNewTyperState()
typedAssign(untpd.Assign(untpd.Ident(name), value), WildcardType)(using tmpCtx)
if !tmpCtx.reporter.hasErrors then
// If there are no errors typing the above, then the named tuple is
// ambiguous and we issue a warning.
report.migrationWarning(AmbiguousNamedTupleAssignment(name, value), tree.srcPos)
case _ => ()

/** Retrieve symbol attached to given tree */
protected def retrieveSym(tree: untpd.Tree)(using Context): Symbol = tree.removeAttachment(SymOfTree) match {
case Some(sym) =>
Expand Down
16 changes: 16 additions & 0 deletions tests/pos/21681d.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
def test1() =
class Person:
def age: Int = ???
def age_=(x: Int): Unit = ???

val person = Person()

(person.age = 29) // no warn (interpreted as `person.age_=(29)`)

def test2() =
class Person:
var age: Int = 28

val person = Person()

(person.age = 29) // no warn (interpreted as `person.age_=(29)`)
7 changes: 7 additions & 0 deletions tests/warn/21681.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E203] Syntax Migration Warning: tests/warn/21681.scala:3:2 ---------------------------------------------------------
3 | (age = 29) // warn
| ^^^^^^^^^^
| Ambiguous syntax: this is interpreted as a named tuple with one element,
| not as an assignment.
|
| To assign a value, use curly braces: `{age = 29}`.
3 changes: 3 additions & 0 deletions tests/warn/21681.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def main() =
var age: Int = 28
(age = 29) // warn
7 changes: 7 additions & 0 deletions tests/warn/21681b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E203] Syntax Migration Warning: tests/warn/21681b.scala:3:2 --------------------------------------------------------
3 | (age = 29) // warn
| ^^^^^^^^^^
| Ambiguous syntax: this is interpreted as a named tuple with one element,
| not as an assignment.
|
| To assign a value, use curly braces: `{age = 29}`.
3 changes: 3 additions & 0 deletions tests/warn/21681b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object Test:
var age: Int = 28
(age = 29) // warn
7 changes: 7 additions & 0 deletions tests/warn/21681c.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E203] Syntax Migration Warning: tests/warn/21681c.scala:5:2 --------------------------------------------------------
5 | (age = 29) // warn
| ^^^^^^^^^^
| Ambiguous syntax: this is interpreted as a named tuple with one element,
| not as an assignment.
|
| To assign a value, use curly braces: `{age = 29}`.
5 changes: 5 additions & 0 deletions tests/warn/21681c.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object Test:
def age: Int = ???
def age_=(x: Int): Unit = ()
age = 29
(age = 29) // warn
7 changes: 7 additions & 0 deletions tests/warn/21770.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E203] Syntax Migration Warning: tests/warn/21770.scala:5:9 ---------------------------------------------------------
5 | f(i => (cache = Some(i))) // warn
| ^^^^^^^^^^^^^^^^^
| Ambiguous syntax: this is interpreted as a named tuple with one element,
| not as an assignment.
|
| To assign a value, use curly braces: `{cache = Some(i)}`.
5 changes: 5 additions & 0 deletions tests/warn/21770.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
def f(g: Int => Unit) = g(0)

def test =
var cache: Option[Int] = None
f(i => (cache = Some(i))) // warn

0 comments on commit 276d0a3

Please sign in to comment.