From becafe4e79a95ea26fdcee5d6fd9a901f4369deb Mon Sep 17 00:00:00 2001 From: zoltanelek Date: Mon, 21 May 2018 18:22:11 +0200 Subject: [PATCH 1/6] Closes #4300 adds better worded pattern match unreachability warning - Used when isInstanceOf is called inside a match case - The new message is "this case will always be taken because the scrutinee has type ``" - Highlights the position of `x: Int` instead of `x` --- .../dotty/tools/dotc/transform/TypeTestsCasts.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index c40cc7b213f2..147a1ad4cd8e 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -90,9 +90,14 @@ object TypeTestsCasts { if (expr.tpe <:< testType) if (expr.tpe.isNotNull) { - ctx.warning( - em"this will always yield true, since `$foundCls` is a subclass of `$testCls`", - expr.pos) + if (inMatch) + ctx.warning( + em"this case will always be taken because the scrutinee has type `$testType`", + tree.pos) + else + ctx.warning( + em"this will always yield true, since `$foundCls` is a subclass of `$testCls`", + expr.pos) constant(expr, Literal(Constant(true))) } else expr.testNotNull From 6834571131f1a3b2c64ae833d269165bbe176633 Mon Sep 17 00:00:00 2001 From: fhenrir Date: Fri, 8 Jun 2018 20:28:52 +0200 Subject: [PATCH 2/6] changed pattern match reachability warning, to reflect more complex matches on tuples --- compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 147a1ad4cd8e..3b2ab252eb85 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -92,7 +92,7 @@ object TypeTestsCasts { if (expr.tpe.isNotNull) { if (inMatch) ctx.warning( - em"this case will always be taken because the scrutinee has type `$testType`", + em"the highlighted part of the pattern match case statement will always succeed, since `$foundCls` is a subclass of `$testCls`", tree.pos) else ctx.warning( From f55d4975ca9781038a0ee793a6f71a050da88caf Mon Sep 17 00:00:00 2001 From: fhenrir Date: Fri, 8 Jun 2018 20:57:56 +0200 Subject: [PATCH 3/6] clearer error message for pattern matching on objects --- compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 3b2ab252eb85..0e019c2c057e 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -92,7 +92,8 @@ object TypeTestsCasts { if (expr.tpe.isNotNull) { if (inMatch) ctx.warning( - em"the highlighted part of the pattern match case statement will always succeed, since `$foundCls` is a subclass of `$testCls`", + em"the type check in the highlighted part of the pattern match case statement " + + em"will always succeed, since `$foundCls` is a subclass of `$testCls`", tree.pos) else ctx.warning( From 215f0bff60fabee1583dfd7e91e0c6c57aa1721f Mon Sep 17 00:00:00 2001 From: fhenrir Date: Fri, 8 Jun 2018 21:52:37 +0200 Subject: [PATCH 4/6] Created a proper error message case class --- .../reporting/diagnostic/ErrorMessageID.java | 1 + .../dotc/reporting/diagnostic/messages.scala | 34 +++++++++++++++++++ .../tools/dotc/transform/TypeTestsCasts.scala | 6 ++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index b9941ee11151..b6c7c42c646d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -130,6 +130,7 @@ public enum ErrorMessageID { DoubleDeclarationID, MatchCaseOnlyNullWarningID, ImportRenamedTwiceID, + PatternMatchAlwaysSucceedsID, ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index ef685b0dc0f2..7c179f52a762 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -2117,4 +2117,38 @@ object messages { val explanation: String = "" } + case class PatternMatchAlwaysSucceeds(foundCls: Symbol, testCls: Symbol)(implicit ctx: Context) extends Message(PatternMatchAlwaysSucceedsID) { + val kind = "Syntax" + val msg: String = + hl""" + |The type check in the highlighted part of the pattern match case statement will always succeed, + |since `$foundCls` is a subclass of `$testCls`. + """ + val codeExampleValidWarning = "def foo(a: Int) = a match { case x: Int => x }" + val codeExampleDisregardWarning = + hl""" + |object Foo { + | def unapply(x: Int): Option[Int] = if (x % 2 == 0) Some(x) else None + |} + | + |class Test { + | def foo(a: Int) = a match { case Foo(x: Int) => x } + |} + """ + val explanation: String = + hl""" + |The highlighted typecheck will always match. + |For example: + | + |$codeExampleValidWarning + | + |The type of the scrutinee 'a' will always be 'Int', so a the 'case x: Int' + |match will always be successful. Maybe a guard clause was forgotten. + | + |In the following counter-example, because of the unapply method, the + |whole match will not be always successful, only the type check. + |$codeExampleDisregardWarning + | + """ + } } diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 0e019c2c057e..aa314a7ba9bc 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -10,6 +10,7 @@ import ValueClasses._ import SymUtils._ import core.Flags._ import util.Positions._ +import reporting.diagnostic.messages.PatternMatchAlwaysSucceeds import reporting.trace @@ -91,10 +92,7 @@ object TypeTestsCasts { if (expr.tpe <:< testType) if (expr.tpe.isNotNull) { if (inMatch) - ctx.warning( - em"the type check in the highlighted part of the pattern match case statement " + - em"will always succeed, since `$foundCls` is a subclass of `$testCls`", - tree.pos) + ctx.warning(PatternMatchAlwaysSucceeds(foundCls, testCls), tree.pos) else ctx.warning( em"this will always yield true, since `$foundCls` is a subclass of `$testCls`", From bf7c85c28295c024875523a39200eb56adbde96e Mon Sep 17 00:00:00 2001 From: fhenrir Date: Sun, 10 Jun 2018 10:20:34 +0200 Subject: [PATCH 5/6] Common TypeTestAlwaysSucceeds warning case class for pattern match and isInstanceOf --- .../reporting/diagnostic/ErrorMessageID.java | 2 +- .../dotc/reporting/diagnostic/messages.scala | 38 +++---------------- .../tools/dotc/transform/TypeTestsCasts.scala | 9 +---- 3 files changed, 8 insertions(+), 41 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index b6c7c42c646d..87136eda5ebe 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -130,7 +130,7 @@ public enum ErrorMessageID { DoubleDeclarationID, MatchCaseOnlyNullWarningID, ImportRenamedTwiceID, - PatternMatchAlwaysSucceedsID, + TypeTestAlwaysSucceedsID, ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 7c179f52a762..d186d1ff95ce 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -2117,38 +2117,10 @@ object messages { val explanation: String = "" } - case class PatternMatchAlwaysSucceeds(foundCls: Symbol, testCls: Symbol)(implicit ctx: Context) extends Message(PatternMatchAlwaysSucceedsID) { - val kind = "Syntax" - val msg: String = - hl""" - |The type check in the highlighted part of the pattern match case statement will always succeed, - |since `$foundCls` is a subclass of `$testCls`. - """ - val codeExampleValidWarning = "def foo(a: Int) = a match { case x: Int => x }" - val codeExampleDisregardWarning = - hl""" - |object Foo { - | def unapply(x: Int): Option[Int] = if (x % 2 == 0) Some(x) else None - |} - | - |class Test { - | def foo(a: Int) = a match { case Foo(x: Int) => x } - |} - """ - val explanation: String = - hl""" - |The highlighted typecheck will always match. - |For example: - | - |$codeExampleValidWarning - | - |The type of the scrutinee 'a' will always be 'Int', so a the 'case x: Int' - |match will always be successful. Maybe a guard clause was forgotten. - | - |In the following counter-example, because of the unapply method, the - |whole match will not be always successful, only the type check. - |$codeExampleDisregardWarning - | - """ + case class TypeTestAlwaysSucceeds(foundCls: Symbol, testCls: Symbol)(implicit ctx: Context) extends Message(TypeTestAlwaysSucceedsID) { + val kind = "Syntax" + val msg = + s"The highlighted type test will always succeed since the scrutinee type ($foundCls) is a subtype of ${testCls}" + val explanation = "" } } diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index aa314a7ba9bc..39ea428c5ac9 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -10,7 +10,7 @@ import ValueClasses._ import SymUtils._ import core.Flags._ import util.Positions._ -import reporting.diagnostic.messages.PatternMatchAlwaysSucceeds +import reporting.diagnostic.messages.TypeTestAlwaysSucceeds import reporting.trace @@ -91,12 +91,7 @@ object TypeTestsCasts { if (expr.tpe <:< testType) if (expr.tpe.isNotNull) { - if (inMatch) - ctx.warning(PatternMatchAlwaysSucceeds(foundCls, testCls), tree.pos) - else - ctx.warning( - em"this will always yield true, since `$foundCls` is a subclass of `$testCls`", - expr.pos) + ctx.warning(TypeTestAlwaysSucceeds(foundCls, testCls), tree.pos) constant(expr, Literal(Constant(true))) } else expr.testNotNull From 1370dd1022f2bc9744f522a659af4d059692db10 Mon Sep 17 00:00:00 2001 From: fhenrir Date: Sun, 10 Jun 2018 10:24:01 +0200 Subject: [PATCH 6/6] Fixed formatting --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 3 +-- 1 file changed, 1 insertion(+), 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 d186d1ff95ce..93ac0c1832b2 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -2119,8 +2119,7 @@ object messages { case class TypeTestAlwaysSucceeds(foundCls: Symbol, testCls: Symbol)(implicit ctx: Context) extends Message(TypeTestAlwaysSucceedsID) { val kind = "Syntax" - val msg = - s"The highlighted type test will always succeed since the scrutinee type ($foundCls) is a subtype of ${testCls}" + val msg = s"The highlighted type test will always succeed since the scrutinee type ($foundCls) is a subtype of ${testCls}" val explanation = "" } }