Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public enum ErrorMessageID {
DoubleDeclarationID,
MatchCaseOnlyNullWarningID,
ImportRenamedTwiceID,
PatternMatchAlwaysSucceedsID,
;

public int errorNumber() {
Expand Down
34 changes: 34 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Copy link
Contributor

Choose a reason for hiding this comment

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

type annotation not needed

hl"""
|The type check in the highlighted part of the pattern match case statement will always succeed,
|since `$foundCls` is a subclass of `$testCls`.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Rewrite as:

hl"""...
    |..."""

Otherwise the resulting string has unnecessary line breaks before and after

val codeExampleValidWarning = "def foo(a: Int) = a match { case x: Int => x }"
val codeExampleDisregardWarning =
Copy link
Contributor

Choose a reason for hiding this comment

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

The user should not disregard the warning. The warning is legitimate, he should have written case Foo(x) instead of case Foo(x: Int)

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 =
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the current explanation. I don't think it gives you more information than the msg.

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
|
"""
}
}
10 changes: 7 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import ValueClasses._
import SymUtils._
import core.Flags._
import util.Positions._
import reporting.diagnostic.messages.PatternMatchAlwaysSucceeds
import reporting.trace


Expand Down Expand Up @@ -90,9 +91,12 @@ 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(PatternMatchAlwaysSucceeds(foundCls, testCls), tree.pos)
else
ctx.warning(
em"this will always yield true, since `$foundCls` is a subclass of `$testCls`",
expr.pos)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would merge the two together by making inMatch a parameter of the message class and name the class something like TypeTestAlwaysSucceeds

constant(expr, Literal(Constant(true)))
}
else expr.testNotNull
Expand Down