Use indentation to judge whether a closing } should be considered part of a code block if { is missing

## Motivating example:

```swift
struct Foo {
  func foo()
}
```

We need to decide whether we should use the `}` to close the opening `{` of the `struct` or `func`.

## Solution

Do what a human would do: Use to decide how to match the braces. If the opening `{` of the `func` is missing and the `}` has an indentation that’s less than that of the `func` keyword, assume that it was indented to match an outer and don’t eat it for the `func`.

Author: ahoppen

Sources/SwiftParser/Declarations.swift

@@ -298,7 +298,7 @@ extension Parser {
     } else {
       whereClause = nil
     }
-    let members = self.parseMemberDeclList()
+    let members = self.parseMemberDeclList(introducer: extensionKeyword)
     return RawExtensionDeclSyntax(
       attributes: attrs.attributes,
       modifiers: attrs.modifiers,
@@ -577,8 +577,11 @@ extension Parser {
     )
   }
 
+  /// `introducer` is the `struct`, `class`, ... keyword that is the cause that the member decl block is being parsed.
+  /// If the left brace is missing, its indentation will be used to judge whether a following `}` was
+  /// indented to close this code block or a surrounding context. See `expectRightBrace`.
   @_spi(RawSyntax)
-  public mutating func parseMemberDeclList() -> RawMemberDeclBlockSyntax {
+  public mutating func parseMemberDeclList(introducer: RawTokenSyntax? = nil) -> RawMemberDeclBlockSyntax {
     var elements = [RawMemberDeclListItemSyntax]()
     let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
     do {
@@ -594,7 +597,7 @@ extension Parser {
         elements.append(newElement)
       }
     }
-    let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
+    let (unexpectedBeforeRBrace, rbrace) = self.expectRightBrace(leftBrace: lbrace, introducer: introducer)
     let members: RawMemberDeclListSyntax
     if elements.isEmpty && (lbrace.isMissing || rbrace.isMissing) {
       members = RawMemberDeclListSyntax(elements: [], arena: self.arena)
@@ -676,7 +679,7 @@ extension Parser {
       whereClause = nil
     }
 
-    let members = self.parseMemberDeclList()
+    let members = self.parseMemberDeclList(introducer: classKeyword)
     return RawClassDeclSyntax(
       attributes: attrs.attributes,
       modifiers: attrs.modifiers,
@@ -798,7 +801,7 @@ extension Parser {
       whereClause = nil
     }
 
-    let members = self.parseMemberDeclList()
+    let members = self.parseMemberDeclList(introducer: enumKeyword)
     return RawEnumDeclSyntax(
       attributes: attrs.attributes, modifiers: attrs.modifiers,
       unexpectedBeforeEnumKeyword,
@@ -943,7 +946,7 @@ extension Parser {
       whereClause = nil
     }
 
-    let members = self.parseMemberDeclList()
+    let members = self.parseMemberDeclList(introducer: structKeyword)
     return RawStructDeclSyntax(
       attributes: attrs.attributes, modifiers: attrs.modifiers,
       unexpectedBeforeStructKeyword,
@@ -1055,7 +1058,7 @@ extension Parser {
       whereClause = nil
     }
 
-    let members = self.parseMemberDeclList()
+    let members = self.parseMemberDeclList(introducer: protocolKeyword)
 
     return RawProtocolDeclSyntax(
       attributes: attrs.attributes, modifiers: attrs.modifiers,
@@ -1183,7 +1186,7 @@ extension Parser {
       whereClause = nil
     }
 
-    let members = self.parseMemberDeclList()
+    let members = self.parseMemberDeclList(introducer: actorKeyword)
     return RawActorDeclSyntax(
       attributes: attrs.attributes,
       modifiers: attrs.modifiers,

Sources/SwiftParser/Parser.swift

@@ -424,6 +424,58 @@ extension Parser {
       self.missingToken(.identifier, text: nil)
     )
   }
+
+  /// Expect a right brace but with a little smart recovery logic:
+  /// If `leftBrace` is missing, we only recover to a `}` whose indentation is greater or equal to that of `introducer`.
+  /// That way, if the developer forgot to to type `{`, we won't eat `}` that were most likely intended to close an outer scope.
+  ///
+  /// If `leftBrace` is present or `introducer` is `nil`, this is equivalent to `self.expect(.rightBrace)`.
+  mutating func expectRightBrace(leftBrace: RawTokenSyntax, introducer: RawTokenSyntax?) -> (unexpected: RawUnexpectedNodesSyntax?, token: RawTokenSyntax) {
+    func indentation(_ pieces: [RawTriviaPiece]) -> RawTriviaPiece? {
+      if pieces.last?.isNewline == true {
+        return .spaces(0)
+      }
+      guard pieces.count >= 2 else {
+        return nil
+      }
+      guard pieces[pieces.count - 2].isNewline else {
+        return nil
+      }
+      switch pieces[pieces.count - 1] {
+      case .spaces, .tabs:
+        return pieces[pieces.count - 1]
+      default:
+        return nil
+      }
+    }
+
+    guard leftBrace.isMissing, let introducer = introducer else {
+      // Fast path for correct parses: If leftBrace is not missing, just `expect`.
+      return self.expect(.rightBrace)
+    }
+
+    var lookahead = self.lookahead()
+    guard let recoveryHandle = lookahead.canRecoverTo([.rightBrace]) else {
+      // We can't recover to '}'. Synthesize it.
+      return (nil, self.missingToken(.rightBrace, text: nil))
+    }
+
+    // We can recover to a '}'. Decide whether we want to eat it based on its indentation.
+    let rightBraceTrivia = self.arena.parseTrivia(source: lookahead.currentToken.leadingTriviaText, position: .leading)
+    switch (indentation(introducer.leadingTriviaPieces), indentation(rightBraceTrivia)) {
+    // Catch cases where the brace has known indentation that is less than that of `introducer`, in which case we don't want to consume it.
+    case (.spaces(let introducerSpaces), .spaces(let rightBraceSpaces)) where rightBraceSpaces < introducerSpaces:
+      return (nil, self.missingToken(.rightBrace, text: nil))
+    case (.tabs(let introducerTabs), .tabs(let rightBraceTabs)) where rightBraceTabs < introducerTabs:
+      return (nil, self.missingToken(.rightBrace, text: nil))
+    case (.spaces, .tabs(0)):
+      return (nil, self.missingToken(.rightBrace, text: nil))
+    case (.tabs, .spaces(0)):
+      return (nil, self.missingToken(.rightBrace, text: nil))
+    default:
+      return self.eat(recoveryHandle)
+    }
+  }
 }
 
 // MARK: Splitting Tokens

Sources/SwiftParser/Statements.swift

@@ -132,7 +132,7 @@ extension Parser {
     // A scope encloses the condition and true branch for any variables bound
     // by a conditional binding. The else branch does *not* see these variables.
     let conditions = self.parseConditionList()
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: ifKeyword)
 
     // The else branch, if any, is outside of the scope of the condition.
     let elseKeyword = self.consume(if: .elseKeyword)
@@ -141,7 +141,7 @@ extension Parser {
       if self.at(.ifKeyword) {
         elseBody = RawSyntax(self.parseIfStatement(ifHandle: .constant(.ifKeyword)))
       } else {
-        elseBody = RawSyntax(self.parseCodeBlock())
+        elseBody = RawSyntax(self.parseCodeBlock(introducer: ifKeyword))
       }
     } else {
       elseBody = nil
@@ -169,7 +169,7 @@ extension Parser {
     let (unexpectedBeforeGuardKeyword, guardKeyword) = self.eat(guardHandle)
     let conditions = self.parseConditionList()
     let (unexpectedBeforeElseKeyword, elseKeyword) = self.expect(.elseKeyword)
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: guardKeyword)
     return RawGuardStmtSyntax(
       unexpectedBeforeGuardKeyword,
       guardKeyword: guardKeyword,
@@ -421,7 +421,7 @@ extension Parser {
   @_spi(RawSyntax)
   public mutating func parseDeferStatement(deferHandle: RecoveryConsumptionHandle) -> RawDeferStmtSyntax {
     let (unexpectedBeforeDeferKeyword, deferKeyword) = self.eat(deferHandle)
-    let items = self.parseCodeBlock()
+    let items = self.parseCodeBlock(introducer: deferKeyword)
     return RawDeferStmtSyntax(
       unexpectedBeforeDeferKeyword,
       deferKeyword: deferKeyword,
@@ -443,7 +443,7 @@ extension Parser {
   @_spi(RawSyntax)
   public mutating func parseDoStatement(doHandle: RecoveryConsumptionHandle) -> RawDoStmtSyntax {
     let (unexpectedBeforeDoKeyword, doKeyword) = self.eat(doHandle)
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: doKeyword)
 
     // If the next token is 'catch', this is a 'do'/'catch' statement.
     var elements = [RawCatchClauseSyntax]()
@@ -487,7 +487,7 @@ extension Parser {
           arena: self.arena))
       } while keepGoing != nil && loopProgress.evaluate(currentToken)
     }
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: catchKeyword)
     return RawCatchClauseSyntax(
       unexpectedBeforeCatchKeyword,
       catchKeyword: catchKeyword,
@@ -510,7 +510,7 @@ extension Parser {
   public mutating func parseWhileStatement(whileHandle: RecoveryConsumptionHandle) -> RawWhileStmtSyntax {
     let (unexpectedBeforeWhileKeyword, whileKeyword) = self.eat(whileHandle)
     let conditions = self.parseConditionList()
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: whileKeyword)
     return RawWhileStmtSyntax(
       unexpectedBeforeWhileKeyword,
       whileKeyword: whileKeyword,
@@ -531,7 +531,7 @@ extension Parser {
   @_spi(RawSyntax)
   public mutating func parseRepeatWhileStatement(repeatHandle: RecoveryConsumptionHandle) -> RawRepeatWhileStmtSyntax {
     let (unexpectedBeforeRepeatKeyword, repeatKeyword) = self.eat(repeatHandle)
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: repeatKeyword)
     let (unexpectedBeforeWhileKeyword, whileKeyword) = self.expect(.whileKeyword)
     let condition = self.parseExpression()
     return RawRepeatWhileStmtSyntax(
@@ -601,7 +601,7 @@ extension Parser {
     }
 
     // stmt-brace
-    let body = self.parseCodeBlock()
+    let body = self.parseCodeBlock(introducer: forKeyword)
     return RawForInStmtSyntax(
       unexpectedBeforeForKeyword,
       forKeyword: forKeyword,
@@ -639,7 +639,7 @@ extension Parser {
 
     let cases = self.parseSwitchCases()
 
-    let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
+    let (unexpectedBeforeRBrace, rbrace) = self.expectRightBrace(leftBrace: lbrace, introducer: switchKeyword)
     return RawSwitchStmtSyntax(
       unexpectedBeforeSwitchKeyword,
       switchKeyword: switchKeyword,

Sources/SwiftParser/TopLevel.swift

@@ -81,10 +81,14 @@ extension Parser {
   /// =======
   ///
   ///     code-block → '{' statements? '}'
-  mutating func parseCodeBlock() -> RawCodeBlockSyntax {
+  ///
+  /// `introducer` is the `while`, `if`, ... keyword that is the cause that the code block is being parsed.
+  /// If the left brace is missing, its indentation will be used to judge whether a following `}` was
+  /// indented to close this code block or a surrounding context. See `expectRightBrace`.
+  mutating func parseCodeBlock(introducer: RawTokenSyntax? = nil) -> RawCodeBlockSyntax {
     let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
     let itemList = parseCodeBlockItemList(isAtTopLevel: false, stopCondition: { $0.at(.rightBrace) })
-    let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
+    let (unexpectedBeforeRBrace, rbrace) = self.expectRightBrace(leftBrace: lbrace, introducer: introducer)
 
     return .init(
       unexpectedBeforeLBrace,

Tests/SwiftParserTest/Declarations.swift

@@ -1109,6 +1109,19 @@ final class DeclarationTests: XCTestCase {
       ]
     )
   }
+
+  func testMatchBracesBasedOnSpaces() {
+    AssertParse(
+      """
+      struct Foo {
+        struct Bar 1️⃣
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected member block in struct")
+      ]
+    )
+  }
 }
 
 extension Parser.DeclAttributes {

Tests/SwiftParserTest/translated/ForeachAsyncTests.swift

@@ -64,8 +64,8 @@ final class ForeachAsyncTests: XCTestCase {
         // FIXME: Bad diagnostics; should be just 'expected 'in' after for-each patter'.
         for await i 1️⃣r { 
         }         
-        for await i in r 2️⃣sum = sum + i; 
-      }3️⃣
+        for await i in r 2️⃣sum = sum + i;3️⃣
+      }
       """,
       diagnostics: [
         // TODO: Old parser expected error on line 21: found an unexpected second identifier in constant declaration
@@ -76,7 +76,7 @@ final class ForeachAsyncTests: XCTestCase {
         DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'in' in 'for' statement"),
         // TODO: Old parser expected error on line 23: expected '{' to start the body of for-each loop
         DiagnosticSpec(locationMarker: "2️⃣", message: "expected '{' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end function"),
+        DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end 'for' statement"),
       ]
     )
   }

Tests/SwiftParserTest/translated/ForeachTests.swift

@@ -41,8 +41,8 @@ final class ForeachTests: XCTestCase {
         // FIXME: Bad diagnostics; should be just 'expected 'in' after for-each patter'.
         for i 1️⃣r { 
         }         
-        for i in r 2️⃣sum = sum + i; 
-      }3️⃣
+        for i in r 2️⃣sum = sum + i;3️⃣
+      }
       """,
       diagnostics: [
         // TODO: Old parser expected error on line 21: found an unexpected second identifier in constant declaration
@@ -53,7 +53,7 @@ final class ForeachTests: XCTestCase {
         DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'in' in 'for' statement"),
         // TODO: Old parser expected error on line 23: expected '{' to start the body of for-each loop
         DiagnosticSpec(locationMarker: "2️⃣", message: "expected '{' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end function"),
+        DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end 'for' statement"),
       ]
     )
   }

Tests/SwiftParserTest/translated/GuardTests.swift

@@ -17,10 +17,9 @@ final class GuardTests: XCTestCase {
         // TODO: Old parser expected error on line 2: missing condition in 'guard' statement
         // TODO: Old parser expected error on line 2: expected 'else' after 'guard' condition
         DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement"),
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in 'guard' statement"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected code block in 'guard' statement"),
         // TODO: Old parser expected error on line 5: missing condition in 'guard' statement
         DiagnosticSpec(locationMarker: "2️⃣", message: "expected expression in 'guard' statement"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end function"),
       ]
     )
   }

Tests/SwiftParserTest/translated/SwitchTests.swift

@@ -23,8 +23,6 @@ final class SwitchTests: XCTestCase {
       }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 2: expected expression in 'switch' statement
-        // TODO: Old parser expected error on line 2: expected identifier in function declaration
         DiagnosticSpec(locationMarker: "1️⃣", message: "expected expression and '{}' to end 'switch' statement"),
         DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in function"),
         DiagnosticSpec(locationMarker: "2️⃣", message: "expected argument list in function declaration")
@@ -37,12 +35,10 @@ final class SwitchTests: XCTestCase {
       """
       func parseError2(x: Int) {
         switch x 1️⃣
-      }2️⃣
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 2: expected '{' after 'switch' subject expression
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in 'switch' statement"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected '}' to end function"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{}' in 'switch' statement"),
       ]
     )
   }