Fix character class range matching

Previously we performed a lexicographic
comparison with the bounds of a character class
range. However this produced surprising results,
and our implementation didn't properly handle
case sensitivity.

Update the logic to instead only allow single
scalar NFC bounds. The input is then converted to
NFC in grapheme semantic mode, and checked against
the range. In scalar semantic mode, the input
scalar is checked on its own. Additionally, fix
the case sensitivity handling such that we check
both the lowercase and uppercase version of the
input against the range.

Author: hamishknight

Sources/_RegexParser/Regex/AST/Atom.swift

@@ -816,8 +816,16 @@ extension AST.Atom {
   /// Whether this atom is valid as the operand of a custom character class
   /// range.
   public var isValidCharacterClassRangeBound: Bool {
-    // If we have a literal character value for this, it can be used as a bound.
-    if literalCharacterValue != nil { return true }
+    if let c = literalCharacterValue {
+      // We only match character range bounds that are single scalars under NFC.
+      // FIXME: Update to use the stdlib's data. We'll need to decide whether or
+      // not the `hasExactlyOneScalar` check should remain. In grapheme semantic
+      // mode, we ought to be able to convert multi-scalar characters to
+      // single-scalar NFC, but in scalar semantic mode it seems like we ought
+      // to preserve the scalar as-written, making multi-scalar cases invalid.
+      // As such there may be value in maintaining this restriction.
+      return c.hasExactlyOneScalar && !c.unicodeScalars.first!.expandsUnderNFC
+    }
     switch kind {
     // \cx, \C-x, \M-x, \M-\C-x, \N{...}
     case .keyboardControl, .keyboardMeta, .keyboardMetaControl, .namedCharacter:

Sources/_RegexParser/Utility/Misc.swift

@@ -19,6 +19,40 @@ extension Substring {
   var string: String { String(self) }
 }
 
+extension Character {
+  /// Whether this character is made up of exactly one Unicode scalar value.
+  public var hasExactlyOneScalar: Bool {
+    let scalars = unicodeScalars
+    return scalars.index(after: scalars.startIndex) == scalars.endIndex
+  }
+}
+
+extension UnicodeScalar {
+  /// Whether a given scalar expands when NFC is applied. This is a stop-gap
+  /// until we're able to use the normalization data provided by the stdlib.
+  var expandsUnderNFC: Bool {
+    if isASCII { return false }
+    // As of Unicode 14, there are 85 code points that expand under NFC.
+    // FIXME: This should use the stdlib data, but we don't have access to it
+    // from _RegexParser.
+    switch value {
+    case 0x344, 0x958, 0x959, 0x95A, 0x95B, 0x95C, 0x95D, 0x95E, 0x95F, 0x9DC,
+      0x9DD, 0x9DF, 0xA33, 0xA36, 0xA59, 0xA5A, 0xA5B, 0xA5E, 0xB5C, 0xB5D,
+      0xF43, 0xF4D, 0xF52, 0xF57, 0xF5C, 0xF69, 0xF73, 0xF75, 0xF76, 0xF78,
+      0xF81, 0xF93, 0xF9D, 0xFA2, 0xFA7, 0xFAC, 0xFB9, 0x2ADC, 0xFB1D, 0xFB1F,
+      0xFB2A, 0xFB2B, 0xFB2C, 0xFB2D, 0xFB2E, 0xFB2F, 0xFB30, 0xFB31, 0xFB32,
+      0xFB33, 0xFB34, 0xFB35, 0xFB36, 0xFB38, 0xFB39, 0xFB3A, 0xFB3B, 0xFB3C,
+      0xFB3E, 0xFB40, 0xFB41, 0xFB43, 0xFB44, 0xFB46, 0xFB47, 0xFB48, 0xFB49,
+      0xFB4A, 0xFB4B, 0xFB4C, 0xFB4D, 0xFB4E, 0x1D15E, 0x1D15F, 0x1D160,
+      0x1D161, 0x1D162, 0x1D163, 0x1D164, 0x1D1BB, 0x1D1BC, 0x1D1BD, 0x1D1BE,
+      0x1D1BF, 0x1D1C0:
+      return true
+    default:
+      return false
+    }
+  }
+}
+
 extension CustomStringConvertible {
   @_alwaysEmitIntoClient
   public var halfWidthCornerQuoted: String {

Sources/_StringProcessing/ConsumerInterface.swift

@@ -342,38 +342,60 @@ extension DSLTree.CustomCharacterClass.Member {
       }
       return c
     case let .range(low, high):
-      // TODO:
-      guard let lhs = low.literalCharacterValue else {
+      guard let lhs = low.literalCharacterValue?.singleNFCScalar else {
         throw Unsupported("\(low) in range")
       }
-      guard let rhs = high.literalCharacterValue else {
+      guard let rhs = high.literalCharacterValue?.singleNFCScalar else {
         throw Unsupported("\(high) in range")
       }
+      guard lhs <= rhs else {
+        throw Unsupported("Invalid range \(low)-\(high)")
+      }
 
-      if opts.isCaseInsensitive {
-        let lhsLower = lhs.lowercased()
-        let rhsLower = rhs.lowercased()
-        guard lhsLower <= rhsLower else { throw Unsupported("Invalid range \(lhs)-\(rhs)") }
-        return { input, bounds in
-          // TODO: check for out of bounds?
-          let curIdx = bounds.lowerBound
-          if (lhsLower...rhsLower).contains(input[curIdx].lowercased()) {
-            // TODO: semantic level
-            return input.index(after: curIdx)
-          }
-          return nil
+      let isCaseInsensitive = opts.isCaseInsensitive
+      let isCharacterSemantic = opts.semanticLevel == .graphemeCluster
+      
+      return { input, bounds in
+        let curIdx = bounds.lowerBound
+        let nextIndex = isCharacterSemantic
+          ? input.index(after: curIdx)
+          : input.unicodeScalars.index(after: curIdx)
+
+        // Under grapheme semantics, we compare based on single NFC scalars. If
+        // such a character is not single scalar under NFC, the match fails. In
+        // scalar semantics, we compare the exact scalar value to the NFC
+        // bounds.
+        let scalar = isCharacterSemantic ? input[curIdx].singleNFCScalar
+                                         : input.unicodeScalars[curIdx]
+        guard let scalar = scalar else { return nil }
+        let scalarRange = lhs ... rhs
+        if scalarRange.contains(scalar) {
+          return nextIndex
         }
-      } else {
-        guard lhs <= rhs else { throw Unsupported("Invalid range \(lhs)-\(rhs)") }
-        return { input, bounds in
-          // TODO: check for out of bounds?
-          let curIdx = bounds.lowerBound
-          if (lhs...rhs).contains(input[curIdx]) {
-            // TODO: semantic level
-            return input.index(after: curIdx)
+
+        // Check for case insensitive matches.
+        func matchesCased(
+          _ cased: (UnicodeScalar.Properties) -> String
+        ) -> Bool {
+          let casedStr = cased(scalar.properties)
+          // In character semantic mode, we need to map to NFC. In scalar
+          // semantics, we should have an exact scalar.
+          let mapped = isCharacterSemantic ? casedStr.singleNFCScalar
+                                           : casedStr.singleScalar
+          guard let mapped = mapped else { return false }
+          return scalarRange.contains(mapped)
+        }
+        if isCaseInsensitive {
+          if scalar.properties.changesWhenLowercased,
+              matchesCased(\.lowercaseMapping) {
+            return nextIndex
+          }
+          if scalar.properties.changesWhenUppercased,
+             matchesCased(\.uppercaseMapping) {
+            return nextIndex
           }
-          return nil
         }
+        return nil
       }
 
     case let .custom(ccc):

Sources/_StringProcessing/Unicode/CharacterProps.swift

@@ -11,10 +11,3 @@
 
 
 // TODO
-
-extension Character {
-  /// Whether this character is made up of exactly one Unicode scalar value.
-  var hasExactlyOneScalar: Bool {
-    unicodeScalars.index(after: unicodeScalars.startIndex) == unicodeScalars.endIndex
-  }
-}

Sources/_StringProcessing/Unicode/NFC.swift

@@ -0,0 +1,44 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2022 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+//
+//===----------------------------------------------------------------------===//
+
+@_spi(_Unicode)
+import Swift
+
+extension Character {
+  /// If the given character consists of a single NFC scalar, returns it. If
+  /// there are multiple NFC scalars, returns `nil`.
+  var singleNFCScalar: UnicodeScalar? {
+    // SwiftStdlib is always >= 5.7 for a shipped StringProcessing.
+    guard #available(SwiftStdlib 5.7, *) else { return nil }
+    var nfcIter = String(self)._nfc.makeIterator()
+    guard let scalar = nfcIter.next(), nfcIter.next() == nil else { return nil }
+    return scalar
+  }
+}
+
+extension String {
+  /// If the given string consists of a single NFC scalar, returns it. If none
+  /// or multiple NFC scalars are present, returns `nil`.
+  var singleNFCScalar: UnicodeScalar? {
+    guard !isEmpty && index(after: startIndex) == endIndex else { return nil }
+    return first!.singleNFCScalar
+  }
+
+  /// If the given string contains a single scalar, returns it. If none or
+  /// multiple scalars are present, returns `nil`.
+  var singleScalar: UnicodeScalar? {
+    let scalars = unicodeScalars
+    guard !scalars.isEmpty &&
+          scalars.index(after: scalars.startIndex) == scalars.endIndex
+    else { return nil }
+    return scalars.first!
+  }
+}

Tests/RegexBuilderTests/RegexDSLTests.swift

@@ -71,6 +71,9 @@ class RegexDSLTests: XCTestCase {
   }
 
   func testCharacterClasses() throws {
+    // Must have new stdlib for character class ranges.
+    guard ensureNewStdlib() else { return }
+
     try _testDSLCaptures(
       ("a c", ("a c", " ", "c")),
       matchType: (Substring, Substring, Substring).self, ==)
@@ -114,6 +117,9 @@ class RegexDSLTests: XCTestCase {
   }
 
   func testCharacterClassOperations() throws {
+    // Must have new stdlib for character class ranges.
+    guard ensureNewStdlib() else { return }
+
     try _testDSLCaptures(
       ("bcdefn1a", "bcdefn1a"),
       ("nbcdef1a", nil),        // fails symmetric difference lookahead
@@ -345,6 +351,9 @@ class RegexDSLTests: XCTestCase {
   }
 
   func testQuantificationBehavior() throws {
+    // Must have new stdlib for character class ranges.
+    guard ensureNewStdlib() else { return }
+
     // Eager by default
     try _testDSLCaptures(
       ("abc1def2", ("abc1def2", "2")),