From f952dd2e2b2664d2a3a339b51c3b252e0ee9984d Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Thu, 24 Jul 2025 14:30:19 -0400 Subject: [PATCH 1/5] First draft of add nn quick fix --- .../dotty/tools/dotc/reporting/messages.scala | 27 +++++++++++++++++++ .../tools/dotc/reporting/CodeActionTest.scala | 14 ++++++++++ 2 files changed, 41 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 9b032e1e7539..3d3241b861c7 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -300,6 +300,10 @@ extends NotFoundMsg(MissingIdentID) { class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tree], addenda: => String*)(using Context) extends TypeMismatchMsg(found, expected)(TypeMismatchID): + private var shouldSuggestNN = false + // Ensures that shouldSuggestNN will always be correctly computed before `actions` is called + msg + def msg(using Context) = // replace constrained TypeParamRefs and their typevars by their bounds where possible // and the bounds are not f-bounds. @@ -344,6 +348,7 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre val (found2, expected2) = if (found1 frozen_<:< expected1) || reported.fbounded then (found, expected) else (found1, expected1) + if found2 frozen_<:< OrNull(expected) then shouldSuggestNN = true val (foundStr, expectedStr) = Formatting.typeDiff(found2.normalized, expected2.normalized) i"""|Found: $foundStr |Required: $expectedStr${reported.notes}""" @@ -360,6 +365,28 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre val treeStr = inTree.map(x => s"\nTree:\n\n${x.show}\n").getOrElse("") treeStr + "\n" + super.explain + override def actions(using Context) = + if shouldSuggestNN then + inTree match { + case Some(tree) if tree != null => + val content = tree.source.content().slice(tree.srcPos.startPos.start, tree.srcPos.endPos.end).mkString + val replacement = tree match + case a @ Apply(fun, args) => "(" + content + ").nn" + case _ => content + List( + CodeAction(title = """Add .nn""", + description = None, + patches = List( + ActionPatch(tree.srcPos.sourcePos, replacement) + ) + ) + ) + case _ => + List() + } + else + List() + end TypeMismatch class NotAMember(site: Type, val name: Name, selected: String, proto: Type, addendum: => String = "")(using Context) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 91074110389e..14834b5d8760 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -179,6 +179,20 @@ class CodeActionTest extends DottyTest: ctxx = ctxx ) + @Test def addNN = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """val s: String|Null = ??? + | val t: String = s""".stripMargin, + title = "Add .nn", + expected = + """val s: String|Null = ??? + | val t: String = (s).nn""".stripMargin, + ctxx = ctxx + ) + // Make sure we're not using the default reporter, which is the ConsoleReporter, // meaning they will get reported in the test run and that's it. private def newContext = From 6c2147d6de4de8fffab4269edd9d42703c7d0f25 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Fri, 25 Jul 2025 13:38:01 -0400 Subject: [PATCH 2/5] Only suggest .nn on value types --- compiler/src/dotty/tools/dotc/reporting/messages.scala | 10 +++++----- .../dotty/tools/dotc/reporting/CodeActionTest.scala | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 3d3241b861c7..0a590ca70ec8 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -300,9 +300,10 @@ extends NotFoundMsg(MissingIdentID) { class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tree], addenda: => String*)(using Context) extends TypeMismatchMsg(found, expected)(TypeMismatchID): - private var shouldSuggestNN = false - // Ensures that shouldSuggestNN will always be correctly computed before `actions` is called - msg + private val shouldSuggestNN = + if expected.isValueType then + found frozen_<:< OrNull(expected) + else false def msg(using Context) = // replace constrained TypeParamRefs and their typevars by their bounds where possible @@ -348,7 +349,6 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre val (found2, expected2) = if (found1 frozen_<:< expected1) || reported.fbounded then (found, expected) else (found1, expected1) - if found2 frozen_<:< OrNull(expected) then shouldSuggestNN = true val (foundStr, expectedStr) = Formatting.typeDiff(found2.normalized, expected2.normalized) i"""|Found: $foundStr |Required: $expectedStr${reported.notes}""" @@ -372,7 +372,7 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre val content = tree.source.content().slice(tree.srcPos.startPos.start, tree.srcPos.endPos.end).mkString val replacement = tree match case a @ Apply(fun, args) => "(" + content + ").nn" - case _ => content + case _ => content + ".nn" List( CodeAction(title = """Add .nn""", description = None, diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 14834b5d8760..ddbdb6b3531a 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -189,7 +189,7 @@ class CodeActionTest extends DottyTest: title = "Add .nn", expected = """val s: String|Null = ??? - | val t: String = (s).nn""".stripMargin, + | val t: String = s.nn""".stripMargin, ctxx = ctxx ) From 8229c8b01fe23a0798641319ba356fa2975068d0 Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Fri, 25 Jul 2025 14:23:09 -0400 Subject: [PATCH 3/5] Remove unnecessary pattern binding and add more tests --- .../dotty/tools/dotc/reporting/messages.scala | 2 +- .../tools/dotc/reporting/CodeActionTest.scala | 59 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 0a590ca70ec8..d13e8a0694a5 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -371,7 +371,7 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre case Some(tree) if tree != null => val content = tree.source.content().slice(tree.srcPos.startPos.start, tree.srcPos.endPos.end).mkString val replacement = tree match - case a @ Apply(fun, args) => "(" + content + ").nn" + case Apply(fun, args) => "(" + content + ").nn" case _ => content + ".nn" List( CodeAction(title = """Add .nn""", diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index ddbdb6b3531a..c9c83c1a4d2f 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -193,6 +193,65 @@ class CodeActionTest extends DottyTest: ctxx = ctxx ) + @Test def addNN2 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """implicit class infixOpTest(val s1: String) extends AnyVal { + | def q(s2: String): String | Null = null + |} + | val s: String = ??? + | val t: String = s q s""".stripMargin, + title = "Add .nn", + expected = + """implicit class infixOpTest(val s1: String) extends AnyVal { + | def q(s2: String): String | Null = null + |} + | val s: String = ??? + | val t: String = (s q s).nn""".stripMargin, + ctxx = ctxx + ) + + @Test def addNN3 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """implicit class infixOpTest(val s1: String) extends AnyVal { + | def q(s2: String, s3: String): String | Null = null + |} + | val s: String = ??? + | val t: String = s q (s, s)""".stripMargin, + title = "Add .nn", + expected = + """implicit class infixOpTest(val s1: String) extends AnyVal { + | def q(s2: String, s3: String): String | Null = null + |} + | val s: String = ??? + | val t: String = (s q (s, s)).nn""".stripMargin, + ctxx = ctxx + ) + + @Test def addNN4 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """implicit class infixOpTest(val s1: String) extends AnyVal { + | def q(s2: String, s3: String): String | Null = null + |} + | val s: String = ??? + | val t: String = s.q(s, s)""".stripMargin, + title = "Add .nn", + expected = + """implicit class infixOpTest(val s1: String) extends AnyVal { + | def q(s2: String, s3: String): String | Null = null + |} + | val s: String = ??? + | val t: String = (s.q(s, s)).nn""".stripMargin, + ctxx = ctxx + ) // Make sure we're not using the default reporter, which is the ConsoleReporter, // meaning they will get reported in the test run and that's it. private def newContext = From 70a169a9dffce37664deef8d3e53f7aefdae6c5b Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Fri, 1 Aug 2025 16:11:27 -0400 Subject: [PATCH 4/5] Add SafeNulls check before computing subtyping relation, fix edge cases for in-line match and if, improve formatting, and add more test Signed-off-by: Seyon Sivatharan --- .../dotty/tools/dotc/reporting/messages.scala | 32 ++++++----- .../tools/dotc/reporting/CodeActionTest.scala | 53 ++++++++++++++++++- 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index d13e8a0694a5..dde8b844a385 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -301,7 +301,7 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre extends TypeMismatchMsg(found, expected)(TypeMismatchID): private val shouldSuggestNN = - if expected.isValueType then + if ctx.mode.is(Mode.SafeNulls) && expected.isValueType then found frozen_<:< OrNull(expected) else false @@ -366,27 +366,25 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre treeStr + "\n" + super.explain override def actions(using Context) = - if shouldSuggestNN then - inTree match { - case Some(tree) if tree != null => - val content = tree.source.content().slice(tree.srcPos.startPos.start, tree.srcPos.endPos.end).mkString - val replacement = tree match - case Apply(fun, args) => "(" + content + ").nn" - case _ => content + ".nn" - List( - CodeAction(title = """Add .nn""", + inTree match { + case Some(tree) if shouldSuggestNN => + val content = tree.source.content().slice(tree.srcPos.startPos.start, tree.srcPos.endPos.end).mkString + val replacement = tree match + case a @ Apply(_, _) if a.applyKind == ApplyKind.Using => + content + ".nn" + case _ @ (Select(_, _) | Ident(_)) => content + ".nn" + case _ => "(" + content + ").nn" + List( + CodeAction(title = """Add .nn""", description = None, patches = List( ActionPatch(tree.srcPos.sourcePos, replacement) ) - ) ) - case _ => - List() - } - else - List() - + ) + case _ => + List() + } end TypeMismatch class NotAMember(site: Type, val name: Name, selected: String, proto: Type, addendum: => String = "")(using Context) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index c9c83c1a4d2f..17282532f801 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -179,7 +179,7 @@ class CodeActionTest extends DottyTest: ctxx = ctxx ) - @Test def addNN = + @Test def addNN1 = val ctxx = newContext ctxx.setSetting(ctxx.settings.YexplicitNulls, true) checkCodeAction( @@ -252,6 +252,57 @@ class CodeActionTest extends DottyTest: | val t: String = (s.q(s, s)).nn""".stripMargin, ctxx = ctxx ) + + @Test def addNN5 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """val s: String | Null = ??? + |val t: String = s match { + | case _: String => "foo" + | case _ => s + |}""".stripMargin, + title = "Add .nn", + expected = + """val s: String | Null = ??? + |val t: String = s match { + | case _: String => "foo" + | case _ => s.nn + |}""".stripMargin, + ctxx = ctxx + ) + + @Test def addNN6 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """val s: String | Null = ??? + |val t: String = if (s != null) "foo" else s""".stripMargin, + title = "Add .nn", + expected = + """val s: String | Null = ??? + |val t: String = if (s != null) "foo" else s.nn""".stripMargin, + ctxx = ctxx + ) + + @Test def addNN7 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """given ctx: String | Null = null + |def f(using c: String): String = c + |val s: String = f(using ctx)""".stripMargin, + title = "Add .nn", + expected = + """given ctx: String | Null = null + |def f(using c: String): String = c + |val s: String = f(using ctx.nn)""".stripMargin, + ctxx = ctxx + ) + // Make sure we're not using the default reporter, which is the ConsoleReporter, // meaning they will get reported in the test run and that's it. private def newContext = From bda8348d8e64e00e5527cacd2a9d3d875dcd93d6 Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Tue, 12 Aug 2025 15:40:25 -0400 Subject: [PATCH 5/5] Add WasTypedInfix sticky attachment and push it to Apply nodes that are typed infix --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 6 +++++- compiler/src/dotty/tools/dotc/reporting/messages.scala | 3 ++- .../test/dotty/tools/dotc/reporting/CodeActionTest.scala | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index d71a6329e8b0..51e601441ead 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -67,6 +67,8 @@ object desugar { */ val TrailingForMap: Property.Key[Unit] = Property.StickyKey() + val WasTypedInfix: Property.Key[Unit] = Property.StickyKey() + /** What static check should be applied to a Match? */ enum MatchCheck { case None, Exhaustive, IrrefutablePatDef, IrrefutableGenFrom @@ -1667,10 +1669,12 @@ object desugar { case _ => Apply(sel, arg :: Nil) - if op.name.isRightAssocOperatorName then + val apply = if op.name.isRightAssocOperatorName then makeOp(right, left, Span(op.span.start, right.span.end)) else makeOp(left, right, Span(left.span.start, op.span.end, op.span.start)) + apply.pushAttachment(WasTypedInfix, ()) + return apply } /** Translate throws type `A throws E1 | ... | En` to diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index dde8b844a385..4cc8411a4fce 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -14,6 +14,7 @@ import printing.Highlighting.* import printing.Formatting import ErrorMessageID.* import ast.Trees +import ast.desugar import config.{Feature, MigrationVersion, ScalaVersion} import transform.patmat.Space import transform.patmat.SpaceEngine @@ -370,7 +371,7 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre case Some(tree) if shouldSuggestNN => val content = tree.source.content().slice(tree.srcPos.startPos.start, tree.srcPos.endPos.end).mkString val replacement = tree match - case a @ Apply(_, _) if a.applyKind == ApplyKind.Using => + case a @ Apply(_, _) if !a.hasAttachment(desugar.WasTypedInfix) => content + ".nn" case _ @ (Select(_, _) | Ident(_)) => content + ".nn" case _ => "(" + content + ").nn" diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 17282532f801..91ff958a7889 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -249,7 +249,7 @@ class CodeActionTest extends DottyTest: | def q(s2: String, s3: String): String | Null = null |} | val s: String = ??? - | val t: String = (s.q(s, s)).nn""".stripMargin, + | val t: String = s.q(s, s).nn""".stripMargin, ctxx = ctxx )