Make AsyncPredicate Sendable and operate only on Sendable types (#1072)

Make Predicate's closure Sendable, and make Predicate Sendable when the returning value is Sendable

Author: younata

Sources/Nimble/ExpectationMessage.swift

@@ -1,4 +1,4 @@
-public indirect enum ExpectationMessage {
+public indirect enum ExpectationMessage: Sendable {
     // --- Primary Expectations ---
     /// includes actual value in output ("expected to <message>, got <actual>")
     case expectedActualValueTo(/* message: */ String)
@@ -204,7 +204,7 @@ extension FailureMessage {
 #if canImport(Darwin)
 import class Foundation.NSObject
 
-public class NMBExpectationMessage: NSObject {
+public final class NMBExpectationMessage: NSObject, Sendable {
     private let msg: ExpectationMessage
 
     internal init(swift msg: ExpectationMessage) {

Sources/Nimble/Matchers/AsyncMatcher.swift

@@ -3,7 +3,7 @@ public protocol AsyncableMatcher<Value>: Sendable {
     func satisfies(_ expression: AsyncExpression<Value>) async throws -> MatcherResult
 }
 
-extension Matcher: AsyncableMatcher where T: Sendable {
+extension Matcher: AsyncableMatcher {
     public func satisfies(_ expression: AsyncExpression<T>) async throws -> MatcherResult {
         try satisfies(await expression.toSynchronousExpression())
     }

Sources/Nimble/Matchers/Matcher.swift

@@ -19,10 +19,10 @@
 /// renamed `NSMatcher` to `Matcher`. In response, we decided to rename `Matcher` to
 /// `Matcher`.
 public struct Matcher<T> {
-    fileprivate var matcher: (Expression<T>) throws -> MatcherResult
+    fileprivate let matcher: @Sendable (Expression<T>) throws -> MatcherResult
 
     /// Constructs a matcher that knows how take a given value
-    public init(_ matcher: @escaping (Expression<T>) throws -> MatcherResult) {
+    public init(_ matcher: @escaping @Sendable (Expression<T>) throws -> MatcherResult) {
         self.matcher = matcher
     }
 
@@ -39,26 +39,28 @@ public struct Matcher<T> {
 @available(*, deprecated, renamed: "Matcher")
 public typealias Predicate = Matcher
 
+extension Matcher: Sendable where T: Sendable {}
+
 /// Provides convenience helpers to defining matchers
 extension Matcher {
     /// Like Matcher() constructor, but automatically guard against nil (actual) values
-    public static func define(matcher: @escaping (Expression<T>) throws -> MatcherResult) -> Matcher<T> {
+    public static func define(matcher: @escaping @Sendable (Expression<T>) throws -> MatcherResult) -> Matcher<T> {
         return Matcher<T> { actual in
             return try matcher(actual)
         }.requireNonNil
     }
 
     /// Defines a matcher with a default message that can be returned in the closure
     /// Also ensures the matcher's actual value cannot pass with `nil` given.
-    public static func define(_ message: String = "match", matcher: @escaping (Expression<T>, ExpectationMessage) throws -> MatcherResult) -> Matcher<T> {
+    public static func define(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>, ExpectationMessage) throws -> MatcherResult) -> Matcher<T> {
         return Matcher<T> { actual in
             return try matcher(actual, .expectedActualValueTo(message))
         }.requireNonNil
     }
 
     /// Defines a matcher with a default message that can be returned in the closure
     /// Unlike `define`, this allows nil values to succeed if the given closure chooses to.
-    public static func defineNilable(_ message: String = "match", matcher: @escaping (Expression<T>, ExpectationMessage) throws -> MatcherResult) -> Matcher<T> {
+    public static func defineNilable(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>, ExpectationMessage) throws -> MatcherResult) -> Matcher<T> {
         return Matcher<T> { actual in
             return try matcher(actual, .expectedActualValueTo(message))
         }
@@ -70,7 +72,7 @@ extension Matcher {
     /// error message.
     ///
     /// Also ensures the matcher's actual value cannot pass with `nil` given.
-    public static func simple(_ message: String = "match", matcher: @escaping (Expression<T>) throws -> MatcherStatus) -> Matcher<T> {
+    public static func simple(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>) throws -> MatcherStatus) -> Matcher<T> {
         return Matcher<T> { actual in
             return MatcherResult(status: try matcher(actual), message: .expectedActualValueTo(message))
         }.requireNonNil
@@ -80,15 +82,15 @@ extension Matcher {
     /// error message.
     ///
     /// Unlike `simple`, this allows nil values to succeed if the given closure chooses to.
-    public static func simpleNilable(_ message: String = "match", matcher: @escaping (Expression<T>) throws -> MatcherStatus) -> Matcher<T> {
+    public static func simpleNilable(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>) throws -> MatcherStatus) -> Matcher<T> {
         return Matcher<T> { actual in
             return MatcherResult(status: try matcher(actual), message: .expectedActualValueTo(message))
         }
     }
 }
 
 /// The Expectation style intended for comparison to a MatcherStatus.
-public enum ExpectationStyle {
+public enum ExpectationStyle: Sendable {
     case toMatch, toNotMatch
 }
 
@@ -123,7 +125,7 @@ public struct MatcherResult {
 public typealias PredicateResult = MatcherResult
 
 /// MatcherStatus is a trinary that indicates if a Matcher matches a given value or not
-public enum MatcherStatus {
+public enum MatcherStatus: Sendable {
     /// Matches indicates if the matcher / matcher passes with the given value
     ///
     /// For example, `equals(1)` returns `.matches` for `expect(1).to(equal(1))`.
@@ -181,7 +183,7 @@ public typealias PredicateStatus = MatcherStatus
 
 extension Matcher {
     // Someday, make this public? Needs documentation
-    internal func after(f: @escaping (Expression<T>, MatcherResult) throws -> MatcherResult) -> Matcher<T> {
+    internal func after(f: @escaping @Sendable (Expression<T>, MatcherResult) throws -> MatcherResult) -> Matcher<T> {
         // swiftlint:disable:previous identifier_name
         return Matcher { actual -> MatcherResult in
             let result = try self.satisfies(actual)
@@ -207,7 +209,7 @@ extension Matcher {
 #if canImport(Darwin)
 import class Foundation.NSObject
 
-public typealias MatcherBlock = (_ actualExpression: Expression<NSObject>) throws -> NMBMatcherResult
+public typealias MatcherBlock = @Sendable (_ actualExpression: Expression<NSObject>) throws -> NMBMatcherResult
 
 /// Provides an easy upgrade path for custom Matchers to be renamed to Matchers
 @available(*, deprecated, renamed: "MatcherBlock")
@@ -225,7 +227,7 @@ public class NMBMatcher: NSObject {
         self.init(matcher: predicate)
     }
 
-    func satisfies(_ expression: @escaping () throws -> NSObject?, location: SourceLocation) -> NMBMatcherResult {
+    func satisfies(_ expression: @escaping @Sendable () throws -> NSObject?, location: SourceLocation) -> NMBMatcherResult {
         let expr = Expression(expression: expression, location: location)
         do {
             return try self.matcher(expr)
@@ -269,7 +271,7 @@ extension MatcherResult {
     }
 }
 
-final public class NMBMatcherStatus: NSObject {
+final public class NMBMatcherStatus: NSObject, Sendable {
     private let status: Int
     private init(status: Int) {
         self.status = status

Sources/Nimble/Matchers/PostNotification.swift

@@ -49,6 +49,22 @@ private let mainThread = pthread_self()
 private let mainThread = Thread.mainThread
 #endif
 
+private final class OnlyOnceChecker: @unchecked Sendable {
+    var hasRun = false
+    let lock = NSRecursiveLock()
+
+    func runOnlyOnce(_ closure: @Sendable () throws -> Void) rethrows {
+        lock.lock()
+        defer {
+            lock.unlock()
+        }
+        if !hasRun {
+            hasRun = true
+            try closure()
+        }
+    }
+}
+
 private func _postNotifications<Out>(
     _ matcher: Matcher<[Notification]>,
     from center: NotificationCenter,
@@ -57,9 +73,16 @@ private func _postNotifications<Out>(
     _ = mainThread // Force lazy-loading of this value
     let collector = NotificationCollector(notificationCenter: center, names: names)
     collector.startObserving()
-    var once: Bool = false
+    let once = OnlyOnceChecker()
 
     return Matcher { actualExpression in
+        guard Thread.isMainThread else {
+            let message = ExpectationMessage
+                .expectedTo("post notifications - but was called off the main thread.")
+                .appended(details: "postNotifications and postDistributedNotifications attempted to run their predicate off the main thread. This is a bug in Nimble.")
+            return PredicateResult(status: .fail, message: message)
+        }
+
         let collectorNotificationsExpression = Expression(
             memoizedExpression: { _ in
                 return collector.observedNotifications
@@ -69,8 +92,7 @@ private func _postNotifications<Out>(
         )
 
         assert(Thread.isMainThread, "Only expecting closure to be evaluated on main thread.")
-        if !once {
-            once = true
+        try once.runOnlyOnce {
             _ = try actualExpression.evaluate()
         }
 

Tests/NimbleTests/SynchronousTest.swift

@@ -37,29 +37,33 @@ final class SynchronousTest: XCTestCase {
     }
 
     func testToProvidesActualValueExpression() {
-        var value: Int?
-        expect(1).to(Matcher.simple { expr in value = try expr.evaluate(); return .matches })
-        expect(value).to(equal(1))
+        let recorder = Recorder<Int?>()
+        expect(1).to(Matcher.simple { expr in recorder.record(try expr.evaluate()); return .matches })
+        expect(recorder.records).to(equal([1]))
     }
 
     func testToProvidesAMemoizedActualValueExpression() {
-        var callCount = 0
-        expect { callCount += 1 }.to(Matcher.simple { expr in
+        let recorder = Recorder<Void>()
+        expect {
+            recorder.record(())
+        }.to(Matcher.simple { expr in
             _ = try expr.evaluate()
             _ = try expr.evaluate()
             return .matches
         })
-        expect(callCount).to(equal(1))
+        expect(recorder.records).to(haveCount(1))
     }
 
     func testToProvidesAMemoizedActualValueExpressionIsEvaluatedAtMatcherControl() {
-        var callCount = 0
-        expect { callCount += 1 }.to(Matcher.simple { expr in
-            expect(callCount).to(equal(0))
+        let recorder = Recorder<Void>()
+        expect {
+            recorder.record(())
+        }.to(Matcher.simple { expr in
+            expect(recorder.records).to(beEmpty())
             _ = try expr.evaluate()
             return .matches
         })
-        expect(callCount).to(equal(1))
+        expect(recorder.records).to(haveCount(1))
     }
 
     func testToMatchAgainstLazyProperties() {
@@ -76,29 +80,29 @@ final class SynchronousTest: XCTestCase {
     }
 
     func testToNotProvidesActualValueExpression() {
-        var value: Int?
-        expect(1).toNot(Matcher.simple { expr in value = try expr.evaluate(); return .doesNotMatch })
-        expect(value).to(equal(1))
+        let recorder = Recorder<Int?>()
+        expect(1).toNot(Matcher.simple { expr in recorder.record(try expr.evaluate()); return .doesNotMatch })
+        expect(recorder.records).to(equal([1]))
     }
 
     func testToNotProvidesAMemoizedActualValueExpression() {
-        var callCount = 0
-        expect { callCount += 1 }.toNot(Matcher.simple { expr in
+        let recorder = Recorder<Void>()
+        expect { recorder.record(()) }.toNot(Matcher.simple { expr in
             _ = try expr.evaluate()
             _ = try expr.evaluate()
             return .doesNotMatch
         })
-        expect(callCount).to(equal(1))
+        expect(recorder.records).to(haveCount(1))
     }
 
     func testToNotProvidesAMemoizedActualValueExpressionIsEvaluatedAtMatcherControl() {
-        var callCount = 0
-        expect { callCount += 1 }.toNot(Matcher.simple { expr in
-            expect(callCount).to(equal(0))
+        let recorder = Recorder<Void>()
+        expect { recorder.record(()) }.toNot(Matcher.simple { expr in
+            expect(recorder.records).to(beEmpty())
             _ = try expr.evaluate()
             return .doesNotMatch
         })
-        expect(callCount).to(equal(1))
+        expect(recorder.records).to(haveCount(1))
     }
 
     func testToNegativeMatches() {
@@ -129,3 +133,24 @@ final class SynchronousTest: XCTestCase {
         }
     }
 }
+
+private final class Recorder<T: Sendable>: @unchecked Sendable {
+    private var _records: [T] = []
+    private let lock = NSRecursiveLock()
+
+    var records: [T] {
+        get {
+            lock.lock()
+            defer {
+                lock.unlock()
+            }
+            return _records
+        }
+    }
+
+    func record(_ value: T) {
+        lock.lock()
+        self._records.append(value)
+        lock.unlock()
+    }
+}