Fix algorithms overload resolution issues

This change addresses two overload resolution problems with the
collection-based algorithm methods. First, when RegexBuilder is
imported, `String` gains `RegexComponent` conformance, which means
the `RegexComponent`-based overloads win for strings, which is
undesirable. Second, if a collection has an element type that can
be expressed as an array literal, collection-based methods get
selected ahead of any standard library counterpart. These two problems
combine in a tricky way for `split` and `contains`.

For `split`, both the collection-based and regex-based versions need
to be marked as `@_disfavoredOverload` so that the problems above can
be resolved. Unfortunately, this sets up an ambiguity once `String`
has `RegexComponent` conformance, so the `RegexBuilder` module
includes separate overloads for `String` and `Substring` that act
as tie-breakers. If introduced in the standard library, these would
be a source-breaking change, as they would win over the `Element`-
based split when referencing the `split` method, as with
`let splitFunction = myString.split`.

For `contains`, the same requirements hold, with the added
complication that the Foundation overlay defines its own
`String.contains(_:)` method with different behavior than included
in these additions. For this reason, the more specific overloads for
`String` and `Substring` can't live in the `RegexBuilder` module,
which creates a problem for source compatibility. As it stands now,
this existing code does not compile with the new algorithm methods
added, as the type of `vowelPredicate` changes from `(Character) ->
Bool` to `(String) -> Bool`:

```
let str = "abcde"
let vowelPredicate = "aeiou".contains
print(str.filter(vowelPredicate))
```

Author: natecook1000

Sources/RegexBuilder/Algorithms.swift

@@ -9,7 +9,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import _StringProcessing
+@_spi(RegexBuilder) import _StringProcessing
 
 // FIXME(rdar://92459215): We should be using 'some RegexComponent' instead of
 // <R: RegexComponent> for the methods below that don't impose any additional
@@ -313,3 +313,31 @@ where Self: BidirectionalCollection, SubSequence == Substring {
     try replace(content(), maxReplacements: maxReplacements, with: replacement)
   }
 }
+
+// String split overload breakers
+
+extension StringProtocol where SubSequence == Substring {
+  @available(SwiftStdlib 5.7, *)
+  public func split(
+    separator: String,
+    maxSplits: Int = .max,
+    omittingEmptySubsequences: Bool = true
+  ) -> [Substring] {
+    return _split(
+      separator: separator,
+      maxSplits: maxSplits,
+      omittingEmptySubsequences: omittingEmptySubsequences)
+  }
+  
+  @available(SwiftStdlib 5.7, *)
+  public func split(
+    separator: Substring,
+    maxSplits: Int = .max,
+    omittingEmptySubsequences: Bool = true
+  ) -> [Substring] {
+    return _split(
+      separator: separator,
+      maxSplits: maxSplits,
+      omittingEmptySubsequences: omittingEmptySubsequences)
+  }
+}

Sources/_StringProcessing/Algorithms/Algorithms/Contains.swift

@@ -15,7 +15,7 @@ extension Collection {
   func contains<Searcher: CollectionSearcher>(
     _ searcher: Searcher
   ) -> Bool where Searcher.Searched == Self {
-    firstRange(of: searcher) != nil
+    _firstRange(of: searcher) != nil
   }
 }
 
