Skip to content

Commit

Permalink
Better pattern matcher error message.
Browse files Browse the repository at this point in the history
For the common case when someone hasn't quite grokked
the significance of lower case in a pattern match.  I'd
like to make all the unreachables errors, not warnings,
but there may be a bug or two to clear out first.

class A {
  def badEquals(x: Any, y: Any) = x match {
    case y => true
    case _ => false
  }
}

a.scala:3: warning: patterns after a variable pattern cannot match (SLS 8.1.1)
If you intended to match against parameter y of method badEquals, you must use backticks, like: case `y` =>
    case y => true
         ^
a.scala:4: warning: unreachable code due to variable pattern 'y' on line 3
    case _ => false
              ^
two warnings found
  • Loading branch information
paulp committed Aug 8, 2012
1 parent 7d3b2de commit 802771b
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 23 deletions.
80 changes: 70 additions & 10 deletions src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala
Expand Up @@ -198,6 +198,69 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
import typer.{typed, context, silent, reallyExists}
// import typer.infer.containsUnchecked

// Why is it so difficult to say "here's a name and a context, give me any
// matching symbol in scope" ? I am sure this code is wrong, but attempts to
// use the scopes of the contexts in the enclosing context chain discover
// nothing. How to associate a name with a symbol would would be a wonderful
// linkage for which to establish a canonical acquisition mechanism.
def matchingSymbolInScope(pat: Tree): Symbol = {
def declarationOfName(tpe: Type, name: Name): Symbol = tpe match {
case PolyType(tparams, restpe) => tparams find (_.name == name) getOrElse declarationOfName(restpe, name)
case MethodType(params, restpe) => params find (_.name == name) getOrElse declarationOfName(restpe, name)
case ClassInfoType(_, _, clazz) => clazz.rawInfo member name
case _ => NoSymbol
}
pat match {
case Bind(name, _) =>
context.enclosingContextChain.foldLeft(NoSymbol: Symbol)((res, ctx) =>
res orElse declarationOfName(ctx.owner.rawInfo, name))
case _ => NoSymbol
}
}

// Issue better warnings than "unreachable code" when people mis-use
// variable patterns thinking they bind to existing identifiers.
//
// Possible TODO: more deeply nested variable patterns, like
// case (a, b) => 1 ; case (c, d) => 2
// However this is a pain (at least the way I'm going about it)
// and I have to think these detailed errors are primarily useful
// for beginners, not people writing nested pattern matches.
def checkMatchVariablePatterns(m: Match) {
// A string describing the first variable pattern
var vpat: String = null
// Using an iterator so we can recognize the last case
val it = m.cases.iterator

def addendum(pat: Tree) = {
matchingSymbolInScope(pat) match {
case NoSymbol => ""
case sym =>
val desc = if (sym.isParameter) s"parameter ${sym.nameString} of" else sym + " in"
s"\nIf you intended to match against $desc ${sym.owner}, you must use backticks, like: case `${sym.nameString}` =>"
}
}

while (it.hasNext) {
val cdef = it.next
// If a default case has been seen, then every succeeding case is unreachable.
if (vpat != null)
context.unit./*error*/warning(cdef.body.pos, "unreachable code due to " + vpat + addendum(cdef.pat))
// If this is a default case and more cases follow, warn about this one so
// we have a reason to mention its pattern variable name and any corresponding
// symbol in scope. Errors will follow from the remaining cases, at least
// once we make the above warning an error.
else if (it.hasNext && (treeInfo isDefaultCase cdef)) {
val vpatName = cdef.pat match {
case Bind(name, _) => s" '$name'"
case _ => ""
}
vpat = s"variable pattern$vpatName on line ${cdef.pat.pos.line}"
context.unit.warning(cdef.pos, s"patterns after a variable pattern cannot match (SLS 8.1.1)" + addendum(cdef.pat))
}
}
}

/** Implement a pattern match by turning its cases (including the implicit failure case)
* into the corresponding (monadic) extractors, and combining them with the `orElse` combinator.
*
Expand All @@ -210,6 +273,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
*/
def translateMatch(match_ : Match): Tree = {
val Match(selector, cases) = match_
checkMatchVariablePatterns(match_)

// we don't transform after uncurry
// (that would require more sophistication when generating trees,
Expand Down Expand Up @@ -3325,16 +3389,12 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
case Some(cds) => cds
}

val allReachable =
if (unchecked) true
else {
val unreachables = unreachableCase(caseDefsWithGuards)
unreachables foreach {cd => reportUnreachable(cd.body.pos)}
// a switch with duplicate cases yields a verify error,
// and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch
// (even though the verify error would disappear, the behaviour would change)
unreachables.isEmpty
}
val allReachable = unchecked || {
// a switch with duplicate cases yields a verify error,
// and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch
// (even though the verify error would disappear, the behaviour would change)
unreachableCase(caseDefsWithGuards) map (cd => reportUnreachable(cd.body.pos)) isEmpty
}

