From 158fbbbf7a12f234cdcf9c636eb57ea04446465d Mon Sep 17 00:00:00 2001 From: Rachel Brindle Date: Sat, 29 Jul 2023 16:53:26 -0700 Subject: [PATCH] Make AsyncPredicate Sendable and operate only on Sendable types Make Predicate's closure Sendable, and make Predicate Sendable when the returning value is Sendable --- Sources/Nimble/ExpectationMessage.swift | 4 +- Sources/Nimble/Matchers/AsyncPredicate.swift | 18 ++--- .../Nimble/Matchers/PostNotification.swift | 29 +++++++-- Sources/Nimble/Matchers/Predicate.swift | 30 +++++---- Tests/NimbleTests/SynchronousTest.swift | 65 +++++++++++++------ 5 files changed, 96 insertions(+), 50 deletions(-) diff --git a/Sources/Nimble/ExpectationMessage.swift b/Sources/Nimble/ExpectationMessage.swift index 4efda7c01..2bff900e4 100644 --- a/Sources/Nimble/ExpectationMessage.swift +++ b/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 , got ") 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) { diff --git a/Sources/Nimble/Matchers/AsyncPredicate.swift b/Sources/Nimble/Matchers/AsyncPredicate.swift index 7f5973257..28158ac9e 100644 --- a/Sources/Nimble/Matchers/AsyncPredicate.swift +++ b/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: AsyncablePredicate { - fileprivate var matcher: (AsyncExpression) async throws -> PredicateResult +public struct AsyncPredicate: AsyncablePredicate, Sendable { + fileprivate var matcher: @Sendable (AsyncExpression) async throws -> PredicateResult - public init(_ matcher: @escaping (AsyncExpression) async throws -> PredicateResult) { + public init(_ matcher: @escaping @Sendable (AsyncExpression) async throws -> PredicateResult) { self.matcher = matcher } @@ -48,7 +48,7 @@ public struct AsyncPredicate: 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) async throws -> PredicateResult) -> AsyncPredicate { + public static func define(matcher: @escaping @Sendable (AsyncExpression) async throws -> PredicateResult) -> AsyncPredicate { return AsyncPredicate { actual in return try await matcher(actual) }.requireNonNil @@ -56,7 +56,7 @@ extension AsyncPredicate { /// 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, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate { + public static func define(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate { return AsyncPredicate { actual in return try await matcher(actual, .expectedActualValueTo(message)) }.requireNonNil @@ -64,7 +64,7 @@ extension AsyncPredicate { /// 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, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate { + public static func defineNilable(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression, ExpectationMessage) async throws -> PredicateResult) -> AsyncPredicate { return AsyncPredicate { 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) async throws -> PredicateStatus) -> AsyncPredicate { + public static func simple(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression) async throws -> PredicateStatus) -> AsyncPredicate { return AsyncPredicate { 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) async throws -> PredicateStatus) -> AsyncPredicate { + public static func simpleNilable(_ message: String = "match", matcher: @escaping @Sendable (AsyncExpression) async throws -> PredicateStatus) -> AsyncPredicate { return AsyncPredicate { 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, PredicateResult) async throws -> PredicateResult) -> AsyncPredicate { + internal func after(f: @escaping @Sendable (AsyncExpression, PredicateResult) async throws -> PredicateResult) -> AsyncPredicate { // swiftlint:disable:previous identifier_name return AsyncPredicate { actual -> PredicateResult in let result = try await self.satisfies(actual) diff --git a/Sources/Nimble/Matchers/PostNotification.swift b/Sources/Nimble/Matchers/PostNotification.swift index 6f604ae8c..618a5f795 100644 --- a/Sources/Nimble/Matchers/PostNotification.swift +++ b/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( _ predicate: Predicate<[Notification]>, from center: NotificationCenter, names: Set = [] ) -> Predicate { - _ = 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( ) assert(Thread.isMainThread, "Only expecting closure to be evaluated on main thread.") - if !once { - once = true + try once.runOnlyOnce { _ = try actualExpression.evaluate() } diff --git a/Sources/Nimble/Matchers/Predicate.swift b/Sources/Nimble/Matchers/Predicate.swift index ce20c6b1c..1d800be3a 100644 --- a/Sources/Nimble/Matchers/Predicate.swift +++ b/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 { - fileprivate var matcher: (Expression) throws -> PredicateResult + fileprivate let matcher: @Sendable (Expression) throws -> PredicateResult /// Constructs a predicate that knows how take a given value - public init(_ matcher: @escaping (Expression) throws -> PredicateResult) { + public init(_ matcher: @escaping @Sendable (Expression) throws -> PredicateResult) { self.matcher = matcher } @@ -33,10 +33,12 @@ public struct Predicate { } } +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) throws -> PredicateResult) -> Predicate { + public static func define(matcher: @escaping @Sendable (Expression) throws -> PredicateResult) -> Predicate { return Predicate { actual in return try matcher(actual) }.requireNonNil @@ -44,7 +46,7 @@ extension Predicate { /// 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, ExpectationMessage) throws -> PredicateResult) -> Predicate { + public static func define(_ message: String = "match", matcher: @escaping @Sendable (Expression, ExpectationMessage) throws -> PredicateResult) -> Predicate { return Predicate { actual in return try matcher(actual, .expectedActualValueTo(message)) }.requireNonNil @@ -52,7 +54,7 @@ extension Predicate { /// 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, ExpectationMessage) throws -> PredicateResult) -> Predicate { + public static func defineNilable(_ message: String = "match", matcher: @escaping @Sendable (Expression, ExpectationMessage) throws -> PredicateResult) -> Predicate { return Predicate { 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) throws -> PredicateStatus) -> Predicate { + public static func simple(_ message: String = "match", matcher: @escaping @Sendable (Expression) throws -> PredicateStatus) -> Predicate { return Predicate { actual in return PredicateResult(status: try matcher(actual), message: .expectedActualValueTo(message)) }.requireNonNil @@ -74,7 +76,7 @@ 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) throws -> PredicateStatus) -> Predicate { + public static func simpleNilable(_ message: String = "match", matcher: @escaping @Sendable (Expression) throws -> PredicateStatus) -> Predicate { return Predicate { actual in return PredicateResult(status: try matcher(actual), message: .expectedActualValueTo(message)) } @@ -82,13 +84,13 @@ extension Predicate { } // 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, PredicateResult) throws -> PredicateResult) -> Predicate { + internal func after(f: @escaping @Sendable (Expression, PredicateResult) throws -> PredicateResult) -> Predicate { // 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) throws -> NMBPredicateResult +public typealias PredicateBlock = @Sendable (_ actualExpression: Expression) 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 diff --git a/Tests/NimbleTests/SynchronousTest.swift b/Tests/NimbleTests/SynchronousTest.swift index 4c5eeac74..399f65a1f 100644 --- a/Tests/NimbleTests/SynchronousTest.swift +++ b/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() + 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() + 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() + 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() + 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() + 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() + 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: @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() + } +}