Skip to content

Commit c5adafc

Browse files
dwijnanddwijnand
authored
Add actionable item to PatternMatchExhaustivity diagnostic (#18314)
The purpose of this PR is to add an actionable item for non-exhaustive pattern match diagnostic, so that people can auto insert missing cases. Relates to scalameta/metals-feature-requests#350
2 parents 5fc691a + 1a592d3 commit c5adafc

File tree

3 files changed

+122
-11
lines changed

3 files changed

+122
-11
lines changed

‎compiler/src/dotty/tools/dotc/reporting/messages.scala‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -844,10 +844,13 @@ extends Message(LossyWideningConstantConversionID):
844844
|Write `.to$targetType` instead."""
845845
def explain(using Context) = ""
846846

847-
class PatternMatchExhaustivity(uncoveredFn: => String, hasMore: Boolean)(using Context)
847+
class PatternMatchExhaustivity(uncoveredCases: Seq[String], tree: untpd.Match)(using Context)
848848
extends Message(PatternMatchExhaustivityID) {
849849
def kind = MessageKind.PatternMatchExhaustivity
850-
lazy val uncovered = uncoveredFn
850+
851+
private val hasMore = uncoveredCases.lengthCompare(6) > 0
852+
val uncovered = uncoveredCases.take(6).mkString(", ")
853+
851854
def msg(using Context) =
852855
val addendum = if hasMore then "(More unmatched cases are elided)" else ""
853856
i"""|${hl("match")} may not be exhaustive.
@@ -862,6 +865,34 @@ extends Message(PatternMatchExhaustivityID) {
862865
| - If an extractor always return ${hl("Some(...)")}, write ${hl("Some[X]")} for its return type
863866
| - Add a ${hl("case _ => ...")} at the end to match all remaining cases
864867
|"""
868+
869+
override def actions(using Context) =
870+
import scala.language.unsafeNulls
871+
val endPos = tree.cases.lastOption.map(_.endPos)
872+
.getOrElse(tree.selector.endPos)
873+
val startColumn = tree.cases.lastOption
874+
.map(_.startPos.startColumn)
875+
.getOrElse(tree.selector.startPos.startColumn + 2)
876+
877+
val pathes = List(
878+
ActionPatch(
879+
srcPos = endPos,
880+
replacement = uncoveredCases.map(c => indent(s"case $c => ???", startColumn))
881+
.mkString("\n", "\n", "")
882+
),
883+
)
884+
List(
885+
CodeAction(title = s"Insert missing cases (${uncoveredCases.size})",
886+
description = None,
887+
patches = pathes
888+
)
889+
)
890+
891+
892+
private def indent(text:String, margin: Int): String = {
893+
import scala.language.unsafeNulls
894+
" " * margin + text
895+
}
865896
}
866897

867898
class UncheckedTypePattern(msgFn: => String)(using Context)

‎compiler/src/dotty/tools/dotc/transform/patmat/Space.scala‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ object SpaceEngine {
769769
checkConstraint(genConstraint(sp))(using ctx.fresh.setNewTyperState())
770770
}
771771

772-
def showSpaces(ss: Seq[Space])(using Context): String = ss.map(show).mkString(", ")
772+
def showSpaces(ss: Seq[Space])(using Context): Seq[String] = ss.map(show)
773773

774774
/** Display spaces */
775775
def show(s: Space)(using Context): String = {
@@ -784,7 +784,7 @@ object SpaceEngine {
784784

785785
def doShow(s: Space, flattenList: Boolean = false): String = s match {
786786
case Empty => "empty"
787-
case Typ(c: ConstantType, _) => "" + c.value.value
787+
case Typ(c: ConstantType, _) => c.value.show
788788
case Typ(tp: TermRef, _) =>
789789
if (flattenList && tp <:< defn.NilType) ""
790790
else tp.symbol.showName
@@ -894,9 +894,8 @@ object SpaceEngine {
894894

895895

896896
if uncovered.nonEmpty then
897-
val hasMore = uncovered.lengthCompare(6) > 0
898-
val deduped = dedup(uncovered.take(6))
899-
report.warning(PatternMatchExhaustivity(showSpaces(deduped), hasMore), m.selector)
897+
val deduped = dedup(uncovered)
898+
report.warning(PatternMatchExhaustivity(showSpaces(deduped), m), m.selector)
900899
}
901900

902901
private def redundancyCheckable(sel: Tree)(using Context): Boolean =

‎compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala‎

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,88 @@ class CodeActionTest extends DottyTest:
5252
// TODO look into trying to remove the extra space that is left behind
5353
"""|final class Test
5454
|""".stripMargin
55+
)
56+
57+
@Test def insertMissingCases =
58+
checkCodeAction(
59+
code =
60+
"""|enum Tree:
61+
| case Node(l: Tree, r: Tree)
62+
| case Leaf(v: String)
63+
|
64+
|object Test:
65+
| def foo(tree: Tree) = tree match {
66+
| case Tree.Node(_, _) => ???
67+
| }
68+
|""".stripMargin,
69+
title = "Insert missing cases (1)",
70+
expected =
71+
"""|enum Tree:
72+
| case Node(l: Tree, r: Tree)
73+
| case Leaf(v: String)
74+
|
75+
|object Test:
76+
| def foo(tree: Tree) = tree match {
77+
| case Tree.Node(_, _) => ???
78+
| case Tree.Leaf(_) => ???
79+
| }
80+
|""".stripMargin,
81+
afterPhase = "patternMatcher"
82+
)
5583

84+
@Test def insertMissingCasesForUnionStringType =
85+
checkCodeAction(
86+
code =
87+
"""object Test:
88+
| def foo(text: "Alice" | "Bob") = text match {
89+
| case "Alice" => ???
90+
| }
91+
|""".stripMargin,
92+
title = "Insert missing cases (1)",
93+
expected =
94+
"""object Test:
95+
| def foo(text: "Alice" | "Bob") = text match {
96+
| case "Alice" => ???
97+
| case "Bob" => ???
98+
| }
99+
|""".stripMargin,
100+
afterPhase = "patternMatcher"
101+
)
102+
103+
@Test def insertMissingCasesForUnionIntType =
104+
checkCodeAction(
105+
code =
106+
"""object Test:
107+
| def foo(text: 1 | 2) = text match {
108+
| case 2 => ???
109+
| }
110+
|""".stripMargin,
111+
title = "Insert missing cases (1)",
112+
expected =
113+
"""object Test:
114+
| def foo(text: 1 | 2) = text match {
115+
| case 2 => ???
116+
| case 1 => ???
117+
| }
118+
|""".stripMargin,
119+
afterPhase = "patternMatcher"
120+
)
121+
122+
@Test def insertMissingCasesUsingBracelessSyntax =
123+
checkCodeAction(
124+
code =
125+
"""object Test:
126+
| def foo(text: 1 | 2) = text match
127+
| case 2 => ???
128+
|""".stripMargin,
129+
title = "Insert missing cases (1)",
130+
expected =
131+
"""object Test:
132+
| def foo(text: 1 | 2) = text match
133+
| case 2 => ???
134+
| case 1 => ???
135+
|""".stripMargin,
136+
afterPhase = "patternMatcher"
56137
)
57138

58139
// Make sure we're not using the default reporter, which is the ConsoleReporter,
@@ -61,16 +142,16 @@ class CodeActionTest extends DottyTest:
61142
val rep = new StoreReporter(null) with UniqueMessagePositions with HideNonSensicalMessages
62143
initialCtx.setReporter(rep).withoutColors
63144

64-
private def checkCodeAction(code: String, title: String, expected: String) =
145+
private def checkCodeAction(code: String, title: String, expected: String, afterPhase: String = "typer") =
65146
ctx = newContext
66147
val source = SourceFile.virtual("test", code).content
67-
val runCtx = checkCompile("typer", code) { (_, _) => () }
148+
val runCtx = checkCompile(afterPhase, code) { (_, _) => () }
68149
val diagnostics = runCtx.reporter.removeBufferedMessages
69-
assertEquals(1, diagnostics.size)
150+
assertEquals("Expected exactly one diagnostic", 1, diagnostics.size)
70151

71152
val diagnostic = diagnostics.head
72153
val actions = diagnostic.msg.actions.toList
73-
assertEquals(1, actions.size)
154+
assertEquals("Expected exactly one action", 1, actions.size)
74155

75156
// TODO account for more than 1 action
76157
val action = actions.head

0 commit comments

Comments
 (0)