if (!allReachable) Nil
else if (noGuards(caseDefsWithGuards)) {
Expand Down
2 changes: 1 addition & 1 deletion src/reflect/scala/reflect/internal/TreeInfo.scala
Expand Up @@ -236,7 +236,7 @@ abstract class TreeInfo {
case _ =>
tree
}

/** Is tree a self or super constructor call? */
def isSelfOrSuperConstrCall(tree: Tree) = {
// stripNamedApply for SI-3584: adaptToImplicitMethod in Typers creates a special context
Expand Down
8 changes: 4 additions & 4 deletions test/files/neg/macro-invalidsig-context-bounds.check
@@ -1,4 +1,4 @@
Impls_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences
def foo[U: c.AbsTypeTag: Numeric](c: Ctx) = {
^
one error found
Impls_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences
def foo[U: c.AbsTypeTag: Numeric](c: Ctx) = {
^
one error found
8 changes: 4 additions & 4 deletions test/files/neg/macro-invalidsig-implicit-params.check
@@ -1,4 +1,4 @@
Impls_Macros_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences
def foo_targs[T, U: c.AbsTypeTag](c: Ctx)(implicit x: c.Expr[Int]) = {
^
one error found
Impls_Macros_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences
def foo_targs[T, U: c.AbsTypeTag](c: Ctx)(implicit x: c.Expr[Int]) = {
^
one error found
27 changes: 27 additions & 0 deletions test/files/neg/newpat_unreachable.check
@@ -0,0 +1,27 @@
newpat_unreachable.scala:6: error: patterns after a variable pattern cannot match (SLS 8.1.1)
If you intended to match against parameter b of method contrivedExample, you must use backticks, like: case `b` =>
case b => println("matched b")
^
newpat_unreachable.scala:7: error: unreachable code due to variable pattern 'b' on line 6
If you intended to match against parameter c of method contrivedExample, you must use backticks, like: case `c` =>
case c => println("matched c")
^
newpat_unreachable.scala:8: error: unreachable code due to variable pattern 'b' on line 6
If you intended to match against value d in class A, you must use backticks, like: case `d` =>
case d => println("matched d")
^
newpat_unreachable.scala:9: error: unreachable code due to variable pattern 'b' on line 6
case _ => println("matched neither")
^
newpat_unreachable.scala:22: error: patterns after a variable pattern cannot match (SLS 8.1.1)
If you intended to match against parameter b of method g, you must use backticks, like: case `b` =>
case b => 1
^
newpat_unreachable.scala:23: error: unreachable code due to variable pattern 'b' on line 22
If you intended to match against parameter c of method h, you must use backticks, like: case `c` =>
case c => 2
^
newpat_unreachable.scala:24: error: unreachable code due to variable pattern 'b' on line 22
case _ => 3
^
7 errors found
1 change: 1 addition & 0 deletions test/files/neg/newpat_unreachable.flags
@@ -0,0 +1 @@
-Xfatal-warnings
29 changes: 29 additions & 0 deletions test/files/neg/newpat_unreachable.scala
@@ -0,0 +1,29 @@
object Test {
class A {
val d = 55

def contrivedExample[A, B, C](a: A, b: B, c: C): Unit = a match {
case b => println("matched b")
case c => println("matched c")
case d => println("matched d")
case _ => println("matched neither")
}

def correctExample[A, B, C](a: A, b: B, c: C): Unit = a match {
case `b` => println("matched b")
case `c` => println("matched c")
case `d` => println("matched d")
case _ => println("matched neither")
}

def f[A](a: A) = {
def g[B](b: B) = {
def h[C](c: C) = a match {
case b => 1
case c => 2
case _ => 3
}
}
}
}
}
8 changes: 7 additions & 1 deletion test/files/neg/pat_unreachable.check
Expand Up @@ -4,4 +4,10 @@ pat_unreachable.scala:5: error: unreachable code
pat_unreachable.scala:9: error: unreachable code
case Seq(x, y) => List(x, y)
^
two errors found
pat_unreachable.scala:23: error: unreachable code
case c => println("matched c")
^
pat_unreachable.scala:24: error: unreachable code
case _ => println("matched neither")
^
four errors found
8 changes: 7 additions & 1 deletion test/files/neg/pat_unreachable.scala
Expand Up @@ -8,7 +8,7 @@ object Test extends App {
case Seq(x, y, _*) => x::y::Nil
case Seq(x, y) => List(x, y)
}

def not_unreachable(xs:Seq[Char]) = xs match {
case Seq(x, y, _*) => x::y::Nil
case Seq(x) => List(x)
Expand All @@ -17,4 +17,10 @@ object Test extends App {
case Seq(x, y) => x::y::Nil
case Seq(x, y, z, _*) => List(x,y)
}

def contrivedExample[A, B, C](a: A, b: B, c: C): Unit = a match {
case b => println("matched b")
case c => println("matched c")
case _ => println("matched neither")
}
}
7 changes: 5 additions & 2 deletions test/files/neg/t6048.check
Expand Up @@ -4,7 +4,10 @@ t6048.scala:3: error: unreachable code
t6048.scala:8: error: unreachable code
case _ if false => x // unreachable
^
t6048.scala:14: error: unreachable code
t6048.scala:13: error: patterns after a variable pattern cannot match (SLS 8.1.1)
case _ => x
^
t6048.scala:14: error: unreachable code due to variable pattern on line 13
case 5 if true => x // unreachable
^
three errors found
four errors found

0 comments on commit 802771b

Please sign in to comment.