Treat range quantifiers as literal if invalid

This matches the PCRE and Oniguruma behavior. Also
make sure the matching engine can handle a 0 range.

Author: hamishknight

Sources/_MatchingEngine/Regex/Parse/LexicalAnalysis.swift

@@ -113,6 +113,22 @@ extension Source {
     try tryEatNonEmpty(sequence: String(c))
   }
 
+  /// Attempt to make a series of lexing steps in `body`, returning `nil` if
+  /// unsuccesful, which will revert the source back to its previous state. If
+  /// an error is thrown, the source will not be reverted.
+  mutating func tryEating<T>(
+    _ body: (inout Source) throws -> T?
+  ) rethrows -> T? {
+    // We don't revert the source if an error is thrown, as it's useful to
+    // maintain the source location in that case.
+    let current = self
+    guard let result = try body(&self) else {
+      self = current
+      return nil
+    }
+    return result
+  }
+
   /// Throws an expected ASCII character error if not matched
   mutating func expectASCII() throws -> Located<Character> {
     try recordLoc { src in
@@ -296,16 +312,11 @@ extension Source {
       if src.tryEat("+") { return .oneOrMore }
       if src.tryEat("?") { return .zeroOrOne }
 
-      // FIXME: Actually, PCRE treats empty as literal `{}`...
-      // But Java 8 errors out?
-      if src.tryEat("{") {
-        // FIXME: Erm, PCRE parses as literal if no lowerbound...
-        let amt = try src.expectRange()
-        try src.expect("}")
-        return amt.value // FIXME: track actual range...
+      return try src.tryEating { src in
+        guard src.tryEat("{"), let range = try src.lexRange(), src.tryEat("}")
+        else { return nil }
+        return range.value
       }
-
-      return nil
     }
     guard let amt = amt else { return nil }
 
@@ -318,56 +329,56 @@ extension Source {
     return (amt, kind)
   }
 
-  /// Consume a range
+  /// Try to consume a range, returning `nil` if unsuccessful.
   ///
   ///     Range       -> ',' <Int> | <Int> ',' <Int>? | <Int>
   ///                  | ExpRange
   ///     ExpRange    -> '..<' <Int> | '...' <Int>
   ///                  | <Int> '..<' <Int> | <Int> '...' <Int>?
-  mutating func expectRange() throws -> Located<Quant.Amount> {
+  mutating func lexRange() throws -> Located<Quant.Amount>? {
     try recordLoc { src in
-      // TODO: lex positive numbers, more specifically...
-
-      let lowerOpt = try src.lexNumber()
-
-      // ',' or '...' or '..<' or nothing
-      let closedRange: Bool?
-      if src.tryEat(",") {
-        closedRange = true
-      } else if src.experimentalRanges && src.tryEat(".") {
-        try src.expect(".")
-        if src.tryEat(".") {
+      try src.tryEating { src in
+        let lowerOpt = try src.lexNumber()
+
+        // ',' or '...' or '..<' or nothing
+        // TODO: We ought to try and consume whitespace here and emit a
+        // diagnostic for the user warning them that it would cause the range to
+        // be treated as literal.
+        let closedRange: Bool?
+        if src.tryEat(",") {
           closedRange = true
+        } else if src.experimentalRanges && src.tryEat(".") {
+          try src.expect(".")
+          if src.tryEat(".") {
+            closedRange = true
+          } else {
+            try src.expect("<")
+            closedRange = false
+          }
         } else {
-          try src.expect("<")
-          closedRange = false
+          closedRange = nil
         }
-      } else {
-        closedRange = nil
-      }
-      // FIXME: wait, why `try!` ?
-      let upperOpt: Located<Int>?
-      if let u = try! src.lexNumber() {
-        upperOpt = (closedRange == true) ? u : Located(u.value-1, u.location)
-      } else {
-        upperOpt = nil
-      }
 
-      switch (lowerOpt, closedRange, upperOpt) {
-      case let (l?, nil, nil):
-        return .exactly(l)
-      case let (l?, true, nil):
-        return .nOrMore(l)
-      case let (nil, _, u?):
-        return .upToN(u)
-      case let (l?, _, u?):
-        // FIXME: source location tracking
-        return .range(l, u)
-
-      case let (nil, nil, u) where u != nil:
-        fatalError("Not possible")
-      default:
-        throw ParseError.misc("Invalid range")
+        let upperOpt = try src.lexNumber()?.map { upper in
+          // If we have an open range, the upper bound should be adjusted down.
+          closedRange == true ? upper : upper - 1
+        }
+
+        switch (lowerOpt, closedRange, upperOpt) {
+        case let (l?, nil, nil):
+          return .exactly(l)
+        case let (l?, true, nil):
+          return .nOrMore(l)
+        case let (nil, _?, u?):
+          return .upToN(u)
+        case let (l?, _?, u?):
+          return .range(l, u)
+
+        case (nil, nil, _?):
+          fatalError("Didn't lex lower bound, but lexed upper bound?")
+        default:
+          return nil
+        }
       }
     }
   }

Sources/_MatchingEngine/Regex/Parse/SourceLocation.swift

@@ -76,6 +76,10 @@ extension Source {
       // externally?
       self.init(v, .fake)
     }
+
+    public func map<U>(_ fn: (T) throws -> U) rethrows -> Located<U> {
+      Located<U>(try fn(value), location)
+    }
   }
 }
 extension AST {

Tests/RegexTests/MatchTests.swift

@@ -181,6 +181,10 @@ extension RegexTests {
       #"a{1,2}?"#, input: "123aaaxyz", match: "a")
     matchTest(
       #"a{1,2}?x"#, input: "123aaaxyz", match: "aax")
+    matchTest(
+      #"xa{0}y"#, input: "123aaaxyz", match: "xy")
+    matchTest(
+      #"xa{0,0}y"#, input: "123aaaxyz", match: "xy")
 
     matchTest("a.*", input: "dcba", match: "a")
 

Tests/RegexTests/ParseTests.swift

@@ -448,6 +448,34 @@ extension RegexTests {
     parseTest(
       #"a{1,2}?"#,
       quantRange(.reluctant, 1...2, "a"))
+    parseTest(
+      #"a{0}"#,
+      exactly(.eager, 0, "a"))
+    parseTest(
+      #"a{0,0}"#,
+      quantRange(.eager, 0...0, "a"))
+
+    // Make sure ranges get treated as literal if invalid.
+    parseTest("{", "{")
+    parseTest("{,", concat("{", ","))
+    parseTest("{}", concat("{", "}"))
+    parseTest("{,}", concat("{", ",", "}"))
+    parseTest("{,6", concat("{", ",", "6"))
+    parseTest("{6", concat("{", "6"))
+    parseTest("{6,", concat("{", "6", ","))
+    parseTest("{+", oneOrMore(.eager, "{"))
+    parseTest("{6,+", concat("{", "6", oneOrMore(.eager, ",")))
+    parseTest("x{", concat("x", "{"))
+    parseTest("x{}", concat("x", "{", "}"))
+    parseTest("x{,}", concat("x", "{", ",", "}"))
+    parseTest("x{,6", concat("x", "{", ",", "6"))
+    parseTest("x{6", concat("x", "{", "6"))
+    parseTest("x{6,", concat("x", "{", "6", ","))
+    parseTest("x{+", concat("x", oneOrMore(.eager, "{")))
+    parseTest("x{6,+", concat("x", "{", "6", oneOrMore(.eager, ",")))
+
+    // TODO: We should emit a diagnostic for this.
+    parseTest("x{3, 5}", concat("x", "{", "3", ",", " ", "5", "}"))
 
     // MARK: Groups