From c5be5dcc4138068834480ac0435381df1c5ec747 Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Thu, 24 Jul 2025 14:30:19 -0400 Subject: [PATCH 1/7] First draft of add nn quick fix --- .../dotty/tools/dotc/reporting/messages.scala | 27 ++++++++++++ .../tools/dotc/reporting/CodeActionTest.scala | 43 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 7b0d525b8703..c0a2ce0a7d40 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -295,6 +295,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. @@ -339,6 +343,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}""" @@ -355,6 +360,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 870da08dcfba..bb5821676d9f 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -136,6 +136,49 @@ class CodeActionTest extends DottyTest: afterPhase = "patternMatcher" ) + @Test def removeNN = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """|val s: String|Null = "foo".nn + |""".stripMargin, + title = "Remove unnecessary .nn", + expected = + """|val s: String|Null = "foo" + |""".stripMargin, + ctxx = ctxx + ) + + + @Test def removeNN2 = + val ctxx = newContext + ctxx.setSetting(ctxx.settings.YexplicitNulls, true) + checkCodeAction( + code = + """val s: String|Null = null.nn + |""".stripMargin, + title = "Remove unnecessary .nn", + expected = + """val s: String|Null = null + |""".stripMargin, + 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 d359e48c0ec7e41bb30c8fdfa45c54273f3c27ad Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 12 Sep 2025 18:48:51 +0200 Subject: [PATCH 2/7] First draft of add nn quick fix [Cherry-picked f952dd2e2b2664d2a3a339b51c3b252e0ee9984d][modified] From 128e6c3bd0f9aa567eb4858202c4ae116709f754 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Fri, 25 Jul 2025 13:38:01 -0400 Subject: [PATCH 3/7] Only suggest .nn on value types [Cherry-picked 6c2147d6de4de8fffab4269edd9d42703c7d0f25] --- 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 c0a2ce0a7d40..e97b8ed15686 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -295,9 +295,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 @@ -343,7 +344,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}""" @@ -367,7 +367,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 bb5821676d9f..fb96f5c714c1 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -175,7 +175,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 8b3b5b8e8db842b745a4a7fe7317ca4f7a280582 Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Fri, 25 Jul 2025 14:23:09 -0400 Subject: [PATCH 4/7] Remove unnecessary pattern binding and add more tests [Cherry-picked 8229c8b01fe23a0798641319ba356fa2975068d0] --- .../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 e97b8ed15686..9a3ab1f27933 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -366,7 +366,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 fb96f5c714c1..f390e861fd33 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -179,6 +179,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 c3f1aee722d4ac821e685c53df1f984d8ebe8850 Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Fri, 1 Aug 2025 16:11:27 -0400 Subject: [PATCH 5/7] 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 [Cherry-picked 70a169a9dffce37664deef8d3e53f7aefdae6c5b] --- .../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 9a3ab1f27933..6fa16f316128 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -296,7 +296,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 @@ -361,27 +361,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 f390e861fd33..bd3d3b62ff7a 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -165,7 +165,7 @@ class CodeActionTest extends DottyTest: ctxx = ctxx ) - @Test def addNN = + @Test def addNN1 = val ctxx = newContext ctxx.setSetting(ctxx.settings.YexplicitNulls, true) checkCodeAction( @@ -238,6 +238,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 6bcec97774538a2874efd9f709b9dcb8a44d6ab5 Mon Sep 17 00:00:00 2001 From: Seyon Sivatharan Date: Tue, 12 Aug 2025 15:40:25 -0400 Subject: [PATCH 6/7] 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 197d0d081f7f..5112dccd2934 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -53,6 +53,8 @@ object desugar { */ val ForArtifact: 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 @@ -1369,10 +1371,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 6fa16f316128..ae98d882a579 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, ScalaVersion} import transform.patmat.Space import transform.patmat.SpaceEngine @@ -365,7 +366,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 bd3d3b62ff7a..ed49ef1172b4 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -235,7 +235,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 ) From 33f52641bc96c268b0ad1a1b5b145fcb78fd966e Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 12 Sep 2025 18:51:24 +0200 Subject: [PATCH 7/7] Add WasTypedInfix sticky attachment and push it to Apply nodes that are typed infix [Cherry-picked bda8348d8e64e00e5527cacd2a9d3d875dcd93d6][modified]