@@ -27,6 +27,7 @@ extension Collection where Element: Equatable {
   /// - Parameter other: A sequence to search for within this collection.
   /// - Returns: `true` if the collection contains the specified sequence,
   /// otherwise `false`.
+  @_disfavoredOverload
   @available(SwiftStdlib 5.7, *)
   public func contains<C: Collection>(_ other: C) -> Bool
     where C.Element == Element
@@ -68,6 +69,7 @@ extension BidirectionalCollection where SubSequence == Substring {
   /// - Parameter regex: A regex to search for within this collection.
   /// - Returns: `true` if the regex was found in the collection, otherwise
   /// `false`.
+  @_disfavoredOverload
   @available(SwiftStdlib 5.7, *)
   public func contains<R: RegexComponent>(_ regex: R) -> Bool {
     contains(RegexConsumer(regex))

Sources/_StringProcessing/Algorithms/Algorithms/FirstRange.swift

@@ -12,7 +12,7 @@
 // MARK: `CollectionSearcher` algorithms
 
 extension Collection {
-  func firstRange<S: CollectionSearcher>(
+  func _firstRange<S: CollectionSearcher>(
     of searcher: S
   ) -> Range<Index>? where S.Searched == Self {
     var state = searcher.state(for: self, in: startIndex..<endIndex)
@@ -75,9 +75,10 @@ extension BidirectionalCollection where SubSequence == Substring {
   /// - Parameter regex: The regex to search for.
   /// - Returns: A range in the collection of the first occurrence of `regex`.
   /// Returns `nil` if `regex` is not found.
+  @_disfavoredOverload
   @available(SwiftStdlib 5.7, *)
   public func firstRange<R: RegexComponent>(of regex: R) -> Range<Index>? {
-    firstRange(of: RegexConsumer(regex))
+    _firstRange(of: RegexConsumer(regex))
   }
 
   @available(SwiftStdlib 5.7, *)

Sources/_StringProcessing/Algorithms/Algorithms/Ranges.swift

@@ -157,7 +157,7 @@ extension ReversedRangesCollection: Sequence {
 // MARK: `CollectionSearcher` algorithms
 
 extension Collection {
-  func ranges<S: CollectionSearcher>(
+  func _ranges<S: CollectionSearcher>(
     of searcher: S
   ) -> RangesCollection<S> where S.Searched == Self {
     RangesCollection(base: self, searcher: searcher)
@@ -178,7 +178,7 @@ extension Collection where Element: Equatable {
   func ranges<C: Collection>(
     of other: C
   ) -> RangesCollection<ZSearcher<Self>> where C.Element == Element {
-    ranges(of: ZSearcher(pattern: Array(other), by: ==))
+    _ranges(of: ZSearcher(pattern: Array(other), by: ==))
   }
 
   // FIXME: Return `some Collection<Range<Index>>` for SE-0346
@@ -191,7 +191,7 @@ extension Collection where Element: Equatable {
   public func ranges<C: Collection>(
     of other: C
   ) -> [Range<Index>] where C.Element == Element {
-    ranges(of: ZSearcher(pattern: Array(other), by: ==)).map { $0 }
+    _ranges(of: ZSearcher(pattern: Array(other), by: ==)).map { $0 }
   }
 }
 
@@ -212,7 +212,7 @@ extension BidirectionalCollection where Element: Comparable {
   ) -> RangesCollection<PatternOrEmpty<TwoWaySearcher<Self>>>
     where C.Element == Element
   {
-    ranges(of: PatternOrEmpty(searcher: TwoWaySearcher(pattern: Array(other))))
+    _ranges(of: PatternOrEmpty(searcher: TwoWaySearcher(pattern: Array(other))))
   }
 
   // FIXME
@@ -231,10 +231,10 @@ extension BidirectionalCollection where Element: Comparable {
 extension BidirectionalCollection where SubSequence == Substring {
   @available(SwiftStdlib 5.7, *)
   @_disfavoredOverload
-  func ranges<R: RegexComponent>(
+  func _ranges<R: RegexComponent>(
     of regex: R
   ) -> RangesCollection<RegexConsumer<R, Self>> {
-    ranges(of: RegexConsumer(regex))
+    _ranges(of: RegexConsumer(regex))
   }
 
   @available(SwiftStdlib 5.7, *)
@@ -251,10 +251,11 @@ extension BidirectionalCollection where SubSequence == Substring {
   /// - Parameter regex: The regex to search for.
   /// - Returns: A collection or ranges in the receiver of all occurrences of
   /// `regex`. Returns an empty collection if `regex` is not found.
+  @_disfavoredOverload
   @available(SwiftStdlib 5.7, *)
   public func ranges<R: RegexComponent>(
     of regex: R
   ) -> [Range<Index>] {
-    Array(ranges(of: RegexConsumer(regex)))
+    Array(_ranges(of: RegexConsumer(regex)))
   }
 }

Sources/_StringProcessing/Algorithms/Algorithms/Replace.swift

@@ -26,7 +26,7 @@ extension RangeReplaceableCollection {
     var result = Self()
     result.append(contentsOf: self[..<index])
 
-    for range in self[subrange].ranges(of: searcher).prefix(maxReplacements) {
+    for range in self[subrange]._ranges(of: searcher).prefix(maxReplacements) {
       result.append(contentsOf: self[index..<range.lowerBound])
       result.append(contentsOf: replacement)
       index = range.upperBound

Sources/_StringProcessing/Algorithms/Algorithms/Split.swift

@@ -34,7 +34,7 @@ struct SplitCollection<Searcher: CollectionSearcher> {
     maxSplits: Int,
     omittingEmptySubsequences: Bool)
   {
-    self.ranges = base.ranges(of: searcher)
+    self.ranges = base._ranges(of: searcher)
     self.maxSplits = maxSplits
     self.omittingEmptySubsequences = omittingEmptySubsequences
   }
@@ -307,13 +307,17 @@ extension Collection where Element: Equatable {
   /// - Parameter separator: The element to be split upon.
   /// - Returns: A collection of subsequences, split from this collection's
   ///   elements.
+  @_disfavoredOverload
   @available(SwiftStdlib 5.7, *)
   public func split<C: Collection>(
     separator: C,
     maxSplits: Int = .max,
     omittingEmptySubsequences: Bool = true
   ) -> [SubSequence] where C.Element == Element {
-    Array(split(by: ZSearcher(pattern: Array(separator), by: ==), maxSplits: maxSplits, omittingEmptySubsequences: omittingEmptySubsequences))
+    Array(split(
+      by: ZSearcher(pattern: Array(separator), by: ==),
+      maxSplits: maxSplits,
+      omittingEmptySubsequences: omittingEmptySubsequences))
   }
 }
 
@@ -352,6 +356,45 @@ extension BidirectionalCollection where Element: Comparable {
 //  }
 }
 
+// String split overload breakers
+//
+// These are underscored and marked as SPI so that the *actual* public overloads
+// are only visible in RegexBuilder, to avoid breaking source with the
+// standard library's function of the same name that takes a `Character`
+// as the separator. *Those* overloads are necessary as tie-breakers between
+// the Collection-based and Regex-based `split`s, which in turn are both marked
+// @_disfavoredOverload to avoid the wrong overload being selected when a
+// collection's element type could be used interchangably with a collection of
+// that element (e.g. `Array<OptionSet>.split(separator: [])`).
+
+extension StringProtocol where SubSequence == Substring {
+  @_spi(RegexBuilder)
+  @available(SwiftStdlib 5.7, *)
+  public func _split(
+    separator: String,
+    maxSplits: Int = .max,
+    omittingEmptySubsequences: Bool = true
+  ) -> [Substring] {
+    Array(split(
+      by: ZSearcher(pattern: Array(separator), by: ==),
+      maxSplits: maxSplits,
+      omittingEmptySubsequences: omittingEmptySubsequences))
+  }
+  
+  @_spi(RegexBuilder)
+  @available(SwiftStdlib 5.7, *)
+  public func _split(
+    separator: Substring,
+    maxSplits: Int = .max,
+    omittingEmptySubsequences: Bool = true
+  ) -> [Substring] {
+    Array(split(
+      by: ZSearcher(pattern: Array(separator), by: ==),
+      maxSplits: maxSplits,
+      omittingEmptySubsequences: omittingEmptySubsequences))
+  }
+}
+
 // MARK: Regex algorithms
 
 @available(SwiftStdlib 5.7, *)
@@ -388,6 +431,9 @@ extension BidirectionalCollection where SubSequence == Substring {
     maxSplits: Int = .max,
     omittingEmptySubsequences: Bool = true
   ) -> [SubSequence] {
-    Array(split(by: RegexConsumer(separator), maxSplits: maxSplits, omittingEmptySubsequences: omittingEmptySubsequences))
+    Array(split(
+      by: RegexConsumer(separator),
+      maxSplits: maxSplits,
+      omittingEmptySubsequences: omittingEmptySubsequences))
   }
 }

Sources/_StringProcessing/Algorithms/Algorithms/Trim.swift

@@ -216,7 +216,6 @@ extension Collection where SubSequence == Self, Element: Equatable {
 }
 
 extension RangeReplaceableCollection where Element: Equatable {
-  @_disfavoredOverload
   /// Removes the initial elements that satisfy the given predicate from the
   /// start of the sequence.
   /// - Parameter predicate: A closure that takes an element of the sequence
@@ -290,6 +289,7 @@ extension BidirectionalCollection where SubSequence == Substring {
   /// - Parameter prefix: The collection to remove from this collection.
   /// - Returns: A collection containing the elements that does not match
   /// `prefix` from the start.
+  @_disfavoredOverload
   @available(SwiftStdlib 5.7, *)
   public func trimmingPrefix<R: RegexComponent>(_ regex: R) -> SubSequence {
     trimmingPrefix(RegexConsumer(regex))

Tests/RegexTests/AlgorithmsTests.swift

@@ -11,6 +11,7 @@
 
 import _StringProcessing
 import XCTest
+//import RegexBuilder
 
 // TODO: Protocol-powered testing
 class RegexConsumerTests: XCTestCase {
@@ -89,6 +90,17 @@ class AlgorithmTests: XCTestCase {
     // `Array<CountedOptionSet>`.
     XCTAssertFalse(cosArray.contains([]))
     XCTAssertEqual(CountedOptionSet.arrayLiteralCreationCount, 3)
+
+    // For these references to resolve to the `Element`-based stdlib function,
+    // the `String`- and `Substring`-based `contains` functions need to be
+    // marked as `@_disfavoredOverload`. However, that means that Foundation's
+    // `String.contains` get selected instead, which has inconsistent behavior.
+    
+    // Test that original `contains` functions are still accessible
+    // let containsRef = "abcd".contains
+    // XCTAssert(type(of: containsRef) == ((Character) -> Bool).self)
+    // let containsParamsRef = "abcd".contains(_:)
+    // XCTAssert(type(of: containsParamsRef) == ((Character) -> Bool).self)
   }
 
   func testRegexRanges() {
@@ -201,12 +213,6 @@ class AlgorithmTests: XCTestCase {
 
     XCTAssertEqual("".split(separator: ""), [])
     XCTAssertEqual("".split(separator: "", omittingEmptySubsequences: false), [""])
-
-    // Test that original `split` functions are still accessible
-    let splitRef = "abcd".split
-    XCTAssert(type(of: splitRef) == ((Character, Int, Bool) -> [Substring]).self)
-    let splitParamsRef = "abcd".split(separator:maxSplits:omittingEmptySubsequences:)
-    XCTAssert(type(of: splitParamsRef) == ((Character, Int, Bool) -> [Substring]).self)
   }
 
   func testSplitSourceCompatibility() {
@@ -225,6 +231,12 @@ class AlgorithmTests: XCTestCase {
     // `Array<CountedOptionSet>`.
     XCTAssertEqual(cosArray.split(separator: []).count, 1)
     XCTAssertEqual(CountedOptionSet.arrayLiteralCreationCount, 3)
+
+    // Test that original `split` functions are still accessible
+    let splitRef = "abcd".split
+    XCTAssert(type(of: splitRef) == ((Character, Int, Bool) -> [Substring]).self)
+    let splitParamsRef = "abcd".split(separator:maxSplits:omittingEmptySubsequences:)
+    XCTAssert(type(of: splitParamsRef) == ((Character, Int, Bool) -> [Substring]).self)
   }
 
   func testSplitPermutations() throws {