Validate capture lists

Begin storing source location on capture lists,
and start erroring on duplicate named captures.

Author: hamishknight

Sources/_RegexParser/Regex/Parse/CaptureList.swift

@@ -26,15 +26,18 @@ extension CaptureList {
     public var name: String?
     public var type: Any.Type?
     public var optionalDepth: Int
+    public var location: SourceLocation
 
     public init(
       name: String? = nil,
       type: Any.Type? = nil,
-      optionalDepth: Int
+      optionalDepth: Int,
+      _ location: SourceLocation
     ) {
       self.name = name
       self.type = type
       self.optionalDepth = optionalDepth
+      self.location = location
     }
   }
 }
@@ -61,13 +64,14 @@ extension AST.Node {
     case let .group(g):
       switch g.kind.value {
       case .capture:
-        list.append(.init(optionalDepth: nesting))
+        list.append(.init(optionalDepth: nesting, g.location))
 
       case .namedCapture(let name):
-        list.append(.init(name: name.value, optionalDepth: nesting))
+        list.append(.init(name: name.value, optionalDepth: nesting, g.location))
 
       case .balancedCapture(let b):
-        list.append(.init(name: b.name?.value, optionalDepth: nesting))
+        list.append(.init(name: b.name?.value, optionalDepth: nesting,
+                          g.location))
 
       default: break
       }
@@ -124,7 +128,8 @@ extension CaptureList.Capture: Equatable {
   public static func == (lhs: Self, rhs: Self) -> Bool {
     lhs.name == rhs.name &&
     lhs.optionalDepth == rhs.optionalDepth &&
-    lhs.type == rhs.type
+    lhs.type == rhs.type &&
+    lhs.location == rhs.location
   }
 }
 extension CaptureList: Equatable {}

Sources/_RegexParser/Regex/Parse/Sema.swift

@@ -34,6 +34,7 @@ extension RegexValidator {
     for opt in ast.globalOptions?.options ?? [] {
       try validateGlobalMatchingOption(opt)
     }
+    try validateCaptures()
     try validateNode(ast.root)
   }
 
@@ -59,6 +60,17 @@ extension RegexValidator {
     }
   }
 
+  func validateCaptures() throws {
+    // TODO: Should this be validated when creating the capture list?
+    var usedNames = Set<String>()
+    for capture in captures.captures {
+      guard let name = capture.name else { continue }
+      guard usedNames.insert(name).inserted else {
+        throw error(.duplicateNamedCapture(name), at: capture.location)
+      }
+    }
+  }
+
   func validateReference(_ ref: AST.Reference) throws {
     switch ref.kind {
     case .absolute(let i):

Sources/_StringProcessing/Regex/DSLTree.swift

@@ -472,7 +472,7 @@ extension DSLTree.Node {
       list.append(.init(
         name: name,
         type: child.valueCaptureType?.base,
-        optionalDepth: nesting))
+        optionalDepth: nesting, .fake))
       child._addCaptures(to: &list, optionalNesting: nesting)
 
     case let .nonCapturingGroup(kind, child):

Tests/RegexTests/CaptureTests.swift

@@ -16,36 +16,44 @@ import XCTest
 
 extension CaptureList.Capture {
   static var cap: Self {
-    return Self(optionalDepth: 0)
+    return Self(optionalDepth: 0, .fake)
   }
 
   static var opt: Self {
-    return Self(optionalDepth: 1)
+    return Self(optionalDepth: 1, .fake)
   }
   static var opt_opt: Self {
-    return Self(optionalDepth: 2)
+    return Self(optionalDepth: 2, .fake)
   }
   static var opt_opt_opt: Self {
-    return Self(optionalDepth: 3)
+    return Self(optionalDepth: 3, .fake)
   }
   static var opt_opt_opt_opt: Self {
-    return Self(optionalDepth: 4)
+    return Self(optionalDepth: 4, .fake)
   }
   static var opt_opt_opt_opt_opt: Self {
-    return Self(optionalDepth: 5)
+    return Self(optionalDepth: 5, .fake)
   }
   static var opt_opt_opt_opt_opt_opt: Self {
-    return Self(optionalDepth: 6)
+    return Self(optionalDepth: 6, .fake)
   }
 
   static func named(_ name: String, opt: Int = 0) -> Self {
-    return Self(name: name, optionalDepth: opt)
+    return Self(name: name, optionalDepth: opt, .fake)
   }
 }
 extension CaptureList {
   static func caps(count: Int) -> Self {
     Self(Array(repeating: .cap, count: count))
   }
+
+  var withoutLocs: Self {
+    var copy = self
+    for idx in copy.captures.indices {
+      copy.captures[idx].location = .fake
+    }
+    return copy
+  }
 }
 
 extension StructuredCapture {
@@ -151,7 +159,7 @@ func captureTest(
   line: UInt = #line
 ) {
   let ast = try! parse(regex, .semantic, .traditional)
-  let capList = ast.root._captureList
+  let capList = ast.root._captureList.withoutLocs
   guard capList == expected else {
     XCTFail("""
       Expected:

Tests/RegexTests/ParseTests.swift

@@ -94,7 +94,7 @@ func parseTest(
             file: file, line: line)
     return
   }
-  let captures = ast.captureList
+  let captures = ast.captureList.withoutLocs
   guard captures == expectedCaptures else {
     XCTFail("""
 
@@ -872,7 +872,7 @@ extension RegexTests {
     parseTest(
       "(?|(?<x>a)|(?<x>b))",
       nonCaptureReset(alt(namedCapture("x", "a"), namedCapture("x", "b"))),
-      throwsError: .unsupported, captures: [.named("x", opt: 1), .named("x", opt: 1)]
+      throwsError: .invalid, captures: [.named("x", opt: 1), .named("x", opt: 1)]
     )
 
     // TODO: Reject mismatched names?
@@ -2539,6 +2539,12 @@ extension RegexTests {
 
     diagnosticTest("(?x)(? : )", .unknownGroupKind("? "))
 
+    diagnosticTest("(?<x>)(?<x>)", .duplicateNamedCapture("x"))
+    diagnosticTest("(?<x>)|(?<x>)", .duplicateNamedCapture("x"))
+    diagnosticTest("((?<x>))(?<x>)", .duplicateNamedCapture("x"))
+    diagnosticTest("(|(?<x>))(?<x>)", .duplicateNamedCapture("x"))
+    diagnosticTest("(?<x>)(?<y>)(?<x>)", .duplicateNamedCapture("x"))
+
     // MARK: Quantifiers
 
     diagnosticTest("*", .quantifierRequiresOperand("*"))