From 008abbda54c95549306597ffeaea81c73c210f96 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Mon, 21 Oct 2024 18:22:43 +0200 Subject: [PATCH 1/3] Warn when named tuples resemble assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Nicolas Stucki <3648029+nicolasstucki@users.noreply.github.com> Co-Authored-By: Oliver Bračevac [Cherry-picked 959f68c1259aa12734432c990d71d3fa2d99427e] --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 9 ++++++--- .../src/dotty/tools/dotc/reporting/ErrorMessageID.scala | 1 + compiler/src/dotty/tools/dotc/reporting/messages.scala | 9 +++++++++ tests/warn/21681.check | 7 +++++++ tests/warn/21681.scala | 3 +++ tests/warn/21770.check | 7 +++++++ tests/warn/21770.scala | 5 +++++ 7 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 tests/warn/21681.check create mode 100644 tests/warn/21681.scala create mode 100644 tests/warn/21770.check create mode 100644 tests/warn/21770.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index e1a6b97fc7d3..482210845fea 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -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) @@ -1638,6 +1639,8 @@ object desugar { if ctx.mode.is(Mode.Type) then AppliedTypeTree(ref(defn.NamedTupleTypeRef), namesTuple :: tup :: Nil) else + if names.length == 1 && ctx.scope.lookup(names.head).is(Flags.Mutable) then + report.migrationWarning(AmbiguousNamedTupleAssignment(names.head, elemValues.head), tree.srcPos) Apply( Apply( TypeApply( diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index db523c879ea2..6d0a85b3ef0f 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 97cd70113c2e..3b7fba1cb52d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -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 = "" diff --git a/tests/warn/21681.check b/tests/warn/21681.check new file mode 100644 index 000000000000..e86ce4e36134 --- /dev/null +++ b/tests/warn/21681.check @@ -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}`. diff --git a/tests/warn/21681.scala b/tests/warn/21681.scala new file mode 100644 index 000000000000..76a19c96e1cb --- /dev/null +++ b/tests/warn/21681.scala @@ -0,0 +1,3 @@ +def main() = + var age: Int = 28 + (age = 29) // warn diff --git a/tests/warn/21770.check b/tests/warn/21770.check new file mode 100644 index 000000000000..0899f11d6ca5 --- /dev/null +++ b/tests/warn/21770.check @@ -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)}`. diff --git a/tests/warn/21770.scala b/tests/warn/21770.scala new file mode 100644 index 000000000000..9696a31d6ba8 --- /dev/null +++ b/tests/warn/21770.scala @@ -0,0 +1,5 @@ +def f(g: Int => Unit) = g(0) + +def test = + var cache: Option[Int] = None + f(i => (cache = Some(i))) // warn From 21886124d21d6940e786a9f28015c15f4550f9ca Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Mon, 21 Oct 2024 19:51:53 +0200 Subject: [PATCH 2/3] Move AmbiguousNamedTupleAssignment check to Typer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Nicolas Stucki <3648029+nicolasstucki@users.noreply.github.com> Co-Authored-By: Oliver Bračevac [Cherry-picked a00a806361666822c2a8098c2f45f8b93df15483] --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 2 -- compiler/src/dotty/tools/dotc/typer/Typer.scala | 13 +++++++++++++ tests/pos/21681d.scala | 16 ++++++++++++++++ tests/warn/21681b.check | 7 +++++++ tests/warn/21681b.scala | 3 +++ 5 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tests/pos/21681d.scala create mode 100644 tests/warn/21681b.check create mode 100644 tests/warn/21681b.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 482210845fea..e66c71731b4f 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1639,8 +1639,6 @@ object desugar { if ctx.mode.is(Mode.Type) then AppliedTypeTree(ref(defn.NamedTupleTypeRef), namesTuple :: tup :: Nil) else - if names.length == 1 && ctx.scope.lookup(names.head).is(Flags.Mutable) then - report.migrationWarning(AmbiguousNamedTupleAssignment(names.head, elemValues.head), tree.srcPos) Apply( Apply( TypeApply( diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index f20c2d313ec7..27619fcc3f0b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3395,6 +3395,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 @@ -3420,6 +3421,18 @@ 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 typedName = typedIdent(untpd.Ident(name), WildcardType) + val sym = typedName.symbol + if sym.exists && (sym.is(Flags.Mutable) || sym.setter.exists) then + 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) => diff --git a/tests/pos/21681d.scala b/tests/pos/21681d.scala new file mode 100644 index 000000000000..97a01dec74aa --- /dev/null +++ b/tests/pos/21681d.scala @@ -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)`) diff --git a/tests/warn/21681b.check b/tests/warn/21681b.check new file mode 100644 index 000000000000..32760e00ebb6 --- /dev/null +++ b/tests/warn/21681b.check @@ -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}`. diff --git a/tests/warn/21681b.scala b/tests/warn/21681b.scala new file mode 100644 index 000000000000..710d69b0dd23 --- /dev/null +++ b/tests/warn/21681b.scala @@ -0,0 +1,3 @@ +object Test: + var age: Int = 28 + (age = 29) // warn From 10c13dadca61911ffc3eb40344fee1f1a316feea Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Tue, 22 Oct 2024 14:09:45 +0200 Subject: [PATCH 3/3] Try to type as an `Assign` [Cherry-picked d1e68f19c99d4171f0c4a6f17cef0ceb72a20bd8] --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 8 +++++--- tests/warn/21681c.check | 7 +++++++ tests/warn/21681c.scala | 5 +++++ 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 tests/warn/21681c.check create mode 100644 tests/warn/21681c.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 27619fcc3f0b..554e9f1c2f51 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3427,9 +3427,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def checkAmbiguousNamedTupleAssignment(tree: untpd.Tuple)(using Context): Unit = tree.trees match case List(NamedArg(name, value)) => - val typedName = typedIdent(untpd.Ident(name), WildcardType) - val sym = typedName.symbol - if sym.exists && (sym.is(Flags.Mutable) || sym.setter.exists) then + 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 _ => () diff --git a/tests/warn/21681c.check b/tests/warn/21681c.check new file mode 100644 index 000000000000..11c427f87cfe --- /dev/null +++ b/tests/warn/21681c.check @@ -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}`. diff --git a/tests/warn/21681c.scala b/tests/warn/21681c.scala new file mode 100644 index 000000000000..5e2eae11708c --- /dev/null +++ b/tests/warn/21681c.scala @@ -0,0 +1,5 @@ +object Test: + def age: Int = ??? + def age_=(x: Int): Unit = () + age = 29 + (age = 29) // warn