Skip to content

Add quick fix to add .nn #23598

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
27 changes: 27 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ 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 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
// and the bounds are not f-bounds.
Expand Down Expand Up @@ -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 Apply(fun, args) => "(" + content + ").nn"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safer to add "( )" by default? For example, (if ... then ... else ...).nn. Then, we only add "( )" if the tree is a ident, select, or a regular apply?

Copy link
Contributor Author

@SuperCl4sh SuperCl4sh Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like the best approach. The only problem I have is detecting a regular apply. Binary infix operators have ApplyKind Regular and I couldn't find another way to determine if Apply's were initially written as a regular non-infix apply in the source code. Do you know if there's a way to detect "true" regular apply's?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've addressed all the other comments in the latest commit, but I'm still unsure how to detect functions written Infix. For the time being, both ApplyKind Regular and InfixTuple have "()" added to them while ApplyKind Using has no brackets.

case _ => content + ".nn"
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)
Expand Down
73 changes: 73 additions & 0 deletions compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,79 @@ 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
)

@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 =
Expand Down
Loading