Merge pull request #940 from ahoppen/ahoppen/recovery-in-precedencegroup

Improve recovery in precedence groups

Author: ahoppen

CodeGeneration/Sources/SyntaxSupport/gyb_generated/DeclNodes.swift

@@ -1258,7 +1258,7 @@ public let DECL_NODES: [Node] = [
        ]),
 
   Node(name: "OperatorDecl",
-       nameForDiagnostics: "operator",
+       nameForDiagnostics: "operator declaration",
        description: "A Swift `operator` declaration.",
        kind: "Decl",
        traits: [

Sources/SwiftParser/Declarations.swift

@@ -2153,12 +2153,19 @@ extension Parser {
           let assignmentKeyword = self.eat(handle)
           let (unexpectedBeforeColon, colon) = self.expect(.colon)
           let (unexpectedBeforeFlag, flag) = self.expectAny([.trueKeyword, .falseKeyword], default: .trueKeyword)
+          let unexpectedAfterFlag: RawUnexpectedNodesSyntax?
+          if flag.isMissing, let unexpectedIdentifier = self.consume(if: .identifier, where: { !$0.isAtStartOfLine }) {
+            unexpectedAfterFlag = RawUnexpectedNodesSyntax([unexpectedIdentifier], arena: self.arena)
+          } else {
+            unexpectedAfterFlag = nil
+          }
           elements.append(RawSyntax(RawPrecedenceGroupAssignmentSyntax(
             assignmentKeyword: assignmentKeyword,
             unexpectedBeforeColon,
             colon: colon,
             unexpectedBeforeFlag,
             flag: flag,
+            unexpectedAfterFlag,
             arena: self.arena
           )))
         case (.higherThan, let handle)?,

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -436,6 +436,16 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    if let unexpected = node.unexpectedBetweenColonAndFlag ?? node.unexpectedAfterFlag, node.flag.presence == .missing {
+      addDiagnostic(unexpected, .invalidFlagAfterPrecedenceGroupAssignment, handledNodes: [unexpected.id, node.flag.id])
+    }
+    return .visitChildren
+  }
+
   public override func visit(_ node: ReturnStmtSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -75,6 +75,7 @@ public enum StaticParserError: String, DiagnosticMessage {
   case defaultOutsideOfSwitch = "'default' label can only appear inside a 'switch' statement"
   case editorPlaceholderInSourceFile = "editor placeholder in source file"
   case expectedExpressionAfterTry = "expected expression after 'try'"
+  case invalidFlagAfterPrecedenceGroupAssignment = "expected 'true' or 'false' after 'assignment'"
   case missingColonInTernaryExprDiagnostic = "expected ':' after '? ...' in ternary expression"
   case operatorShouldBeDeclaredWithoutBody = "operator should no longer be declared with body"
   case standaloneSemicolonStatement = "standalone ';' statements are not allowed"

Sources/SwiftSyntax/gyb_generated/SyntaxEnum.swift

@@ -598,7 +598,7 @@ public enum SyntaxEnum {
     case .enumDecl:
       return "enum"
     case .operatorDecl:
-      return "operator"
+      return "operator declaration"
     case .designatedTypeList:
       return nil
     case .designatedTypeElement:

Sources/SwiftSyntax/gyb_generated/syntax_nodes/SyntaxNodes.swift

@@ -10172,7 +10172,7 @@ public struct OperatorPrecedenceAndTypesSyntax: SyntaxProtocol, SyntaxHashable {
     case 2:
       return nil
     case 3:
-      return nil
+      return "precedence group"
     case 4:
       return nil
     case 5:

Tests/SwiftParserTest/translated/OperatorDeclTests.swift

@@ -142,7 +142,7 @@ final class OperatorDeclTests: XCTestCase {
       prefix operator1️⃣
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected name in operator")
+        DiagnosticSpec(message: "expected name in operator declaration")
       ]
     )
   }
@@ -154,7 +154,7 @@ final class OperatorDeclTests: XCTestCase {
       prefix operator %%+
       """,
       diagnostics: [
-        DiagnosticSpec(message: "unexpected code before operator"),
+        DiagnosticSpec(message: "unexpected code before operator declaration"),
       ]
     )
   }
@@ -208,7 +208,6 @@ final class OperatorDeclTests: XCTestCase {
       infix operator --1️⃣aa
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: 'aa' is considered an identifier and must not appear within an operator name
         DiagnosticSpec(message: "'aa' is considered an identifier and must not appear within an operator name", fixIts: ["remove 'aa'"])
       ], fixedSource: "infix operator --"
     )
@@ -220,7 +219,6 @@ final class OperatorDeclTests: XCTestCase {
       infix operator 1️⃣aa--: A
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: 'aa' is considered an identifier and must not appear within an operator name
         DiagnosticSpec(message: "'aa' is considered an identifier and must not appear within an operator name", highlight: "aa", fixIts: ["remove 'aa'"]),
       ], fixedSource: "infix operator --: A"
     )
@@ -302,51 +300,91 @@ final class OperatorDeclTests: XCTestCase {
       infix operator --- : 1️⃣;
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 3: expected precedence group name after ':' in operator declaration
-        DiagnosticSpec(message: "expected identifier in operator"),
+        DiagnosticSpec(message: "expected precedence group in operator declaration"),
       ]
     )
   }
 
-  func testOperatorDecl15() {
+  func testOperatorDecl15a() {
     AssertParse(
       """
       precedencegroup 1️⃣{ 
         associativity: right
       }
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected name in precedencegroup"),
+      ]
+    )
+  }
+
+  func testOperatorDecl15b() {
+    AssertParse(
+      """
       precedencegroup A {
-        associativity 2️⃣right 
+        associativity 1️⃣right
       }
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected ':' in 'associativity' property of precedencegroup"),
+      ]
+    )
+  }
+
+
+  func testOperatorDecl15c() {
+    AssertParse(
+      """
       precedencegroup B {
-        3️⃣precedence 123 
+        1️⃣precedence 123
       }
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "unexpected code 'precedence 123' in precedencegroup"),
+      ]
+    )
+  }
+
+
+  func testOperatorDecl15d() {
+    AssertParse(
+      """
       precedencegroup C {
-        associativity: sinister 
+        associativity: sinister
       }
+      """
+    )
+  }
+
+
+  func testOperatorDecl15e() {
+    AssertParse(
+      """
       precedencegroup D {
-        assignment: 4️⃣no 
+        assignment: 1️⃣no
       }
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected 'true' or 'false' after 'assignment'"),
+      ]
+    )
+  }
+
+
+  func testOperatorDecl15() {
+    AssertParse(
+      """
       precedencegroup E {
-        higherThan:5️⃣
+        higherThan:1️⃣
       }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected identifier after 'precedencegroup'
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected name in precedencegroup"),
-        // TODO: Old parser expected error on line 5: expected colon after attribute name in precedence group
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' in 'associativity' property of precedencegroup"),
-        // TODO: Old parser expected error on line 8: 'precedence' is not a valid precedence group attribute
-        DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'precedence 123' in precedencegroup"),
-        // TODO: Old parser expected error on line 11: expected 'none', 'left', or 'right' after 'associativity'
-        // TODO: Old parser expected error on line 14: expected 'true' or 'false' after 'assignment'
-        DiagnosticSpec(locationMarker: "4️⃣", message: "expected 'true' in 'assignment' property of precedencegroup"),
-        DiagnosticSpec(locationMarker: "4️⃣", message: "unexpected code 'no' in precedencegroup"),
-        DiagnosticSpec(locationMarker: "5️⃣", message: "expected name in 'relation' property of precedencegroup"),
-        // TODO: Old parser expected error on line 18: expected name of related precedence group after 'higherThan'
+        DiagnosticSpec(message: "expected name in 'relation' property of precedencegroup"),
       ]
     )
   }
 
