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/AsyncPredicate.swift

@@ -29,10 +29,10 @@ extension Predicate: AsyncablePredicate {
 /// These can also be used with either `Expectation`s or `AsyncExpectation`s.
 /// But these can only be used from async contexts, and are unavailable in Objective-C.
 /// You can, however, call regular Predicates from an AsyncPredicate, if you wish to compose one like that.
-public struct AsyncPredicate<T>: AsyncablePredicate {
-    fileprivate var matcher: (AsyncExpression<T>) async throws -> PredicateResult
+public struct AsyncPredicate<T: Sendable>: AsyncablePredicate, Sendable {
+    fileprivate var matcher: @Sendable (AsyncExpression<T>) async throws -> PredicateResult
 
-    public init(_ matcher: @escaping (AsyncExpression<T>) async throws -> PredicateResult) {
+    public init(_ matcher: @escaping @Sendable (AsyncExpression<T>) async throws -> PredicateResult) {
         self.matcher = matcher
     }
 
@@ -48,23 +48,23 @@ public struct AsyncPredicate<T>: AsyncablePredicate {
 /// Provides convenience helpers to defining predicates
 extension AsyncPredicate {
     /// Like Predicate() constructor, but automatically guard against nil (actual) values
-    public static func define(matcher: @escaping (AsyncExpression<T>) async throws -> PredicateResult) -> AsyncPredicate<T> {
+    public static func define(matcher: @escaping @Sendable (AsyncExpression<T>) async throws -> PredicateResult) -> AsyncPredicate<T> {
         return AsyncPredicate<T> { actual in
             return try await matcher(actual)
         }.requireNonNil
     }
 
     /// Defines a predicate with a default message that can be returned in the closure
     /// Also ensures the predicate's actual value cannot pass with `nil` given.
-    public static func define(_ message: String = "match", matcher: @escaping (AsyncExpression<T>, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate<T> {
+    public static func define(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression<T>, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate<T> {
         return AsyncPredicate<T> { actual in
             return try await matcher(actual, .expectedActualValueTo(message))
         }.requireNonNil
     }
 
     /// Defines a predicate 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 (AsyncExpression<T>, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate<T> {
+    public static func defineNilable(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression<T>, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate<T> {
         return AsyncPredicate<T> { actual in
             return try await matcher(actual, .expectedActualValueTo(message))
         }
@@ -74,7 +74,7 @@ extension AsyncPredicate {
     /// error message.
     ///
     /// Also ensures the predicate's actual value cannot pass with `nil` given.
-    public static func simple(_ message: String = "match", matcher: @escaping (AsyncExpression<T>) async throws -> PredicateStatus) -> AsyncPredicate<T> {
+    public static func simple(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression<T>) async throws -> PredicateStatus) -> AsyncPredicate<T> {
         return AsyncPredicate<T> { actual in
             return PredicateResult(status: try await matcher(actual), message: .expectedActualValueTo(message))
         }.requireNonNil
@@ -84,7 +84,7 @@ extension AsyncPredicate {
     /// 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 (AsyncExpression<T>) async throws -> PredicateStatus) -> AsyncPredicate<T> {
+    public static func simpleNilable(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression<T>) async throws -> PredicateStatus) -> AsyncPredicate<T> {
         return AsyncPredicate<T> { actual in
             return PredicateResult(status: try await matcher(actual), message: .expectedActualValueTo(message))
         }
@@ -93,7 +93,7 @@ extension AsyncPredicate {
 
 extension AsyncPredicate {
     // Someday, make this public? Needs documentation
-    internal func after(f: @escaping (AsyncExpression<T>, PredicateResult) async throws -> PredicateResult) -> AsyncPredicate<T> {
+    internal func after(f: @escaping @Sendable (AsyncExpression<T>, PredicateResult) async throws -> PredicateResult) -> AsyncPredicate<T> {
         // swiftlint:disable:previous identifier_name
         return AsyncPredicate { actual -> PredicateResult in
             let result = try await self.satisfies(actual)

Sources/Nimble/Matchers/PostNotification.swift

@@ -43,19 +43,39 @@ internal class NotificationCollector {
     }
 }
 
-private let mainThread = pthread_self()
+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>(
     _ predicate: Predicate<[Notification]>,
     from center: NotificationCenter,
     names: Set<Notification.Name> = []
 ) -> Predicate<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 Predicate { 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
@@ -65,8 +85,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()
         }
 

Sources/Nimble/Matchers/Predicate.swift

@@ -17,10 +17,10 @@
 /// predicates are simple wrappers around closures to provide static type information and
 /// allow composition and wrapping of existing behaviors.
 public struct Predicate<T> {
-    fileprivate var matcher: (Expression<T>) throws -> PredicateResult
+    fileprivate let matcher: @Sendable (Expression<T>) throws -> PredicateResult
 
     /// Constructs a predicate that knows how take a given value
-    public init(_ matcher: @escaping (Expression<T>) throws -> PredicateResult) {
+    public init(_ matcher: @escaping @Sendable (Expression<T>) throws -> PredicateResult) {
         self.matcher = matcher
     }
 
@@ -33,26 +33,28 @@ public struct Predicate<T> {
     }
 }
 
+extension Predicate: Sendable where T: Sendable {}
+
 /// Provides convenience helpers to defining predicates
 extension Predicate {
     /// Like Predicate() constructor, but automatically guard against nil (actual) values
-    public static func define(matcher: @escaping (Expression<T>) throws -> PredicateResult) -> Predicate<T> {
+    public static func define(matcher: @escaping @Sendable (Expression<T>) throws -> PredicateResult) -> Predicate<T> {
         return Predicate<T> { actual in
             return try matcher(actual)
         }.requireNonNil
     }
 
     /// Defines a predicate with a default message that can be returned in the closure
     /// Also ensures the predicate's actual value cannot pass with `nil` given.
-    public static func define(_ message: String = "match", matcher: @escaping (Expression<T>, ExpectationMessage) throws -> PredicateResult) -> Predicate<T> {
+    public static func define(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>, ExpectationMessage) throws -> PredicateResult) -> Predicate<T> {
         return Predicate<T> { actual in
             return try matcher(actual, .expectedActualValueTo(message))
         }.requireNonNil
     }
 
     /// Defines a predicate 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 -> PredicateResult) -> Predicate<T> {
+    public static func defineNilable(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>, ExpectationMessage) throws -> PredicateResult) -> Predicate<T> {
         return Predicate<T> { actual in
             return try matcher(actual, .expectedActualValueTo(message))
         }
@@ -64,7 +66,7 @@ extension Predicate {
     /// error message.
     ///
     /// Also ensures the predicate's actual value cannot pass with `nil` given.
-    public static func simple(_ message: String = "match", matcher: @escaping (Expression<T>) throws -> PredicateStatus) -> Predicate<T> {
+    public static func simple(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>) throws -> PredicateStatus) -> Predicate<T> {
         return Predicate<T> { actual in
             return PredicateResult(status: try matcher(actual), message: .expectedActualValueTo(message))
         }.requireNonNil
@@ -74,21 +76,21 @@ extension Predicate {
     /// 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 -> PredicateStatus) -> Predicate<T> {
+    public static func simpleNilable(_ message: String = "match", matcher: @escaping @Sendable (Expression<T>) throws -> PredicateStatus) -> Predicate<T> {
         return Predicate<T> { actual in
             return PredicateResult(status: try matcher(actual), message: .expectedActualValueTo(message))
         }
     }
 }
 
 // The Expectation style intended for comparison to a PredicateStatus.
-public enum ExpectationStyle {
+public enum ExpectationStyle: Sendable {
     case toMatch, toNotMatch
 }
 
 /// The value that a Predicates return to describe if the given (actual) value matches the
 /// predicate.
-public struct PredicateResult {
+public struct PredicateResult: Sendable {
     /// Status indicates if the predicate matches, does not match, or fails.
     public var status: PredicateStatus
     /// The error message that can be displayed if it does not match
@@ -113,7 +115,7 @@ public struct PredicateResult {
 }
 
 /// PredicateStatus is a trinary that indicates if a Predicate matches a given value or not
-public enum PredicateStatus {
+public enum PredicateStatus: Sendable {
     /// Matches indicates if the predicate / matcher passes with the given value
     ///
     /// For example, `equals(1)` returns `.matches` for `expect(1).to(equal(1))`.
@@ -167,7 +169,7 @@ public enum PredicateStatus {
 
 extension Predicate {
     // Someday, make this public? Needs documentation
-    internal func after(f: @escaping (Expression<T>, PredicateResult) throws -> PredicateResult) -> Predicate<T> {
+    internal func after(f: @escaping @Sendable (Expression<T>, PredicateResult) throws -> PredicateResult) -> Predicate<T> {
         // swiftlint:disable:previous identifier_name
         return Predicate { actual -> PredicateResult in
             let result = try self.satisfies(actual)
@@ -193,7 +195,7 @@ extension Predicate {
 #if canImport(Darwin)
 import class Foundation.NSObject
 
-public typealias PredicateBlock = (_ actualExpression: Expression<NSObject>) throws -> NMBPredicateResult
+public typealias PredicateBlock = @Sendable (_ actualExpression: Expression<NSObject>) throws -> NMBPredicateResult
 
 public class NMBPredicate: NSObject {
     private let predicate: PredicateBlock
@@ -202,7 +204,7 @@ public class NMBPredicate: NSObject {
         self.predicate = predicate
     }
 
-    func satisfies(_ expression: @escaping () throws -> NSObject?, location: SourceLocation) -> NMBPredicateResult {
+    func satisfies(_ expression: @escaping  @Sendable() throws -> NSObject?, location: SourceLocation) -> NMBPredicateResult {
         let expr = Expression(expression: expression, location: location)
         do {
             return try self.predicate(expr)
@@ -238,7 +240,7 @@ extension PredicateResult {
     }
 }
 
-final public class NMBPredicateStatus: NSObject {
+final public class NMBPredicateStatus: NSObject, Sendable {
     private let status: Int
     private init(status: Int) {
         self.status = status

Tests/NimbleTests/SynchronousTest.swift

@@ -37,29 +37,33 @@ final class SynchronousTest: XCTestCase {
     }
 
     func testToProvidesActualValueExpression() {
-        var value: Int?
-        expect(1).to(Predicate.simple { expr in value = try expr.evaluate(); return .matches })
-        expect(value).to(equal(1))
+        let recorder = Recorder<Int?>()
+        expect(1).to(Predicate.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(Predicate.simple { expr in
+        let recorder = Recorder<Void>()
+        expect {
+            recorder.record(())
+        }.to(Predicate.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(Predicate.simple { expr in
-            expect(callCount).to(equal(0))
+        let recorder = Recorder<Void>()
+        expect {
+            recorder.record(())
+        }.to(Predicate.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(Predicate.simple { expr in value = try expr.evaluate(); return .doesNotMatch })
-        expect(value).to(equal(1))
+        let recorder = Recorder<Int?>()
+        expect(1).toNot(Predicate.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(Predicate.simple { expr in
+        let recorder = Recorder<Void>()
+        expect { recorder.record(()) }.toNot(Predicate.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(Predicate.simple { expr in
-            expect(callCount).to(equal(0))
+        let recorder = Recorder<Void>()
+        expect { recorder.record(()) }.toNot(Predicate.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()
+    }
+}