Improve syntax error recovery

 - Drop Indented region only after seeing an OUTDENT node. This aligns
   region stack popping with the other closing tokens `]`, `)`, and `}`
   and thereby fixes a problem in `skip`.

 - When skipping, allow OUTDENT insertions even in n on-indent regions
   if the outdent matches a previous indent. This improves recovert when
   closing `]`, `)`, or `}` are missing.

 - When skipping an OUTDENT that matches a previous indent can discard
   nested regions.

Fixes #14507

Author: odersky

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -252,16 +252,9 @@ object Parsers {
 
     /** Skip on error to next safe point.
      */
-    protected def skip(): Unit =
-      val lastRegion = in.currentRegion
-      in.skipping = true
-      def atStop =
-        in.token == EOF
-        || skipStopTokens.contains(in.token) && (in.currentRegion eq lastRegion)
-      while !atStop do
-        in.nextToken()
+    def skip(): Unit =
+      in.skip()
       lastErrorOffset = in.offset
-      in.skipping = false
 
     def warning(msg: Message, sourcePos: SourcePosition): Unit =
       report.warning(msg, sourcePos)

compiler/src/dotty/tools/dotc/parsing/Scanners.scala

@@ -170,8 +170,6 @@ object Scanners {
     /** A switch whether operators at the start of lines can be infix operators */
     private[Scanners] var allowLeadingInfixOperators = true
 
-    var skipping = false
-
     var debugTokenStream = false
     val showLookAheadOnDebug = false
 
@@ -274,7 +272,35 @@ object Scanners {
     private val prev = newTokenData
 
     /** The current region. This is initially an Indented region with zero indentation width. */
-    var currentRegion: Region = Indented(IndentWidth.Zero, Set(), EMPTY, null)
+    var currentRegion: Region = Indented(IndentWidth.Zero, EMPTY, null)
+
+// Error recovery ------------------------------------------------------------
+
+    private def lastKnownIndentWidth: IndentWidth =
+      def recur(r: Region): IndentWidth =
+        if r.knownWidth == null then recur(r.enclosing) else r.knownWidth
+      recur(currentRegion)
+
+    private var skipping = false
+
+    /** Skip on error to next safe point.
+     */
+    def skip(): Unit =
+      val lastRegion = currentRegion
+      skipping = true
+      def atStop =
+        token == EOF
+        || (currentRegion eq lastRegion)
+            && (skipStopTokens.contains(token)
+                || token == OUTDENT && indentWidth(offset) < lastKnownIndentWidth)
+          // stop at OUTDENT if the new indentwidth is smaller than the indent width of
+          // currentRegion. This corrects for the problem that sometimes we don't see an INDENT
+          // when skipping and therefore might erroneously end up syncing on a nested OUTDENT.
+      // println(s"\nSTART SKIP AT ${sourcePos().line + 1}, $this in $currentRegion")
+      while !atStop do
+        nextToken()
+      // println(s"\nSTOP SKIP AT ${sourcePos().line + 1}, $this in $currentRegion")
+      skipping = false
 
 // Get next token ------------------------------------------------------------
 
@@ -305,27 +331,34 @@ object Scanners {
       nextToken()
       result
 
+    private inline def dropUntil(inline matches: Region => Boolean): Unit =
+      while
+        !currentRegion.isOutermost
+        && {
+          val isLast = matches(currentRegion)
+          currentRegion = currentRegion.enclosing
+          !isLast
+        }
+      do ()
+
     def adjustSepRegions(lastToken: Token): Unit = (lastToken: @switch) match {
       case LPAREN | LBRACKET =>
         currentRegion = InParens(lastToken, currentRegion)
       case LBRACE =>
         currentRegion = InBraces(currentRegion)
       case RBRACE =>
-        def dropBraces(): Unit = currentRegion match {
-          case r: InBraces =>
-            currentRegion = r.enclosing
-          case _ =>
-            if (!currentRegion.isOutermost) {
-              currentRegion = currentRegion.enclosing
-              dropBraces()
-            }
-        }
-        dropBraces()
+        dropUntil(_.isInstanceOf[InBraces])
       case RPAREN | RBRACKET =>
         currentRegion match {
           case InParens(prefix, outer) if prefix + 1 == lastToken => currentRegion = outer
           case _ =>
         }
+      case OUTDENT =>
+        currentRegion match
+          case r: Indented => currentRegion = r.enclosing
+          case r =>
+            if skipping && r.enclosing.isClosedByUndentAt(indentWidth(offset)) then
+              dropUntil(_.isInstanceOf[Indented])
       case STRINGLIT =>
         currentRegion match {
           case InString(_, outer) => currentRegion = outer
@@ -413,8 +446,8 @@ object Scanners {
             || {
               r.outer match
                 case null => true
-                case Indented(outerWidth, others, _, _) =>
-                  outerWidth < nextWidth && !others.contains(nextWidth)
+                case ro @ Indented(outerWidth, _, _) =>
+                  outerWidth < nextWidth && !ro.otherIndentWidths.contains(nextWidth)
                 case outer =>
                   outer.indentWidth < nextWidth
             }
@@ -520,6 +553,15 @@ object Scanners {
       var lastWidth = IndentWidth.Zero
       var indentPrefix = EMPTY
       val nextWidth = indentWidth(offset)
+
+      // If nextWidth is an indentation level not yet seen by enclosing indentation
+      // region, invoke `handler`.
+      def handleNewIndentWidth(r: Region, handler: Indented => Unit): Unit = r match
+        case r @ Indented(curWidth, prefix, outer)
+        if curWidth < nextWidth && !r.otherIndentWidths.contains(nextWidth) && nextWidth != lastWidth =>
+          handler(r)
+        case _ =>
+
       currentRegion match
         case r: Indented =>
           indentIsSignificant = indentSyntax
@@ -548,32 +590,30 @@ object Scanners {
           else if !isLeadingInfixOperator(nextWidth) && !statCtdTokens.contains(lastToken) && lastToken != INDENT then
             currentRegion match
               case r: Indented =>
-                currentRegion = r.enclosing
                 insert(OUTDENT, offset)
-              case r: InBraces if !closingRegionTokens.contains(token) =>
+                if next.token != COLON then
+                  handleNewIndentWidth(r.enclosing, ir =>
+                    errorButContinue(
+                      i"""The start of this line does not match any of the previous indentation widths.
+                          |Indentation width of current line : $nextWidth
+                          |This falls between previous widths: ${ir.width} and $lastWidth"""))
+              case r: InBraces if !closingRegionTokens.contains(token) && !skipping =>
                 report.warning("Line is indented too far to the left, or a `}` is missing", sourcePos())
-              case _ =>
+              case r =>
+                if skipping && r.enclosing.isClosedByUndentAt(nextWidth) then
+                  insert(OUTDENT, offset)
 
         else if lastWidth < nextWidth
              || lastWidth == nextWidth && (lastToken == MATCH || lastToken == CATCH) && token == CASE then
           if canStartIndentTokens.contains(lastToken) then
-            currentRegion = Indented(nextWidth, Set(), lastToken, currentRegion)
+            currentRegion = Indented(nextWidth, lastToken, currentRegion)
             insert(INDENT, offset)
           else if lastToken == SELFARROW then
             currentRegion.knownWidth = nextWidth
         else if (lastWidth != nextWidth)
           errorButContinue(spaceTabMismatchMsg(lastWidth, nextWidth))
-      currentRegion match
-        case Indented(curWidth, others, prefix, outer)
-        if curWidth < nextWidth && !others.contains(nextWidth) && nextWidth != lastWidth =>
-          if token == OUTDENT && next.token != COLON then
-            errorButContinue(
-              i"""The start of this line does not match any of the previous indentation widths.
-                  |Indentation width of current line : $nextWidth
-                  |This falls between previous widths: $curWidth and $lastWidth""")
-          else
-            currentRegion = Indented(curWidth, others + nextWidth, prefix, outer)
-        case _ =>
+      if token != OUTDENT || next.token == COLON then
+        handleNewIndentWidth(currentRegion, _.otherIndentWidths += nextWidth)
     end handleNewLine
 
     def spaceTabMismatchMsg(lastWidth: IndentWidth, nextWidth: IndentWidth) =
@@ -593,7 +633,7 @@ object Scanners {
         val nextWidth = indentWidth(next.offset)
         val lastWidth = currentRegion.indentWidth
         if lastWidth < nextWidth then
-          currentRegion = Indented(nextWidth, Set(), COLONEOL, currentRegion)
+          currentRegion = Indented(nextWidth, COLONEOL, currentRegion)
           offset = next.offset
           token = INDENT
     end observeIndented
@@ -608,7 +648,6 @@ object Scanners {
          && !(token == CASE && r.prefix == MATCH)
          && next.token == EMPTY  // can be violated for ill-formed programs, e.g. neg/i12605.sala
       =>
-        currentRegion = r.enclosing
         insert(OUTDENT, offset)
       case _ =>
 
@@ -623,9 +662,7 @@ object Scanners {
     }
 
     def closeIndented() = currentRegion match
-      case r: Indented if !r.isOutermost =>
-        insert(OUTDENT, offset)
-        currentRegion = r.outer
+      case r: Indented if !r.isOutermost => insert(OUTDENT, offset)
       case _ =>
 
     /** - Join CASE + CLASS => CASECLASS, CASE + OBJECT => CASEOBJECT
@@ -656,7 +693,6 @@ object Scanners {
           currentRegion match
             case r: Indented if isEnclosedInParens(r.outer) =>
               insert(OUTDENT, offset)
-              currentRegion = r.outer
             case _ =>
               lookAhead()
               if isAfterLineEnd
@@ -1518,6 +1554,26 @@ object Scanners {
       if enclosing.knownWidth == null then enclosing.useOuterWidth()
       knownWidth = enclosing.knownWidth
 
+    /** Does `width` represent an undent of an enclosing indentation region?
+     *  This is the case if there is an indentation region that goes deeper than `width`
+     *  and that is enclosed in a region that contains `width` as an indentation width.
+     */
+    def isClosedByUndentAt(width: IndentWidth): Boolean = this match
+      case _: Indented =>
+        !isOutermost && width <= indentWidth && enclosing.coversIndent(width)
+      case _ =>
+        enclosing.isClosedByUndentAt(width)
+
+    /** A region "covers" an indentation with `width` if it has `width` as known
+     *  indentation width (either as primary, or in case of an Indent region as
+     *  alternate width).
+     */
+    protected def coversIndent(w: IndentWidth): Boolean =
+      knownWidth != null && w == indentWidth
+
+    def toList: List[Region] =
+      this :: (if outer == null then Nil else outer.toList)
+
     private def delimiter = this match
       case _: InString => "}(in string)"
       case InParens(LPAREN, _) => ")"
@@ -1528,11 +1584,10 @@ object Scanners {
 
     /** Show open regions as list of lines with decreasing indentations */
     def visualize: String =
-      indentWidth.toPrefix
-      + delimiter
-      + outer.match
-          case null => ""
-          case next: Region => "\n" + next.visualize
+      toList.map(r => s"${r.indentWidth.toPrefix}${r.delimiter}").mkString("\n")
+
+    override def toString: String =
+      toList.map(r => s"(${r.indentWidth}, ${r.delimiter})").mkString(" in ")
   end Region
 
   case class InString(multiLine: Boolean, outer: Region) extends Region
@@ -1542,13 +1597,18 @@ object Scanners {
 
   /** A class describing an indentation region.
    *  @param width   The principal indendation width
-   *  @param others  Other indendation widths > width of lines in the same region
    *  @param prefix  The token before the initial <indent> of the region
    */
-  case class Indented(width: IndentWidth, others: Set[IndentWidth], prefix: Token, outer: Region | Null) extends Region:
+  case class Indented(width: IndentWidth, prefix: Token, outer: Region | Null) extends Region:
     knownWidth = width
 
-  def topLevelRegion(width: IndentWidth) = Indented(width, Set(), EMPTY, null)
+    /** Other indendation widths > width of lines in the same region */
+    var otherIndentWidths: Set[IndentWidth] = Set()
+
+    override def coversIndent(w: IndentWidth) = width == w || otherIndentWidths.contains(w)
+  end Indented
+
+  def topLevelRegion(width: IndentWidth) = Indented(width, EMPTY, null)
 
   enum IndentWidth {
     case Run(ch: Char, n: Int)

compiler/src/dotty/tools/dotc/parsing/Tokens.scala

@@ -287,7 +287,7 @@ object Tokens extends TokensCommon {
 
   final val endMarkerTokens = identifierTokens | BitSet(IF, WHILE, FOR, MATCH, TRY, NEW, THROW, GIVEN, VAL, THIS)
 
-  final val skipStopTokens = BitSet(SEMI, COMMA, NEWLINE, NEWLINES, RBRACE, RPAREN, RBRACKET, OUTDENT)
+  final val skipStopTokens = BitSet(SEMI, COMMA, NEWLINE, NEWLINES, RBRACE, RPAREN, RBRACKET)
 
   final val softModifierNames = Set(nme.inline, nme.opaque, nme.open, nme.transparent, nme.infix)
 }

tests/neg/i12605.scala

@@ -1,4 +1,3 @@
 object Foo:
   def joe(): List[(Int, Int)] =
     List((2, 3), (3, 4)).filter case (a, b) => b > a // error // error
-// error

tests/neg/i14507.scala

@@ -0,0 +1,8 @@
+class C:
+  def test = g
+  def f(b: Boolean): Int =
+    if b then // error
+    end if
+    42
+  def g = 27
+end C

tests/neg/indent.scala

@@ -4,6 +4,6 @@ object Test {
   val y3 =
     if (1) max 10 gt 0  // error: end of statement expected but integer literal found // error // error
       1
-    else
+    else // error
       2
-} // error
+}

tests/neg/parser-stability-19.scala

@@ -1,5 +1,5 @@
 object x0 {
   case class x0[]():                  // error
     def x0( ) ]                       // error
-    def x0 ( x0:x0 ):x0.type = x1 x0  // error // error
+    def x0 ( x0:x0 ):x0.type = x1 x0
 // error

tests/neg/syntax-error-recovery.scala

@@ -0,0 +1,19 @@
+object Test {
+  def foo(x: Int) = {
+      if (x < 0 then // error
+        1
+      else
+        2
+  }
+  println(bar) // error
+
+  def foo2(x: Int) =
+      if (x < 0 then // error
+        1
+      else
+        2
+  println(baz) // error
+
+}
+
+

tests/neg/t6810.check

@@ -22,11 +22,3 @@
 30 |  val b = '
    |          ^
    |          unclosed character literal
--- Warning: tests/neg/t6810.scala:6:0 ----------------------------------------------------------------------------------
-6 |'                     // but not embedded EOL sequences not represented as escapes
-  |^
-  |Line is indented too far to the left, or a `}` is missing
--- Warning: tests/neg/t6810.scala:31:0 ---------------------------------------------------------------------------------
-31 |'        // anypos-error CR seen as EOL by scanner; FSR, error only on open quote, unlike `y`
-   |^
-   |Line is indented too far to the left, or a `}` is missing