From f3bf3c093f345ee3b53d94033923265f061724d8 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Sun, 30 Jul 2023 19:30:30 +0200 Subject: [PATCH 1/8] Add actionable item to PatternMatchExhaustivity diag [Cherry-picked 90a3db73af0b294d8c7ed741caa8f2652822242c] --- .../dotty/tools/dotc/reporting/messages.scala | 26 +++++++++++++-- .../tools/dotc/transform/patmat/Space.scala | 7 ++-- .../tools/dotc/reporting/CodeActionTest.scala | 32 ++++++++++++++++--- .../tools/dotc/reporting/TestReporter.scala | 2 +- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index f9b98c44ffc1..82d1dfd2aea7 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -844,10 +844,12 @@ extends Message(LossyWideningConstantConversionID): |Write `.to$targetType` instead.""" def explain(using Context) = "" -class PatternMatchExhaustivity(uncoveredFn: => String, hasMore: Boolean)(using Context) +class PatternMatchExhaustivity(val uncovered: Seq[String], tree: untpd.Match)(using Context) extends Message(PatternMatchExhaustivityID) { def kind = MessageKind.PatternMatchExhaustivity - lazy val uncovered = uncoveredFn + + private val hasMore = uncovered.lengthCompare(6) > 0 + def msg(using Context) = val addendum = if hasMore then "(More unmatched cases are elided)" else "" i"""|${hl("match")} may not be exhaustive. @@ -862,6 +864,26 @@ extends Message(PatternMatchExhaustivityID) { | - If an extractor always return ${hl("Some(...)")}, write ${hl("Some[X]")} for its return type | - Add a ${hl("case _ => ...")} at the end to match all remaining cases |""" + + override def actions(using Context) = + import scala.language.unsafeNulls + val endPos = tree.cases.lastOption.map(_.endPos).getOrElse(tree.selector.endPos) + val startColumn = tree.cases.lastOption.map(_.startPos.startColumn).getOrElse(tree.selector.startPos.startColumn + 2) + val pathes = List( + ActionPatch(endPos, uncovered.map(c=> indent(s"case $c => ???", startColumn)).mkString("\n", "\n", "")), + ) + List( + CodeAction(title = s"Insert missing cases (${uncovered.size})", + description = None, + patches = pathes + ) + ) + + + private def indent(text:String, margin: Int): String = { + import scala.language.unsafeNulls + " " * margin + text + } } class UncheckedTypePattern(msgFn: => String)(using Context) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index bc9e5158f14b..9b523da8c33b 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -771,7 +771,7 @@ object SpaceEngine { checkConstraint(genConstraint(sp))(using ctx.fresh.setNewTyperState()) } - def showSpaces(ss: Seq[Space])(using Context): String = ss.map(show).mkString(", ") + def showSpaces(ss: Seq[Space])(using Context): Seq[String] = ss.map(show) /** Display spaces */ def show(s: Space)(using Context): String = { @@ -896,9 +896,8 @@ object SpaceEngine { if uncovered.nonEmpty then - val hasMore = uncovered.lengthCompare(6) > 0 - val deduped = dedup(uncovered.take(6)) - report.warning(PatternMatchExhaustivity(showSpaces(deduped), hasMore), m.selector) + val deduped = dedup(uncovered) + report.warning(PatternMatchExhaustivity(showSpaces(deduped), m), m.selector) } private def redundancyCheckable(sel: Tree)(using Context): Boolean = diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 1740542b46b0..d38f1ef5028e 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -52,7 +52,31 @@ class CodeActionTest extends DottyTest: // TODO look into trying to remove the extra space that is left behind """|final class Test |""".stripMargin + ) + @Test def insertMissingCases = + checkCodeAction( + code = """|enum Tree: + | case Node(l: Tree, r: Tree) + | case Leaf(v: String) + | + |object Test: + | def foo(tree: Tree) = tree match { + | case Tree.Node(_, _) => ??? + | } + |""".stripMargin, + title = "Insert missing cases (1)", + expected = """|enum Tree: + | case Node(l: Tree, r: Tree) + | case Leaf(v: String) + | + |object Test: + | def foo(tree: Tree) = tree match { + | case Tree.Node(_, _) => ??? + | case Tree.Leaf(_) => ??? + | } + |""".stripMargin, + afterPhase = "patternMatcher" ) // Make sure we're not using the default reporter, which is the ConsoleReporter, @@ -61,16 +85,16 @@ class CodeActionTest extends DottyTest: val rep = new StoreReporter(null) with UniqueMessagePositions with HideNonSensicalMessages initialCtx.setReporter(rep).withoutColors - private def checkCodeAction(code: String, title: String, expected: String) = + private def checkCodeAction(code: String, title: String, expected: String, afterPhase: String = "typer") = ctx = newContext val source = SourceFile.virtual("test", code).content - val runCtx = checkCompile("typer", code) { (_, _) => () } + val runCtx = checkCompile(afterPhase, code) { (_, _) => () } val diagnostics = runCtx.reporter.removeBufferedMessages - assertEquals(1, diagnostics.size) + assertEquals("Expected exactly one diagnostic", 1, diagnostics.size) val diagnostic = diagnostics.head val actions = diagnostic.msg.actions.toList - assertEquals(1, actions.size) + assertEquals("Expected exactly one action", 1, actions.size) // TODO account for more than 1 action val action = actions.head diff --git a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala index 940fc875a021..52f57b7066ae 100644 --- a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala +++ b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala @@ -75,7 +75,7 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M // Here we add extra information that we should know about the error message val extra = dia.msg match { - case pm: PatternMatchExhaustivity => s": ${pm.uncovered}" + case pm: PatternMatchExhaustivity => s": ${pm.uncovered.mkString("\n")}" case _ => "" } From dd664a33fac31d0bc3f5d60bdece5967c46265c8 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Mon, 31 Jul 2023 07:45:11 +0200 Subject: [PATCH 2/8] Fix manually code format [Cherry-picked 38844efcd097a8859d506772efbce56c929d7369] --- .../tools/dotc/reporting/CodeActionTest.scala | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index d38f1ef5028e..873300dba6a6 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -56,26 +56,28 @@ class CodeActionTest extends DottyTest: @Test def insertMissingCases = checkCodeAction( - code = """|enum Tree: - | case Node(l: Tree, r: Tree) - | case Leaf(v: String) - | - |object Test: - | def foo(tree: Tree) = tree match { - | case Tree.Node(_, _) => ??? - | } - |""".stripMargin, + code = + """|enum Tree: + | case Node(l: Tree, r: Tree) + | case Leaf(v: String) + | + |object Test: + | def foo(tree: Tree) = tree match { + | case Tree.Node(_, _) => ??? + | } + |""".stripMargin, title = "Insert missing cases (1)", - expected = """|enum Tree: - | case Node(l: Tree, r: Tree) - | case Leaf(v: String) - | - |object Test: - | def foo(tree: Tree) = tree match { - | case Tree.Node(_, _) => ??? - | case Tree.Leaf(_) => ??? - | } - |""".stripMargin, + expected = + """|enum Tree: + | case Node(l: Tree, r: Tree) + | case Leaf(v: String) + | + |object Test: + | def foo(tree: Tree) = tree match { + | case Tree.Node(_, _) => ??? + | case Tree.Leaf(_) => ??? + | } + |""".stripMargin, afterPhase = "patternMatcher" ) From 9da99a2fb243f9fdf2a07ce7c51169b00fa3806b Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Mon, 31 Jul 2023 07:48:12 +0200 Subject: [PATCH 3/8] Add failing test for union string type [Cherry-picked 7df226611ffe9b117df352566e1ab81b58a4f79b] --- .../tools/dotc/reporting/CodeActionTest.scala | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 873300dba6a6..3cba43c216ef 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -81,6 +81,25 @@ class CodeActionTest extends DottyTest: afterPhase = "patternMatcher" ) + @Test def insertMissingCasesForUnionStringType = + checkCodeAction( + code = + """object Test: + | def foo(text: "Alice" | "Bob") = text match { + | case "Alice" => ??? + | } + |""".stripMargin, + title = "Insert missing cases (1)", + expected = + """object Test: + | def foo(text: "Alice" | "Bob") = text match { + | case "Alice" => ??? + | case "Bob" => ??? + | } + |""".stripMargin, + afterPhase = "patternMatcher" + ) + // 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 6b4a9eaa45ccba8cf028f48ec9d6f14b113d9fdf Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Mon, 31 Jul 2023 11:21:18 +0200 Subject: [PATCH 4/8] Restore limit of cases in the reporter message [Cherry-picked 0e47303bc0fc6000a5d178e94a432607feb1682e] --- compiler/src/dotty/tools/dotc/reporting/messages.scala | 9 +++++---- .../test/dotty/tools/dotc/reporting/TestReporter.scala | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 82d1dfd2aea7..0c9297c1aaa0 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -844,11 +844,12 @@ extends Message(LossyWideningConstantConversionID): |Write `.to$targetType` instead.""" def explain(using Context) = "" -class PatternMatchExhaustivity(val uncovered: Seq[String], tree: untpd.Match)(using Context) +class PatternMatchExhaustivity(uncoveredCases: Seq[String], tree: untpd.Match)(using Context) extends Message(PatternMatchExhaustivityID) { def kind = MessageKind.PatternMatchExhaustivity - private val hasMore = uncovered.lengthCompare(6) > 0 + private val hasMore = uncoveredCases.lengthCompare(6) > 0 + val uncovered = uncoveredCases.take(6).mkString(", ") def msg(using Context) = val addendum = if hasMore then "(More unmatched cases are elided)" else "" @@ -870,10 +871,10 @@ extends Message(PatternMatchExhaustivityID) { val endPos = tree.cases.lastOption.map(_.endPos).getOrElse(tree.selector.endPos) val startColumn = tree.cases.lastOption.map(_.startPos.startColumn).getOrElse(tree.selector.startPos.startColumn + 2) val pathes = List( - ActionPatch(endPos, uncovered.map(c=> indent(s"case $c => ???", startColumn)).mkString("\n", "\n", "")), + ActionPatch(endPos, uncoveredCases.map(c=> indent(s"case $c => ???", startColumn)).mkString("\n", "\n", "")), ) List( - CodeAction(title = s"Insert missing cases (${uncovered.size})", + CodeAction(title = s"Insert missing cases (${uncoveredCases.size})", description = None, patches = pathes ) diff --git a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala index 52f57b7066ae..940fc875a021 100644 --- a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala +++ b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala @@ -75,7 +75,7 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M // Here we add extra information that we should know about the error message val extra = dia.msg match { - case pm: PatternMatchExhaustivity => s": ${pm.uncovered.mkString("\n")}" + case pm: PatternMatchExhaustivity => s": ${pm.uncovered}" case _ => "" } From 3636d21504d6db1b10719f8b4947583e255bb0f5 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Mon, 31 Jul 2023 14:32:00 +0200 Subject: [PATCH 5/8] Fix string literals rendering [Cherry-picked f039f7d404c04fcce2cf1e6f5f372c548a51ed3e] --- .../tools/dotc/transform/patmat/Space.scala | 3 ++- .../tools/dotc/reporting/CodeActionTest.scala | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 9b523da8c33b..bbb47a7bd667 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -786,7 +786,8 @@ object SpaceEngine { def doShow(s: Space, flattenList: Boolean = false): String = s match { case Empty => "empty" - case Typ(c: ConstantType, _) => "" + c.value.value + case Typ(ConstantType(const : Constant), _) if const.tag == StringTag => "\"" + const.value + "\"" + case Typ(ConstantType(const : Constant), _) => "" + const.value case Typ(tp: TermRef, _) => if (flattenList && tp <:< defn.NilType) "" else tp.symbol.showName diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 3cba43c216ef..96e0a5afd4a8 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -100,6 +100,25 @@ class CodeActionTest extends DottyTest: afterPhase = "patternMatcher" ) + @Test def insertMissingCasesForUnionIntType = + checkCodeAction( + code = + """object Test: + | def foo(text: 1 | 2) = text match { + | case 2 => ??? + | } + |""".stripMargin, + title = "Insert missing cases (1)", + expected = + """object Test: + | def foo(text: 1 | 2) = text match { + | case 2 => ??? + | case 1 => ??? + | } + |""".stripMargin, + afterPhase = "patternMatcher" + ) + // 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 c329fc5827d94a168c3ac36ae03e41202911e642 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Mon, 31 Jul 2023 14:35:12 +0200 Subject: [PATCH 6/8] Format code [Cherry-picked 363a1e26ad3abc453a001adf23e9f27ad2c0a4bb] --- .../dotty/tools/dotc/reporting/messages.scala | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 0c9297c1aaa0..03a3d8e57438 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -868,11 +868,19 @@ extends Message(PatternMatchExhaustivityID) { override def actions(using Context) = import scala.language.unsafeNulls - val endPos = tree.cases.lastOption.map(_.endPos).getOrElse(tree.selector.endPos) - val startColumn = tree.cases.lastOption.map(_.startPos.startColumn).getOrElse(tree.selector.startPos.startColumn + 2) + val endPos = tree.cases.lastOption.map(_.endPos) + .getOrElse(tree.selector.endPos) + val startColumn = tree.cases.lastOption + .map(_.startPos.startColumn) + .getOrElse(tree.selector.startPos.startColumn + 2) + val pathes = List( - ActionPatch(endPos, uncoveredCases.map(c=> indent(s"case $c => ???", startColumn)).mkString("\n", "\n", "")), - ) + ActionPatch( + srcPos = endPos, + replacement = uncoveredCases.map(c => indent(s"case $c => ???", startColumn)) + .mkString("\n", "\n", "") + ), + ) List( CodeAction(title = s"Insert missing cases (${uncoveredCases.size})", description = None, From 8407e321933780bdd90ea8833f04a15a6f55f452 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Tue, 1 Aug 2023 18:52:42 +0200 Subject: [PATCH 7/8] Simplify const rendering [Cherry-picked 8386b64f4cfe4b9aa6b95606dc0e142e5954e56a] --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index bbb47a7bd667..2464ca448763 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -786,8 +786,7 @@ object SpaceEngine { def doShow(s: Space, flattenList: Boolean = false): String = s match { case Empty => "empty" - case Typ(ConstantType(const : Constant), _) if const.tag == StringTag => "\"" + const.value + "\"" - case Typ(ConstantType(const : Constant), _) => "" + const.value + case Typ(c: ConstantType, _) => c.value.show case Typ(tp: TermRef, _) => if (flattenList && tp <:< defn.NilType) "" else tp.symbol.showName From 4623a102c6a949c2e06f709bebdccb0e3b865bd5 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Sat, 5 Aug 2023 11:41:56 +0200 Subject: [PATCH 8/8] Add test with braceless syntax [Cherry-picked 1a592d3cb68c82baa2876bb55f8cd036d831cebf] --- .../tools/dotc/reporting/CodeActionTest.scala | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 96e0a5afd4a8..870da08dcfba 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -119,6 +119,23 @@ class CodeActionTest extends DottyTest: afterPhase = "patternMatcher" ) + @Test def insertMissingCasesUsingBracelessSyntax = + checkCodeAction( + code = + """object Test: + | def foo(text: 1 | 2) = text match + | case 2 => ??? + |""".stripMargin, + title = "Insert missing cases (1)", + expected = + """object Test: + | def foo(text: 1 | 2) = text match + | case 2 => ??? + | case 1 => ??? + |""".stripMargin, + afterPhase = "patternMatcher" + ) + // 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 =