Allow scoped removal of extended syntax in multi-line literals

Relax the ban on unsetting extended syntax in a
multi-line literal such that it does not apply to
a scoped unset e.g `(?-x:...)`, as long as it does
not span multiple lines.

Author: hamishknight

Sources/_RegexParser/Regex/Parse/Diagnostics.swift

@@ -45,6 +45,7 @@ enum ParseError: Error, Hashable {
   case confusableCharacter(Character)
 
   case quoteMayNotSpanMultipleLines
+  case unsetExtendedSyntaxMayNotSpanMultipleLines
 
   case cannotReferToWholePattern
 
@@ -143,6 +144,8 @@ extension ParseError: CustomStringConvertible {
       return "'\(c)' is confusable for a metacharacter; use '\\u{...}' instead"
     case .quoteMayNotSpanMultipleLines:
       return "quoted sequence may not span multiple lines in multi-line literal"
+    case .unsetExtendedSyntaxMayNotSpanMultipleLines:
+      return "group that unsets extended syntax may not span multiple lines in multi-line literal"
     case .cannotReferToWholePattern:
       return "cannot refer to whole pattern here"
     case .quantifierRequiresOperand(let q):

Sources/_RegexParser/Regex/Parse/LexicalAnalysis.swift

@@ -835,10 +835,6 @@ extension Source {
         if opt.isSemanticMatchingLevel {
           throw ParseError.cannotRemoveSemanticsOptions
         }
-        // Extended syntax may not be removed if in multi-line mode.
-        if context.syntax.contains(.multilineLiteral) && opt.isAnyExtended {
-          throw ParseError.cannotRemoveExtendedSyntaxInMultilineMode
-        }
         removing.append(opt)
       }
       return .init(caretLoc: nil, adding: adding, minusLoc: ateMinus.location,

Sources/_RegexParser/Regex/Parse/Parse.swift

@@ -289,12 +289,16 @@ extension Parser {
   /// Apply the syntax options of a given matching option sequence to the
   /// current set of options.
   private mutating func applySyntaxOptions(
-    of opts: AST.MatchingOptionSequence
-  ) {
+    of opts: AST.MatchingOptionSequence, isScoped: Bool
+  ) throws {
     func mapOption(_ option: SyntaxOptions,
                    _ pred: (AST.MatchingOption) -> Bool) {
       if opts.resetsCurrentOptions {
-        context.syntax.remove(option)
+        // Extended syntax is never reset in a multi-line literal.
+        if !(option == .extendedSyntax &&
+             context.syntax.contains(.multilineLiteral)) {
+          context.syntax.remove(option)
+        }
       }
       if opts.adding.contains(where: pred) {
         context.syntax.insert(option)
@@ -311,22 +315,42 @@ extension Parser {
     mapOption(.namedCapturesOnly, .namedCapturesOnly)
 
     // (?x), (?xx)
-    // We skip this for multi-line, as extended syntax is always enabled there.
+    // We skip this for unscoped multi-line, as extended syntax is always
+    // enabled there. A scoped group however (?-x:...) allows extended syntax to
+    // be disabled.
     // TODO: PCRE differentiates between (?x) and (?xx) where only the latter
     // handles non-semantic whitespace in a custom character class. Other
     // engines such as Oniguruma, Java, and ICU do this under (?x). Therefore,
     // treat (?x) and (?xx) as the same option here. If we ever get a strict
     // PCRE mode, we will need to change this to handle that.
-    if !context.syntax.contains(.multilineLiteral) {
+    if !isScoped && context.syntax.contains(.multilineLiteral) {
+      // An unscoped removal of extended syntax is not allowed in a multi-line
+      // literal.
+      if let opt = opts.removing.first(where: \.isAnyExtended) {
+        throw Source.LocatedError(
+          ParseError.cannotRemoveExtendedSyntaxInMultilineMode, opt.location)
+      }
+      // We may have an unscoped change of extended syntax, but it must either
+      // be:
+      // - An addition of extended syntax
+      // - A reset of matching options
+      // The former is redundant, and the latter shouldn't affect extended
+      // syntax in a multi-line literal. So we don't need to do anything extra
+      // here.
+    } else {
+      // We either have a scoped change of extended syntax, or this is a
+      // single-line literal.
       mapOption(.extendedSyntax, \.isAnyExtended)
     }
   }
 
   /// Apply the syntax options of a matching option changing group to the
   /// current set of options.
-  private mutating func applySyntaxOptions(of group: AST.Group.Kind) {
+  private mutating func applySyntaxOptions(
+    of group: AST.Group.Kind, isScoped: Bool
+  ) throws {
     if case .changeMatchingOptions(let seq) = group {
-      applySyntaxOptions(of: seq)
+      try applySyntaxOptions(of: seq, isScoped: isScoped)
     }
   }
 
@@ -337,14 +361,25 @@ extension Parser {
     context.recordGroup(kind.value)
 
     let currentSyntax = context.syntax
-    applySyntaxOptions(of: kind.value)
+    try applySyntaxOptions(of: kind.value, isScoped: true)
     defer {
       context.syntax = currentSyntax
     }
-
+    let unsetsExtendedSyntax = currentSyntax.contains(.extendedSyntax) &&
+                              !context.syntax.contains(.extendedSyntax)
     let child = try parseNode()
     try source.expect(")")
-    return .init(kind, child, loc(start))
+    let groupLoc = loc(start)
+
+    // In multi-line literals, the body of a group that unsets extended syntax
+    // may not span multiple lines.
+    if unsetsExtendedSyntax &&
+        context.syntax.contains(.multilineLiteral) &&
+        source[child.location.range].spansMultipleLinesInRegexLiteral {
+      throw Source.LocatedError(
+        ParseError.unsetExtendedSyntaxMayNotSpanMultipleLines, groupLoc)
+    }
+    return .init(kind, child, groupLoc)
   }
 
   /// Consume the body of an absent function.
@@ -438,7 +473,7 @@ extension Parser {
       // If we have a change matching options atom, apply the syntax options. We
       // already take care of scoping syntax options within a group.
       if case .changeMatchingOptions(let opts) = atom.kind {
-        applySyntaxOptions(of: opts)
+        try applySyntaxOptions(of: opts, isScoped: false)
       }
       // TODO: track source locations
       return .atom(atom)
@@ -592,7 +627,7 @@ public func parse<S: StringProtocol>(
   return ast
 }
 
-extension String {
+extension StringProtocol {
   /// Whether the given string is considered multi-line for a regex literal.
   var spansMultipleLinesInRegexLiteral: Bool {
     unicodeScalars.contains(where: { $0 == "\n" || $0 == "\r" })

Tests/RegexTests/ParseTests.swift

@@ -1777,6 +1777,13 @@ extension RegexTests {
         " ", "b"
       )
     )
+    parseTest(
+      "(?x) a (?^: b)", concat(
+        changeMatchingOptions(matchingOptions(adding: .extended)),
+        "a",
+        changeMatchingOptions(unsetMatchingOptions(), concat(" ", "b"))
+      )
+    )
 
     parseTest("[ # abc]", charClass(" ", "#", " ", "a", "b", "c"))
     parseTest("[#]", charClass("#"))
@@ -2193,6 +2200,14 @@ extension RegexTests {
       /#
       """, changeMatchingOptions(unsetMatchingOptions())
     )
+    parseWithDelimitersTest("""
+      #/
+      (?^:
+      # comment
+      )
+      /#
+      """, changeMatchingOptions(unsetMatchingOptions(), empty())
+    )
 
     // (?x) has no effect.
     parseWithDelimitersTest("""
@@ -2203,6 +2218,27 @@ extension RegexTests {
       """, changeMatchingOptions(matchingOptions(adding: .extended))
     )
 
+    // Scoped removal of extended syntax is allowed as long as it does not span
+    // multiple lines.
+    parseWithDelimitersTest("""
+      #/
+      (?-x:a b)
+      /#
+      """, changeMatchingOptions(
+        matchingOptions(removing: .extended),
+        concat("a", " ", "b")
+      )
+    )
+    parseWithDelimitersTest("""
+      #/
+      (?-xx:a b)
+      /#
+      """, changeMatchingOptions(
+        matchingOptions(removing: .extraExtended),
+        concat("a", " ", "b")
+      )
+    )
+
     parseWithDelimitersTest(#"""
       #/
       \p{
@@ -2782,17 +2818,35 @@ extension RegexTests {
       /#
       """, .cannotRemoveExtendedSyntaxInMultilineMode
     )
+
+    // Scoped removal of extended syntax may not span multiple lines
     diagnosticWithDelimitersTest("""
       #/
-      (?-x:a b)
+      (?-x:a b
+      )
       /#
-      """, .cannotRemoveExtendedSyntaxInMultilineMode
+      """, .unsetExtendedSyntaxMayNotSpanMultipleLines
     )
     diagnosticWithDelimitersTest("""
       #/
-      (?-xx:a b)
+      (?-x:a
+      b)
       /#
-      """, .cannotRemoveExtendedSyntaxInMultilineMode
+      """, .unsetExtendedSyntaxMayNotSpanMultipleLines
+    )
+    diagnosticWithDelimitersTest("""
+      #/
+      (?-xx:
+      a b)
+      /#
+      """, .unsetExtendedSyntaxMayNotSpanMultipleLines
+    )
+    diagnosticWithDelimitersTest("""
+      #/
+      (?x-x:
+      a b)
+      /#
+      """, .unsetExtendedSyntaxMayNotSpanMultipleLines
     )
 
     diagnosticWithDelimitersTest(#"""