Add diagnostic for unexpected second identifier

Author: kimdv

Sources/SwiftParser/Patterns.swift

@@ -219,7 +219,12 @@ extension Parser {
         let labelAndColon = self.consume(if: .identifier, followedBy: .colon)
         let (label, colon) = (labelAndColon?.0, labelAndColon?.1)
         let pattern = self.parsePattern()
-        let trailingComma = self.consume(if: .comma)
+        var trailingComma = self.consume(if: .comma)
+
+        if trailingComma == nil && currentToken.rawTokenKind == .identifier {
+          trailingComma = self.missingToken(RawTokenKind.comma)
+        }
+
         keepGoing = trailingComma != nil
         elements.append(
           RawTuplePatternElementSyntax(

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -878,6 +878,24 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if let tuplePatternSyntax = node.pattern.as(TuplePatternSyntax.self) {
+      for element in tuplePatternSyntax.elements where element.hasError && element.trailingComma?.isMissingAllTokens == true {
+        if let previousToken = node.previousToken(viewMode: .sourceAccurate) {
+          addDiagnostic(element, position: element.nextToken?.position, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []))
+        }
+      }
+    } else if node.typeAnnotation?.hasError == true, let typeAnnotation = node.typeAnnotation, let previousToken = node.previousToken(viewMode: .sourceAccurate), typeAnnotation.colon.hasError {
+      addDiagnostic(typeAnnotation.type, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []))
+    }
+
+    return .visitChildren
+  }
+
   public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -341,13 +341,16 @@ public struct SpaceSeparatedIdentifiersError: ParserError {
   public let additionalTokens: [TokenSyntax]
 
   public var message: String {
+    let text: String
     if let name = firstToken.parent?.ancestorOrSelf(mapping: {
       $0.nodeTypeNameForDiagnostics(allowBlockNames: false)
     }) {
-      return "found an unexpected second identifier in \(name)"
+      text = "found an unexpected second identifier in \(name)"
     } else {
-      return "found an unexpected second identifier"
+      text = "found an unexpected second identifier"
     }
+
+    return "\(text); is there an accidental break?"
   }
 }
 

Tests/SwiftParserTest/ParserTests.swift

@@ -54,7 +54,7 @@ public class ParserTests: XCTestCase {
     }
   }
 
-  /// Run parsr tests on all of the Swift files in the given path, recursively.
+  /// Run parser tests on all of the Swift files in the given path, recursively.
   func runParserTests(
     name: String,
     path: URL,

Tests/SwiftParserTest/TypeTests.swift

@@ -22,7 +22,8 @@ final class TypeTests: XCTestCase {
       var foo 1️⃣Bar = 1
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected ':' in type annotation")
+        DiagnosticSpec(message: "expected ':' in type annotation"),
+        DiagnosticSpec(message: "found an unexpected second identifier in variable; is there an accidental break?"),
       ]
     )
   }

Tests/SwiftParserTest/translated/InvalidTests.swift

@@ -394,7 +394,7 @@ final class InvalidTests: XCTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(
-          message: "found an unexpected second identifier in function",
+          message: "found an unexpected second identifier in function; is there an accidental break?",
           fixIts: [
             "join the identifiers together",
             "join the identifiers together with camel-case",
@@ -413,7 +413,7 @@ final class InvalidTests: XCTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(
-          message: "found an unexpected second identifier in function",
+          message: "found an unexpected second identifier in function; is there an accidental break?",
           fixIts: [
             "join the identifiers together",
             "join the identifiers together with camel-case",
@@ -432,7 +432,7 @@ final class InvalidTests: XCTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(
-          message: "found an unexpected second identifier in function",
+          message: "found an unexpected second identifier in function; is there an accidental break?",
           fixIts: [
             "join the identifiers together",
             "join the identifiers together with camel-case",
@@ -450,7 +450,7 @@ final class InvalidTests: XCTestCase {
       func cat 1️⃣Mouse() {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: ["join the identifiers together"])
+        DiagnosticSpec(message: "found an unexpected second identifier in function; is there an accidental break?", fixIts: ["join the identifiers together"])
       ],
       fixedSource: "func catMouse() {}"
     )
@@ -463,7 +463,7 @@ final class InvalidTests: XCTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(
-          message: "found an unexpected second identifier in function",
+          message: "found an unexpected second identifier in function; is there an accidental break?",
           fixIts: [
             "join the identifiers together",
             "join the identifiers together with camel-case",

Tests/SwiftParserTest/translated/RecoveryTests.swift

@@ -765,7 +765,7 @@ final class RecoveryTests: XCTestCase {
       protocol Multi 1️⃣ident {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "found an unexpected second identifier in protocol")
+        DiagnosticSpec(message: "found an unexpected second identifier in protocol; is there an accidental break?")
       ]
     )
   }
@@ -776,7 +776,7 @@ final class RecoveryTests: XCTestCase {
       class CCC 1️⃣CCC<T> {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "found an unexpected second identifier in class")
+        DiagnosticSpec(message: "found an unexpected second identifier in class; is there an accidental break?")
       ],
       fixedSource: """
         class CCCCCC<T> {}
@@ -793,7 +793,7 @@ final class RecoveryTests: XCTestCase {
       }
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum; is there an accidental break?"),
         DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'"),
         DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
       ]
@@ -812,11 +812,12 @@ final class RecoveryTests: XCTestCase {
       }
       """#,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct; is there an accidental break?"),
         DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' in type annotation"),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "found an unexpected second identifier in variable; is there an accidental break?"),
         DiagnosticSpec(locationMarker: "3️⃣", message: #"unexpected code ': Int = ""' before function"#),
-        // TODO: (good first issue) Old parser expected error on line 4: found an unexpected second identifier in variable declaration; is there an accidental break?
         DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' in type annotation"),
+        DiagnosticSpec(locationMarker: "4️⃣", message: "found an unexpected second identifier in variable; is there an accidental break?"),
       ]
     )
   }
@@ -827,8 +828,8 @@ final class RecoveryTests: XCTestCase {
       let (efg 1️⃣hij, foobar) = (5, 6)
       """,
       diagnostics: [
-        // TODO: (good first issue) Old parser expected error on line 1: found an unexpected second identifier in constant declaration; is there an accidental break?
-        DiagnosticSpec(message: "unexpected code 'hij, foobar' in tuple pattern")
+        DiagnosticSpec(message: "expected ',' in tuple pattern"),
+        DiagnosticSpec(message: "found an unexpected second identifier in variable; is there an accidental break?"),
       ]
     )
   }