diff --git a/CHANGELOG.md b/CHANGELOG.md index 18b30fc..f202691 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable changes are documented here. The format follows [Keep a Changelog](h - **A user `onCancel` handler that started new work during a sealed model's teardown drain was silently dropped.** `AnyContext.onRemoval` seals a model's `Cancellations` store, then drains it synchronously via `cancelAll()`/`cancelAll(for:)`. If one of the drained `onCancel` closures itself called `node.task { }`, a nested `node.onCancel { }`, or `forEach`, that registration landed on the already-sealed store: `Cancellations.register` cancelled it immediately before its body ever ran, with no signal — indistinguishable from the expected-silent case of a registration racing teardown from a different thread. `Cancellations.register` now checks a thread-local, `threadLocals.isDrainingCancellations`, set only around the synchronous drain in `cancelAll()`/`cancelAll(for:)`, and calls `reportIssue` (outside the lock) when a same-thread registration lands on a store sealed by the drain it's running inside of. A cross-thread race against teardown still stays silent, as intended — including a `Task { }` spawned from an `onCancel` handler that registers later: a thread-local, unlike a `@TaskLocal`, isn't inherited by that task. `OnCancelDuringTeardownRegistrationTests` is the regression coverage. +- **SwiftModel's `==` overloads that return `TestPredicate` no longer hijack plain comparisons in app code.** The two public `==` operators behind `expect { a == b }` (which let a failure print both sides) were visible to every file that imports SwiftModel, and outside a builder they could win: `let immediate = self.outcome == .needsNewPlayer` in non-test code inferred `TestPredicate` rather than `Bool`, and needed an explicit `: Bool`. Both operators are now `@_disfavoredOverload`, so an untyped comparison infers `Bool` again. Disfavoring them alone would have cost `expect { }` its diffs: the builder's `Bool` `buildExpression` was already disfavored, so each path carried one disfavored declaration and the solver broke the tie toward the concrete `==` (`expect { model.outcome == .needsNewPlayer }` degraded from the `− .needsNewPlayer / + .pending` diff to a bare "Expectation not met"). `AssertBuilder.buildExpression(Bool)` is therefore removed. A plain `Bool` now reaches the existing `buildExpression(Bool?)` through a value-to-optional conversion, and that extra cost keeps the tie going to `TestPredicate`. Builder failure output for optional and enum implicit-member comparisons is byte-identical to before on Swift 6.3 and 6.4. Two new `builder … == …` snapshots in `OutputSnapshotTests` now pin it, where before only an explicitly `TestPredicate`-annotated predicate was covered. Comparisons like `expect { model.count == 99 }`, where a concrete `==` exists, now also show the diff on Swift 6.3, where they previously printed just `count == 3`. On 6.4 they print what they did before. `TestPredicateOverloadTests` (plain `import SwiftModel`) is the regression test for inference outside a builder, and it fails to compile without the fix. + --- ## [1.0.21] — Write-lock holder resolves through read probes (AB-BA deadlock fix) diff --git a/Sources/SwiftModel/Testing/ModelTester.swift b/Sources/SwiftModel/Testing/ModelTester.swift index a9abffc..2aee6be 100644 --- a/Sources/SwiftModel/Testing/ModelTester.swift +++ b/Sources/SwiftModel/Testing/ModelTester.swift @@ -386,11 +386,14 @@ public extension AssertBuilder { layers.flatMap { $0 } } - @_disfavoredOverload - static func buildExpression(_ predicate: @autoclosure @escaping @Sendable () -> Bool, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column) -> Result { - [Predicate(predicate: predicate, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column))] - } - + // Overload resolution inside the builder: the `==` overloads below that return + // `TestPredicate` are `@_disfavoredOverload` (so plain code outside a builder infers `Bool`), + // and so is this one. That leaves the `TestPredicate` path (rich lhs/rhs failure diff) and + // the plain `Bool ==` path tied on disfavored overloads, and the tie must go to + // `TestPredicate`. There is deliberately no `buildExpression(Bool)`: a plain `Bool` reaches + // this `Bool?` overload through a value-to-optional conversion, and that extra cost is what + // breaks the tie. Adding a `Bool` overload back makes `expect { model.outcome == .x }` lose + // its diff (pinned by the `builder … == …` snapshots in `OutputSnapshotTests`). @_disfavoredOverload static func buildExpression(_ predicate: @autoclosure @escaping @Sendable () -> Bool?, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column) -> Result { [Predicate(predicate: { predicate() == true }, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column))] @@ -412,10 +415,14 @@ public struct TestPredicate: Sendable { var values: @Sendable () -> (Any, Any)? = { nil } } +// `==` returning `TestPredicate`, for `expect { a == b }` failures that show both sides. Disfavored +// so they never win outside the `AssertBuilder`: `let b = x == y` in app code must infer `Bool`. +@_disfavoredOverload public func == (lhs: @escaping @Sendable @autoclosure () -> T, rhs: @escaping @Sendable @autoclosure () -> T) -> TestPredicate { TestPredicate(predicate: { lhs() == rhs() }, values: { (lhs(), rhs()) }) } +@_disfavoredOverload public func == (lhs: @escaping @Sendable @autoclosure () -> T?, rhs: @escaping @Sendable @autoclosure () -> T) -> TestPredicate { TestPredicate(predicate: { lhs() == rhs() }, values: { (lhs() as Any, rhs() as Any) }) } diff --git a/Tests/SwiftModelSnapshotTests/OutputSnapshotTests.swift b/Tests/SwiftModelSnapshotTests/OutputSnapshotTests.swift index 71551f2..b644471 100644 --- a/Tests/SwiftModelSnapshotTests/OutputSnapshotTests.swift +++ b/Tests/SwiftModelSnapshotTests/OutputSnapshotTests.swift @@ -23,6 +23,23 @@ private struct SimpleCounter { var count: Int = 0 } +// A model with an optional property, for the `T? == T` builder overload. +@Model +private struct OptionalCounter { + var count: Int? = nil +} + +private enum Outcome: Equatable, Sendable { + case pending + case needsNewPlayer +} + +// A model with an enum property, for builder `==` against an implicit member. +@Model +private struct OutcomeHolder { + var outcome: Outcome = .pending +} + // A parent model that holds a child model, for testing how replacement diffs look @Model private struct ItemHolder { @@ -257,6 +274,57 @@ struct TesterAssertOutputTests { } } + // The `==` overloads returning `TestPredicate` are `@_disfavoredOverload` (so a plain + // `let b = x == y` outside a builder infers `Bool`), and so is the builder's `Bool?` + // `buildExpression`; the tie must still go to the `TestPredicate` path so both sides print. + // These pin that with no type annotation, for shapes that have no concrete `Bool ==` shortcut + // (optional vs. value, enum vs. implicit member — the latter is the shape that regressed when + // only the operators were disfavored). Shapes like `Int == literal` resolve toolchain- + // dependently (Swift 6.3 takes the `TestPredicate` path, 6.4 the concrete `Int.==`), so they + // are deliberately not snapshotted here. + @Test("builder optional == predicate failure shows lhs/rhs diff") + func builderOptionalEqualityFailureMessage() async { + await TestAccessOverrides.$hardCapNanoseconds.withValue(50_000_000) { + await assertIssueSnapshot { + await withModelTesting(exhaustivity: .off) { + let model = OptionalCounter().withAnchor() + model.count = 3 + await expect { model.count == 99 } + } + } matches: { + """ + Expectation not met: OptionalCounter.count: … + + − 99 + + 3 + + (Expected: −, Actual: +) + """ + } + } + } + + @Test("builder enum == implicit member failure shows lhs/rhs diff") + func builderEnumEqualityFailureMessage() async { + await TestAccessOverrides.$hardCapNanoseconds.withValue(50_000_000) { + await assertIssueSnapshot { + await withModelTesting(exhaustivity: .off) { + let model = OutcomeHolder().withAnchor() + await expect { model.outcome == .needsNewPlayer } + } + } matches: { + """ + Expectation not met: OutcomeHolder.outcome: … + + − Outcome.needsNewPlayer + + Outcome.pending + + (Expected: −, Actual: +) + """ + } + } + } + @Test("passing assertion emits no issues") func passingAssertionNoIssues() async { await assertIssueSnapshot { diff --git a/Tests/SwiftModelTests/TestPredicateOverloadTests.swift b/Tests/SwiftModelTests/TestPredicateOverloadTests.swift new file mode 100644 index 0000000..8d23cb0 --- /dev/null +++ b/Tests/SwiftModelTests/TestPredicateOverloadTests.swift @@ -0,0 +1,53 @@ +import Testing +import SwiftModel // plain import, like a downstream app: sees only the public `==` overloads + +// SwiftModel's public `==` overloads returning `TestPredicate` exist for the `expect { a == b }` +// builder, so failures can print both sides. They're `@_disfavoredOverload`, so ordinary code +// outside a builder must keep inferring `Bool`. (The builder side is pinned by the +// `builder … == …` snapshots in `SwiftModelSnapshotTests/OutputSnapshotTests.swift`.) + +private enum Outcome: Equatable, Sendable { + case pending + case needsNewPlayer +} + +private struct Wrapper: Equatable, Sendable { + var value: Int +} + +/// Only accepts `Bool`: passing an untyped `let` here fails to compile if it inferred `TestPredicate`. +private func requireBool(_ value: Bool) -> Bool { value } + +private func genericEquals(_ lhs: T, _ rhs: T) -> Bool { + let result = lhs == rhs + return requireBool(result) +} + +struct TestPredicateOverloadTests { + @Test func equalityOutsideBuilderInfersBool() { + let outcome = Outcome.pending + let immediate = outcome == .needsNewPlayer + #expect(type(of: immediate) == Bool.self) + #expect(requireBool(immediate) == false) + } + + @Test func optionalEqualityOutsideBuilderInfersBool() { + let count: Int? = 3 + let matches = count == 3 + #expect(type(of: matches) == Bool.self) + #expect(requireBool(matches)) + } + + @Test func structEqualityOutsideBuilderInfersBool() { + let a = Wrapper(value: 1) + let b = Wrapper(value: 2) + let same = a == b + #expect(type(of: same) == Bool.self) + #expect(requireBool(same) == false) + } + + @Test func genericEqualityOutsideBuilderInfersBool() { + #expect(genericEquals(Outcome.needsNewPlayer, .needsNewPlayer)) + #expect(genericEquals(Wrapper(value: 1), Wrapper(value: 2)) == false) + } +}