From 62d73eacac0272c6b5a2fe7890949030702e1e56 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Mon, 22 May 2017 18:41:07 +0200 Subject: [PATCH 1/4] fix #2502: improve warning message of exhaustivity check --- .../dotc/reporting/diagnostic/messages.scala | 7 ++++++- .../tools/dotc/transform/patmat/Space.scala | 2 +- tests/patmat/i2502.check | 1 + tests/patmat/i2502.scala | 17 +++++++++++++++++ tests/patmat/t4691.check | 2 +- 5 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 tests/patmat/i2502.check create mode 100644 tests/patmat/i2502.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 69c8dba57ad8..bf5a7923b046 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -789,7 +789,12 @@ object messages { |It would fail on: $uncovered""" - val explanation = "" + val explanation = + hl"""|There are several ways to make the match exhaustive: + | - add missing cases as shown in the warning + | - change the return type of irrefutable extractors as `Some[T]` if exist + | - add a wildcard `_` case at the end + |""" } case class MatchCaseUnreachable()(implicit ctx: Context) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 96c1da50b62e..a9703d330a9d 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -657,7 +657,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { else if (tp.classSymbol.is(CaseClass)) // use constructor syntax for case class showType(tp) + signature(tp).map(_ => "_").mkString("(", ", ", ")") - else if (signature(tp).nonEmpty) + else if (sym.is(Flags.Case) && signature(tp).nonEmpty) tp.classSymbol.name + signature(tp).map(_ => "_").mkString("(", ", ", ")") else if (decomposed) "_: " + showType(tp) else "_" diff --git a/tests/patmat/i2502.check b/tests/patmat/i2502.check new file mode 100644 index 000000000000..fe2194e650cf --- /dev/null +++ b/tests/patmat/i2502.check @@ -0,0 +1 @@ +5: Pattern Match Exhaustivity: _: BTypes.ClassBType \ No newline at end of file diff --git a/tests/patmat/i2502.scala b/tests/patmat/i2502.scala new file mode 100644 index 000000000000..2ccb20c135c5 --- /dev/null +++ b/tests/patmat/i2502.scala @@ -0,0 +1,17 @@ +abstract class BTypes { + trait BType + + sealed trait RefBType extends BType { + def classOrArrayType: String = this match { + case ClassBType(internalName) => internalName + case a: ArrayBType => "" + } + } + + final class ClassBType(val internalName: String) extends RefBType + class ArrayBType extends RefBType + + object ClassBType { + def unapply(x: ClassBType): Option[String] = None + } +} \ No newline at end of file diff --git a/tests/patmat/t4691.check b/tests/patmat/t4691.check index 5fbcb267ab80..692dcd358f45 100644 --- a/tests/patmat/t4691.check +++ b/tests/patmat/t4691.check @@ -1 +1 @@ -15: Pattern Match Exhaustivity: NodeType2(_) +15: Pattern Match Exhaustivity: _: NodeType2 From e820624c86ac2a23242eefac2c64a242284106d0 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 23 May 2017 09:33:34 +0200 Subject: [PATCH 2/4] address review feedback --- .../dotc/reporting/diagnostic/messages.scala | 6 +++--- .../tools/dotc/transform/patmat/Space.scala | 12 +++++++++--- tests/patmat/i2502b.check | 1 + tests/patmat/i2502b.scala | 17 +++++++++++++++++ 4 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 tests/patmat/i2502b.check create mode 100644 tests/patmat/i2502b.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index bf5a7923b046..096f2234f4e8 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -791,9 +791,9 @@ object messages { val explanation = hl"""|There are several ways to make the match exhaustive: - | - add missing cases as shown in the warning - | - change the return type of irrefutable extractors as `Some[T]` if exist - | - add a wildcard `_` case at the end + | - Add missing cases as shown in the warning + | - If an extractor always return Some(...), write Some[X] for its return type + | - Add a case _ => ... at the end to match all remaining cases |""" } diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index a9703d330a9d..6c7b04c779ff 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -641,6 +641,14 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { /** Display spaces */ def show(s: Space): String = { + + // does the companion object of the given symbol have custom unapply + def hasCustomUnapply(sym: Symbol): Boolean = { + val companion = sym.companionModule + companion.findMember(nme.unapply, NoPrefix, Flags.Synthetic).exists || + companion.findMember(nme.unapplySeq, NoPrefix, Flags.Synthetic).exists + } + def doShow(s: Space, mergeList: Boolean = false): String = s match { case Empty => "" case Typ(c: ConstantType, _) => c.value.show @@ -654,11 +662,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { if (mergeList) "_*" else "_: List" else if (scalaConsType.isRef(sym)) if (mergeList) "_" else "List(_)" - else if (tp.classSymbol.is(CaseClass)) + else if (tp.classSymbol.is(CaseClass) && !hasCustomUnapply(tp.classSymbol)) // use constructor syntax for case class showType(tp) + signature(tp).map(_ => "_").mkString("(", ", ", ")") - else if (sym.is(Flags.Case) && signature(tp).nonEmpty) - tp.classSymbol.name + signature(tp).map(_ => "_").mkString("(", ", ", ")") else if (decomposed) "_: " + showType(tp) else "_" case Kon(tp, params) => diff --git a/tests/patmat/i2502b.check b/tests/patmat/i2502b.check new file mode 100644 index 000000000000..fe2194e650cf --- /dev/null +++ b/tests/patmat/i2502b.check @@ -0,0 +1 @@ +5: Pattern Match Exhaustivity: _: BTypes.ClassBType \ No newline at end of file diff --git a/tests/patmat/i2502b.scala b/tests/patmat/i2502b.scala new file mode 100644 index 000000000000..9550c84973d9 --- /dev/null +++ b/tests/patmat/i2502b.scala @@ -0,0 +1,17 @@ +abstract class BTypes { + trait BType + + sealed trait RefBType extends BType { + def classOrArrayType: String = this match { + case ClassBType(internalName) => internalName + case a: ArrayBType => "" + } + } + + final case class ClassBType(val internalName: String) extends RefBType + class ArrayBType extends RefBType + + object ClassBType { + def unapply(x: RefBType): Option[String] = None + } +} From 7de84b49e53621ef2a1e112c0ac0b3e6b2dc4132 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 23 May 2017 09:36:13 +0200 Subject: [PATCH 3/4] add quote around code in explanation message --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 096f2234f4e8..9934ebb716a5 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -792,8 +792,8 @@ object messages { val explanation = hl"""|There are several ways to make the match exhaustive: | - Add missing cases as shown in the warning - | - If an extractor always return Some(...), write Some[X] for its return type - | - Add a case _ => ... at the end to match all remaining cases + | - If an extractor always return 'Some(...)', write 'Some[X]' for its return type + | - Add a 'case _ => ...' at the end to match all remaining cases |""" } From 14276cdace82279c4f348f4e331afc647d171c58 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 23 May 2017 13:04:55 +0200 Subject: [PATCH 4/4] use named args to avoid confusion --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 6c7b04c779ff..d37002094372 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -642,11 +642,11 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { /** Display spaces */ def show(s: Space): String = { - // does the companion object of the given symbol have custom unapply + /** does the companion object of the given symbol have custom unapply */ def hasCustomUnapply(sym: Symbol): Boolean = { val companion = sym.companionModule - companion.findMember(nme.unapply, NoPrefix, Flags.Synthetic).exists || - companion.findMember(nme.unapplySeq, NoPrefix, Flags.Synthetic).exists + companion.findMember(nme.unapply, NoPrefix, excluded = Synthetic).exists || + companion.findMember(nme.unapplySeq, NoPrefix, excluded = Synthetic).exists } def doShow(s: Space, mergeList: Boolean = false): String = s match {