Improve recovery for identifiers and text segment options

Scan to the closing delimiter of an invalid
identifier, and better diagnose an invalid text
segment option.

Author: hamishknight

Sources/_RegexParser/Regex/Parse/Diagnostics.swift

@@ -55,6 +55,7 @@ enum ParseError: Error, Hashable {
 
   case unknownGroupKind(String)
   case unknownCalloutKind(String)
+  case unknownTextSegmentMatchingOption(Character)
 
   case invalidMatchingOption(Character)
   case cannotRemoveMatchingOptionsAfterCaret
@@ -166,6 +167,8 @@ extension ParseError: CustomStringConvertible {
       return "unknown group kind '(\(str)'"
     case let .unknownCalloutKind(str):
       return "unknown callout kind '\(str)'"
+    case let .unknownTextSegmentMatchingOption(m):
+      return "unknown text segment mode '\(m)'; expected 'w' or 'g'"
     case let .invalidMatchingOption(c):
       return "invalid matching option '\(c)'"
     case .cannotRemoveMatchingOptionsAfterCaret:

Sources/_RegexParser/Regex/Parse/LexicalAnalysis.swift

@@ -812,18 +812,28 @@ extension Parser {
         case "S": return .asciiOnlySpace
         case "W": return .asciiOnlyWord
         case "y":
-          p.expect("{")
+          // Default to grapheme cluster if unknown.
+          let recoveryMode = OptKind.textSegmentGraphemeMode
+          guard p.expect("{") else { return recoveryMode }
+
+          guard let optChar = p.tryEatWithLoc(), optChar.value != "}" else {
+            p.errorAtCurrentPosition(.expected("text segment mode"))
+            return recoveryMode
+          }
           let opt: OptKind
-          if p.tryEat("w") {
+          switch optChar.value {
+          case "w":
             opt = .textSegmentWordMode
-          } else {
-            p.expect("g")
+          case "g":
             opt = .textSegmentGraphemeMode
+          case let x:
+            p.error(.unknownTextSegmentMatchingOption(x), at: optChar.location)
+            opt = recoveryMode
           }
           p.expect("}")
           return opt
 
-          // Swift semantic level options
+        // Swift semantic level options
         case "X": return .graphemeClusterSemantics
         case "u": return .unicodeScalarSemantics
         case "b": return .byteSemantics
@@ -958,6 +968,8 @@ extension Parser {
       }
       guard let str = p.tryEatPrefix(\.isWordCharacter) else {
         p.error(.identifierMustBeAlphaNumeric(kind), at: firstChar.location)
+        // Try skip ahead to the closing delimiter for better recovery.
+        _ = p.lexUntil { $0.src.isEmpty || $0.src.starts(with: ending) }
         return ""
       }
       return str.value

Tests/RegexTests/ParseTests.swift

@@ -2802,11 +2802,10 @@ extension RegexTests {
     diagnosticTest(#"(?^"#, .expected(")"))
     diagnosticTest(#"(?^i"#, .expected(")"))
 
-    // TODO: These errors could be better.
-    diagnosticTest(#"(?y)"#, .expected("{"), .expected("g"), .expected("}"), unsupported: true)
-    diagnosticTest(#"(?y{)"#, .expected("g"), .expected("}"), unsupported: true)
+    diagnosticTest(#"(?y)"#, .expected("{"), unsupported: true)
+    diagnosticTest(#"(?y{)"#, .unknownTextSegmentMatchingOption(")"), .expected("}"), .expected(")"), unsupported: true)
     diagnosticTest(#"(?y{g)"#, .expected("}"), unsupported: true)
-    diagnosticTest(#"(?y{x})"#, .expected("g"), .expected("}"), .invalidMatchingOption("}"), unsupported: true)
+    diagnosticTest(#"(?y{x})"#, .unknownTextSegmentMatchingOption("x"), unsupported: true)
 
     diagnosticTest(#"(?P"#, .expected(")"))
     diagnosticTest(#"(?R"#, .expected(")"), unsupported: true)
@@ -3086,18 +3085,17 @@ extension RegexTests {
     diagnosticTest(#"(?k)"#, .unknownGroupKind("?k"))
     diagnosticTest(#"(?P#)"#, .invalidMatchingOption("#"))
 
-    // TODO: We shouldn't emit the expected closing delimiter here and elsewhere.
-    diagnosticTest(#"(?<#>)"#, .expected(">"), .identifierMustBeAlphaNumeric(.groupName))
+    diagnosticTest(#"(?<#>)"#, .identifierMustBeAlphaNumeric(.groupName))
     diagnosticTest(#"(?'1A')"#, .identifierCannotStartWithNumber(.groupName))
 
     // TODO: It might be better if tried to consume up to the closing `'` and
     // diagnosed an invalid group name based on that.
     diagnosticTest(#"(?'abc ')"#, .expected("'"))
 
-    diagnosticTest("(?'🔥')", .identifierMustBeAlphaNumeric(.groupName), .expected("'"))
+    diagnosticTest("(?'🔥')", .identifierMustBeAlphaNumeric(.groupName))
 
     diagnosticTest(#"(?'-')"#, .expectedIdentifier(.groupName), unsupported: true)
-    diagnosticTest(#"(?'--')"#, .identifierMustBeAlphaNumeric(.groupName), .expected("'"), unsupported: true)
+    diagnosticTest(#"(?'--')"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
     diagnosticTest(#"(?'a-b-c')"#, .expected("'"), unsupported: true)
 
     diagnosticTest("(?x)(? : )", .unknownGroupKind("? "))
@@ -3178,12 +3176,12 @@ extension RegexTests {
     diagnosticTest(#"\g{0}"#, .cannotReferToWholePattern)
     diagnosticTest(#"(?(0))"#, .cannotReferToWholePattern, unsupported: true)
 
-    diagnosticTest(#"(?&&)"#, .identifierMustBeAlphaNumeric(.groupName), .unbalancedEndOfGroup, .expected(")"), unsupported: true)
-    diagnosticTest(#"(?&-1)"#, .identifierMustBeAlphaNumeric(.groupName), .unbalancedEndOfGroup, .expected(")"), unsupported: true)
-    diagnosticTest(#"(?P>+1)"#, .identifierMustBeAlphaNumeric(.groupName), .unbalancedEndOfGroup, .expected(")"), unsupported: true)
-    diagnosticTest(#"(?P=+1)"#, .identifierMustBeAlphaNumeric(.groupName), .unbalancedEndOfGroup, .expected(")"), unsupported: true)
-    diagnosticTest(#"\k'#'"#, .identifierMustBeAlphaNumeric(.groupName), .expected("'"), unsupported: true)
-    diagnosticTest(#"(?&#)"#, .identifierMustBeAlphaNumeric(.groupName), .unbalancedEndOfGroup, .expected(")"), unsupported: true)
+    diagnosticTest(#"(?&&)"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
+    diagnosticTest(#"(?&-1)"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
+    diagnosticTest(#"(?P>+1)"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
+    diagnosticTest(#"(?P=+1)"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
+    diagnosticTest(#"\k'#'"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
+    diagnosticTest(#"(?&#)"#, .identifierMustBeAlphaNumeric(.groupName), unsupported: true)
 
     diagnosticTest(#"(?P>1)"#, .identifierCannotStartWithNumber(.groupName), unsupported: true)
     diagnosticTest(#"\k{1}"#, .identifierCannotStartWithNumber(.groupName), .invalidNamedReference("1"))