+
   func testOperatorDecl16() {
     AssertParse(
       """
@@ -364,10 +402,7 @@ final class OperatorDeclTests: XCTestCase {
         associativity: none
         associativity: left 
       }
-      """,
-      diagnostics: [
-        // TODO: Old parser expected error on line 3: 'associativity' attribute for precedence group declared multiple times
-      ]
+      """
     )
   }
 
@@ -378,10 +413,7 @@ final class OperatorDeclTests: XCTestCase {
         assignment: true 
         assignment: false 
       }
-      """,
-      diagnostics: [
-        // TODO: Old parser expected error on line 3: 'assignment' attribute for precedence group declared multiple times
-      ]
+      """
     )
   }
 
@@ -391,34 +423,25 @@ final class OperatorDeclTests: XCTestCase {
       class Foo {
         infix operator ||| 
       }
-      """,
-      diagnostics: [
-        // TODO: Old parser expected error on line 2: 'operator' may only be declared at file scope
-      ]
+      """
     )
   }
 
   func testOperatorDecl20() {
     AssertParse(
       """
       infix operator **<< : UndeclaredPrecedenceGroup
-      """,
-      diagnostics: [
-        // TODO: Old parser expected error on line 1: unknown precedence group 'UndeclaredPrecedenceGroup'
-      ]
+      """
     )
   }
 
   func testOperatorDecl21() {
+    // TODO: We should not allow specification of multiple precedence groups
     AssertParse(
       """
       protocol Proto {}
       infix operator *<*< : F, Proto
-      """,
-      diagnostics: [
-        // TODO: Old parser expected error on line 2: consecutive statements on a line must be separated by ';'
-        // TODO: Old parser expected error on line 2: expected expression
-      ]
+      """
     )
   }
 
@@ -436,9 +459,7 @@ final class OperatorDeclTests: XCTestCase {
       postfix operator ++:1️⃣
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: only infix operators may declare a precedence, Fix-It replacements: 20 - 21 = ''
-        DiagnosticSpec(message: "expected identifier in operator"),
-        // TODO: Old parser expected error on line 2: expected precedence group name after ':' in operator declaration
+        DiagnosticSpec(message: "expected precedence group in operator declaration"),
       ]
     )
   }

gyb_syntax_support/DeclNodes.py

@@ -726,7 +726,7 @@
          ]),
 
     # operator-decl -> attribute? modifiers? 'operator' operator
-    Node('OperatorDecl', name_for_diagnostics='operator', kind='Decl',
+    Node('OperatorDecl', name_for_diagnostics='operator declaration', kind='Decl',
          traits=['IdentifiedDecl'],
          description='A Swift `operator` declaration.',
          children=[
@@ -773,7 +773,7 @@
          ''',
          children=[
              Child('Colon', kind='ColonToken'),
-             Child('PrecedenceGroup', kind='IdentifierToken',
+             Child('PrecedenceGroup', name_for_diagnostics='precedence group', kind='IdentifierToken',
                    description='''
                    The precedence group for this operator
                    '''),