SI-5702 Pattern parser halts on star

In patterns, the parser halts when it sees stars.
This means it does not handle infix notation for a case class
named "*".  This patch uses lookahead to decide whether to parse
'_' '*' as a sequence pattern or as the start of infix.
(For both normal and error cases, the tokens are always consumed
immediately.) Error messages are improved for _* (as a help to learners)
and slightly improved recovery helps the parse continue.
The entry point for XML patterns is now distinct; otherwise,
the change is local to pattern3-simplepattern; the entry point
for simplepattern() is unchanged because it is commented
"hook for IDE."

18 files changed, 276 insertions, 58 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala b/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
index 93fa9a60f6..f702f44338 100755
--- a/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
@@ -397,7 +397,7 @@ trait MarkupParsers {
 
     /** xScalaPatterns  ::= patterns
      */
-    def xScalaPatterns: List[Tree] = escapeToScala(parser.seqPatterns(), "pattern")
+    def xScalaPatterns: List[Tree] = escapeToScala(parser.xmlSeqPatterns(), "pattern")
 
     def reportSyntaxError(pos: Int, str: String) = parser.syntaxError(pos, str)
     def reportSyntaxError(str: String) {
diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
index bca1cc4596..337ca7671c 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
@@ -1705,11 +1705,11 @@ self =>
      *  was threaded through methods as boolean seqOK.
      */
     trait SeqContextSensitive extends PatternContextSensitive {
-      /** Returns Some(tree) if it finds a star and prematurely ends parsing.
-       *  This is an artifact of old implementation which has proven difficult
-       *  to cleanly extract.
-       */
-      def interceptStarPattern(top: Tree): Option[Tree]
+      // is a sequence pattern _* allowed?
+      def isSequenceOK: Boolean
+
+      // are we in an XML pattern?
+      def isXML: Boolean = false
 
       def functionArgType(): Tree = argType()
       def argType(): Tree = {
@@ -1796,45 +1796,98 @@ self =>
       /** {{{
        *  Pattern3    ::= SimplePattern
        *                |  SimplePattern {Id [nl] SimplePattern}
-       *  SeqPattern3 ::= SeqSimplePattern [ `*' | `?' | `+' ]
-       *                |  SeqSimplePattern {Id [nl] SeqSimplePattern}
        *  }}}
        */
       def pattern3(): Tree = {
+        var top = simplePattern(badPattern3)
+        // after peekahead
+        def acceptWildStar() = atPos(top.pos.startOrPoint, in.prev.offset)(Star(stripParens(top)))
+        def peekahead() = {
+          in.prev copyFrom in
+          in.nextToken()
+        }
+        def pushback() = {
+          in.next copyFrom in
+          in copyFrom in.prev
+        }
+        // See SI-3189, SI-4832 for motivation. Cf SI-3480 for counter-motivation.
+        // TODO: dredge out the remnants of regexp patterns.
+        // /{/ peek for _*) or _*} (for xml escape)
+        if (isSequenceOK) {
+          top match {
+            case Ident(nme.WILDCARD) if (isRawStar) =>
+              peekahead()
+              in.token match {
+                case RBRACE if (isXML) => return acceptWildStar()
+                case RPAREN if (!isXML) => return acceptWildStar()
+                case _ => pushback()
+              }
+            case _ =>
+          }
+        }
         val base = opstack
-        var top = simplePattern()
-        interceptStarPattern(top) foreach { x => return x }
-
         while (isIdent && in.name != raw.BAR) {
-          top = reduceStack(
-            false, base, top, precedence(in.name), treeInfo.isLeftAssoc(in.name))
+          top = reduceStack(false, base, top, precedence(in.name), treeInfo.isLeftAssoc(in.name))
           val op = in.name
           opstack = OpInfo(top, op, in.offset) :: opstack
           ident()
-          top = simplePattern()
+          top = simplePattern(badPattern3)
         }
         stripParens(reduceStack(false, base, top, 0, true))
       }
+      def badPattern3(): Tree = {
+        def isComma = in.token == COMMA
+        def isAnyBrace = in.token == RPAREN || in.token == RBRACE
+        val badStart = "illegal start of simple pattern"
+        // better recovery if don't skip delims of patterns
+        var skip = !(isComma || isAnyBrace)
+        val msg = if (!opstack.isEmpty && opstack.head.operator == nme.STAR) {
+            opstack.head.operand match {
+              case Ident(nme.WILDCARD) =>
+                if (isSequenceOK && isComma)
+                  "bad use of _* (a sequence pattern must be the last pattern)"
+                else if (isSequenceOK && isAnyBrace) {
+                  skip = true  // do skip bad paren; scanner may skip bad brace already
+                  "bad brace or paren after _*"
+                } else if (!isSequenceOK && isAnyBrace)
+                  "bad use of _* (sequence pattern not allowed)"
+                else badStart
+              case _ =>
+                if (isSequenceOK && isAnyBrace)
+                  "use _* to match a sequence"
+                else if (isComma || isAnyBrace)
+                  "trailing * is not a valid pattern"
+                else badStart
+            }
+          } else {
+            badStart
+          }
+        syntaxErrorOrIncomplete(msg, skip)
+        errorPatternTree
+      }
 
       /** {{{
        *  SimplePattern    ::= varid
        *                    |  `_'
        *                    |  literal
        *                    |  XmlPattern
-       *                    |  StableId  [TypeArgs] [`(' [SeqPatterns] `)']
+       *                    |  StableId  /[TypeArgs]/ [`(' [Patterns] `)']
+       *                    |  StableId  [`(' [Patterns] `)']
+       *                    |  StableId  [`(' [Patterns] `,' [varid `@'] `_' `*' `)']
        *                    |  `(' [Patterns] `)'
-       *  SimpleSeqPattern ::= varid
-       *                    |  `_'
-       *                    |  literal
-       *                    |  XmlPattern
-       *                    |  `<' xLiteralPattern
-       *                    |  StableId [TypeArgs] [`(' [SeqPatterns] `)']
-       *                    |  `(' [SeqPatterns] `)'
        *  }}}
        *
        * XXX: Hook for IDE
        */
       def simplePattern(): Tree = {
+        // simple diagnostics for this entry point
+        def badStart(): Tree = {
+          syntaxErrorOrIncomplete("illegal start of simple pattern", true)
+          errorPatternTree
+        }
+        simplePattern(badStart)
+      }
+      def simplePattern(onError: () => Tree): Tree = {
         val start = in.offset
         in.token match {
           case IDENTIFIER | BACKQUOTED_IDENT | THIS =>
@@ -1867,8 +1920,7 @@ self =>
           case XMLSTART =>
             xmlLiteralPattern()
           case _ =>
-            syntaxErrorOrIncomplete("illegal start of simple pattern", true)
-            errorPatternTree
+            onError()
         }
       }
     }
@@ -1879,16 +1931,16 @@ self =>
     }
     /** The implementation for parsing inside of patterns at points where sequences are allowed. */
     object seqOK extends SeqContextSensitive {
-      // See ticket #3189 for the motivation for the null check.
-      // TODO: dredge out the remnants of regexp patterns.
-      // ... and now this is back the way it was because it caused #3480.
-      def interceptStarPattern(top: Tree): Option[Tree] =
-        if (isRawStar) Some(atPos(top.pos.startOrPoint, in.skipToken())(Star(stripParens(top))))
-        else None
+      val isSequenceOK = true
     }
     /** The implementation for parsing inside of patterns at points where sequences are disallowed. */
     object noSeq extends SeqContextSensitive {
-      def interceptStarPattern(top: Tree) = None
+      val isSequenceOK = false
+    }
+    /** For use from xml pattern, where sequence is allowed and encouraged. */
+    object xmlSeqOK extends SeqContextSensitive {
+      val isSequenceOK = true
+      override val isXML = true
     }
     /** These are default entry points into the pattern context sensitive methods:
      *  they are all initiated from non-pattern context.
@@ -1902,7 +1954,8 @@ self =>
     /** Default entry points into some pattern contexts. */
     def pattern(): Tree = noSeq.pattern()
     def patterns(): List[Tree] = noSeq.patterns()
-    def seqPatterns(): List[Tree] = seqOK.patterns() // Also called from xml parser
+    def seqPatterns(): List[Tree] = seqOK.patterns()
+    def xmlSeqPatterns(): List[Tree] = xmlSeqOK.patterns() // Called from xml parser
     def argumentPatterns(): List[Tree] = inParens {
       if (in.token == RPAREN) Nil
       else seqPatterns()
diff --git a/test/files/neg/t1878-typer.check b/test/files/neg/t1878-typer.check
new file mode 100644
index 0000000000..e3a20d0be7
--- /dev/null
+++ b/test/files/neg/t1878-typer.check
@@ -0,0 +1,4 @@
+t1878-typer.scala:4: error: _* may only come last
+    case <p> { _* } </p> =>
+                ^
+one error found
diff --git a/test/files/neg/t1878-typer.scala b/test/files/neg/t1878-typer.scala
new file mode 100644
index 0000000000..1eb0cb7dff
--- /dev/null
+++ b/test/files/neg/t1878-typer.scala
@@ -0,0 +1,6 @@
+object Test extends App {
+  // illegal - bug #1764
+  null match {
+    case <p> { _* } </p> =>
+  }
+}
diff --git a/test/files/neg/t1878.check b/test/files/neg/t1878.check
index b47367e12c..ac2071c3d8 100644
--- a/test/files/neg/t1878.check
+++ b/test/files/neg/t1878.check
@@ -1,21 +1,7 @@
-t1878.scala:3: error: _* may only come last
+t1878.scala:3: error: bad use of _* (a sequence pattern must be the last pattern)
   val err1 = "" match { case Seq(f @ _*, ',') => f }
-                                 ^
-t1878.scala:3: error: scrutinee is incompatible with pattern type;
- found   : Seq[A]
- required: String
-  val err1 = "" match { case Seq(f @ _*, ',') => f }
-                                ^
-t1878.scala:3: error: not found: value f
-  val err1 = "" match { case Seq(f @ _*, ',') => f }
-                                                 ^
-t1878.scala:3: error: value _2 is not a member of object Seq
-  val err1 = "" match { case Seq(f @ _*, ',') => f }
-             ^
-t1878.scala:9: error: _* may only come last
+                                       ^
+t1878.scala:9: error: bad use of _* (a sequence pattern must be the last pattern)
   val List(List(_*, arg2), _) = List(List(1,2,3), List(4,5,6))
-                 ^
-t1878.scala:13: error: _* may only come last
-    case <p> { _* } </p> =>
-                ^
-6 errors found
+                  ^
+two errors found
diff --git a/test/files/neg/t1878.scala b/test/files/neg/t1878.scala
index 5278fbb7bd..99fee48a96 100644
--- a/test/files/neg/t1878.scala
+++ b/test/files/neg/t1878.scala
@@ -8,8 +8,10 @@ object Test extends App {
   // illegal
   val List(List(_*, arg2), _) = List(List(1,2,3), List(4,5,6))
   
+  /* see t1878-typer.scala
   // illegal - bug #1764
   null match {
     case <p> { _* } </p> =>
   }
+  */
 }
diff --git a/test/files/neg/t3189.check b/test/files/neg/t3189.check
new file mode 100644
index 0000000000..3913c526a2
--- /dev/null
+++ b/test/files/neg/t3189.check
@@ -0,0 +1,4 @@
+t3189.scala:2: error: use _* to match a sequence
+  val Array(a,b*) = ("": Any)
+                ^
+one error found
diff --git a/test/pending/neg/t3189.scala b/test/files/neg/t3189.scala
index 4ea4bb7581..4ea4bb7581 100644
--- a/test/pending/neg/t3189.scala
+++ b/test/files/neg/t3189.scala
diff --git a/test/files/neg/t5702-neg-bad-and-wild.check b/test/files/neg/t5702-neg-bad-and-wild.check
new file mode 100644
index 0000000000..eae81ad5f2
--- /dev/null
+++ b/test/files/neg/t5702-neg-bad-and-wild.check
@@ -0,0 +1,28 @@
+t5702-neg-bad-and-wild.scala:10: error: bad use of _* (a sequence pattern must be the last pattern)
+      case List(1, _*,) => // bad use of _* (a sequence pattern must be the last pattern)
+                     ^
+t5702-neg-bad-and-wild.scala:10: error: illegal start of simple pattern
+      case List(1, _*,) => // bad use of _* (a sequence pattern must be the last pattern)
+                      ^
+t5702-neg-bad-and-wild.scala:12: error: illegal start of simple pattern
+      case List(1, _*3,) => // illegal start of simple pattern
+                       ^
+t5702-neg-bad-and-wild.scala:14: error: use _* to match a sequence
+      case List(1, x*) => // use _* to match a sequence
+                     ^
+t5702-neg-bad-and-wild.scala:15: error: trailing * is not a valid pattern
+      case List(x*, 1) => // trailing * is not a valid pattern
+                  ^
+t5702-neg-bad-and-wild.scala:16: error: trailing * is not a valid pattern
+      case (1, x*) => // trailing * is not a valid pattern
+                 ^
+t5702-neg-bad-and-wild.scala:17: error: bad use of _* (sequence pattern not allowed)
+      case (1, x@_*) => // bad use of _* (sequence pattern not allowed)
+                   ^
+t5702-neg-bad-and-wild.scala:23: error: bad use of _* (a sequence pattern must be the last pattern)
+    val K(ns @ _*, x) = k // bad use of _* (a sequence pattern must be the last pattern)
+                 ^
+t5702-neg-bad-and-wild.scala:24: error: bad use of _* (sequence pattern not allowed)
+    val (b, _ * ) = Pair(5,6) // bad use of _* (sequence pattern not allowed)
+                ^
+9 errors found
diff --git a/test/files/neg/t5702-neg-bad-and-wild.scala b/test/files/neg/t5702-neg-bad-and-wild.scala
new file mode 100644
index 0000000000..3833a002b1
--- /dev/null
+++ b/test/files/neg/t5702-neg-bad-and-wild.scala
@@ -0,0 +1,29 @@
+
+object Test {
+  case class K(i: Int)
+
+  def main(args: Array[String]) {
+    val k = new K(9)
+    val is = List(1,2,3)
+
+    is match {
+      case List(1, _*,) => // bad use of _* (a sequence pattern must be the last pattern)
+                           // illegal start of simple pattern
+      case List(1, _*3,) => // illegal start of simple pattern
+      //case List(1, _*3:) =>  // poor recovery by parens
+      case List(1, x*) => // use _* to match a sequence
+      case List(x*, 1) => // trailing * is not a valid pattern
+      case (1, x*) => // trailing * is not a valid pattern
+      case (1, x@_*) => // bad use of _* (sequence pattern not allowed)
+    }
+
+// good syntax, bad semantics, detected by typer
+//gowild.scala:14: error: star patterns must correspond with varargs parameters
+    val K(is @ _*) = k
+    val K(ns @ _*, x) = k // bad use of _* (a sequence pattern must be the last pattern)
+    val (b, _ * ) = Pair(5,6) // bad use of _* (sequence pattern not allowed)
+// no longer complains
+//bad-and-wild.scala:15: error: ')' expected but '}' found.
+  }
+}
+
diff --git a/test/files/neg/t5702-neg-bad-brace.check b/test/files/neg/t5702-neg-bad-brace.check
new file mode 100644
index 0000000000..503f7d95ed
--- /dev/null
+++ b/test/files/neg/t5702-neg-bad-brace.check
@@ -0,0 +1,10 @@
+t5702-neg-bad-brace.scala:14: error: Unmatched closing brace '}' ignored here
+      case List(1, _*} =>
+                     ^
+t5702-neg-bad-brace.scala:14: error: illegal start of simple pattern
+      case List(1, _*} =>
+                       ^
+t5702-neg-bad-brace.scala:15: error: ')' expected but '}' found.
+    }
+    ^
+three errors found
diff --git a/test/files/neg/t5702-neg-bad-brace.scala b/test/files/neg/t5702-neg-bad-brace.scala
new file mode 100644
index 0000000000..16a341cf8c
--- /dev/null
+++ b/test/files/neg/t5702-neg-bad-brace.scala
@@ -0,0 +1,17 @@
+
+object Test {
+
+  def main(args: Array[String]) {
+    val is = List(1,2,3)
+
+    is match {
+// the erroneous brace is ignored, so we can't halt on it.
+// maybe brace healing can detect overlapping unmatched (...}
+// In this case, the fix emits an extra error:
+// t5702-neg-bad-brace.scala:10: error: Unmatched closing brace '}' ignored here
+// t5702-neg-bad-brace.scala:10: error: illegal start of simple pattern (i.e., =>)
+// t5702-neg-bad-brace.scala:11: error: ')' expected but '}' found.
+      case List(1, _*} =>
+    }
+  }
+}
diff --git a/test/files/neg/t5702-neg-bad-xbrace.check b/test/files/neg/t5702-neg-bad-xbrace.check
new file mode 100644
index 0000000000..d88638aee9
--- /dev/null
+++ b/test/files/neg/t5702-neg-bad-xbrace.check
@@ -0,0 +1,7 @@
+t5702-neg-bad-xbrace.scala:19: error: bad brace or paren after _*
+          case <year>{_*)}</year> => y
+                        ^
+t5702-neg-bad-xbrace.scala:28: error: bad brace or paren after _*
+    val <top>{a, z@_*)}</top> = xml
+                     ^
+two errors found
diff --git a/test/files/neg/t5702-neg-bad-xbrace.scala b/test/files/neg/t5702-neg-bad-xbrace.scala
new file mode 100644
index 0000000000..64bbdb18be
--- /dev/null
+++ b/test/files/neg/t5702-neg-bad-xbrace.scala
@@ -0,0 +1,31 @@
+
+object Test {
+  def main(args: Array[String]) {
+    /* PiS example, minus a brace
+    val yearMade = 1965
+    val old =
+      <a>{ if (yearMade < 2000) <old>yearMade}</old>
+           else xml.NodeSeq.Empty } </a>
+    println(old)
+    */
+
+    // bad brace or paren after _*
+    // actually, we know it's a bad paren...
+    // we skip it because not in a context looking for rparen
+    val xyear = <year>1965</year>
+    val ancient =
+      <b>{
+        val when = xyear match {
+          case <year>{_*)}</year> => y
+          case _ => "2035"
+        }
+        <old>{when}</old>
+      }</b>
+    println(ancient)
+
+    val xml = <top><a>apple</a><b>boy</b><c>child</c></top>
+    // bad brace or paren after _*
+    val <top>{a, z@_*)}</top> = xml
+    println("A for "+ a +", ending with "+ z)
+  }
+}
diff --git a/test/files/neg/t5702-neg-ugly-xbrace.check b/test/files/neg/t5702-neg-ugly-xbrace.check
new file mode 100644
index 0000000000..7d80bbf6be
--- /dev/null
+++ b/test/files/neg/t5702-neg-ugly-xbrace.check
@@ -0,0 +1,19 @@
+t5702-neg-ugly-xbrace.scala:11: error: bad brace or paren after _*
+    val <top>{a, z@_*)</top> = xml
+                     ^
+t5702-neg-ugly-xbrace.scala:12: error: Missing closing brace `}' assumed here
+    println("A for "+ a +", ending with "+ z)
+    ^
+t5702-neg-ugly-xbrace.scala:13: error: in XML literal: in XML content, please use '}}' to express '}'
+  }
+  ^
+t5702-neg-ugly-xbrace.scala:11: error:  I encountered a '}' where I didn't expect one, maybe this tag isn't closed <top>
+    val <top>{a, z@_*)</top> = xml
+         ^
+t5702-neg-ugly-xbrace.scala:14: error: illegal start of simple pattern
+}
+^
+t5702-neg-ugly-xbrace.scala:14: error: '}' expected but eof found.
+}
+ ^
+6 errors found
diff --git a/test/files/neg/t5702-neg-ugly-xbrace.scala b/test/files/neg/t5702-neg-ugly-xbrace.scala
new file mode 100644
index 0000000000..0ff7bfa09d
--- /dev/null
+++ b/test/files/neg/t5702-neg-ugly-xbrace.scala
@@ -0,0 +1,14 @@
+
+object Test {
+  def main(args: Array[String]) {
+
+    val xml = <top><a>apple</a><b>boy</b><c>child</c></top>
+    // This is the more likely typo, and the uglier parse.
+    // We could turn it into a } if } does not follow (to
+    // avoid handing }} back to xml) but that is quite ad hoc.
+    // Assuming } for ) after _* would not be not outlandish.
+    // bad brace or paren after _*
+    val <top>{a, z@_*)</top> = xml
+    println("A for "+ a +", ending with "+ z)
+  }
+}
diff --git a/test/files/pos/t5702-pos-infix-star.scala b/test/files/pos/t5702-pos-infix-star.scala
new file mode 100644
index 0000000000..756bcdd8de
--- /dev/null
+++ b/test/files/pos/t5702-pos-infix-star.scala
@@ -0,0 +1,15 @@
+
+object Test {
+  case class *(a: Int, b: Int)
+  type Star = *
+  case class P(a: Int, b: Star) // alias still required
+
+  def main(args: Array[String]) {
+    val v = new *(6,7)
+    val x * y = v
+    printf("%d,%d\n",x,y)
+    val p = P(5, v)
+    val P(a, b * c) = p
+    printf("%d,%d,%d\n",a,b,c)
+  }
+}
diff --git a/test/pending/neg/t3189.check b/test/pending/neg/t3189.check
deleted file mode 100644
index 43dd0f29a0..0000000000
--- a/test/pending/neg/t3189.check
+++ /dev/null
@@ -1,7 +0,0 @@
-t3189.scala:2: error: illegal start of simple pattern
-  val Array(a,b*) = ("": Any)
-                ^
-t3189.scala:3: error: ')' expected but '}' found.
-}
-^
-two errors found