From 392c527c5cf61b1706569de812b621f4124101fe Mon Sep 17 00:00:00 2001 From: Andreas Grosam Date: Wed, 10 Dec 2025 15:18:23 +0100 Subject: [PATCH] Improve observer lifecycle and KVO compliance - Ensure prefix property is KVO compliant by replacing dots with underscores - Fix observer lifecycle: cancel and recreate subscription when store changes - Add AnyObject constraint to UserDefaultsStore protocol for identity comparison - Refactor UserDefaultsStoreMock: make helper methods static --- .github/copilot-instructions.md | 92 +++++++++++++++++++ .../MacroSettings/UserDefaultsStore.swift | 2 +- Sources/Settings/SwiftUI/AppSetting.swift | 27 ++++-- .../UserDefaultsStoreEnvironment.swift | 19 +++- Sources/SettingsMock/UserDefaultsMock.swift | 16 ++-- 5 files changed, 139 insertions(+), 17 deletions(-) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..0ba117c --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,92 @@ +# Settings Framework - AI Coding Agent Instructions + +## Architecture Overview + +This is a **Swift macro-based framework** for type-safe UserDefaults access with SwiftUI integration. The codebase uses Swift 6.2 with strict concurrency checking. + +**Core Components:** +- **Macro Layer** (`Sources/SettingsMacros/`): `@Settings` and `@Setting` macros generate code at compile time +- **Runtime Layer** (`Sources/Settings/MacroSettings/`, `MacroSetting/`): Protocol infrastructure that macros generate code against +- **SwiftUI Integration** (`Sources/Settings/SwiftUI/`): `@AppSetting` property wrapper with environment-based store injection +- **Mock Layer** (`Sources/SettingsMock/`): `UserDefaultsStoreMock` for testing and previews + +**Key Design Decisions:** +- `__Settings_Container` protocol uses `any UserDefaultsStore` (existential types) instead of associated types to enable **dynamic store switching** at runtime (critical for SwiftUI mocking) +- Thread-safety via `OSAllocatedUnfairLock` instead of actor isolation to satisfy Swift 6 Sendable requirements for metatypes captured in Combine closures +- Double-underscore prefixed protocols (`__Settings_Container`, `__Attribute`) are internal implementation details generated by macros + +## Critical Workflows + +### Building & Testing +```bash +swift build --build-tests # Build all targets including tests +swift test # Run all 92 tests +swift test --filter SettingsMacroExpansionTests # Run specific test suite +``` + +### Release Process +1. Update `CHANGELOG.md` with new version entry +2. Commit changes locally +3. Push and create PR with label: `enhancement`, `breaking`, or `bugfix` +4. Squash-merge PR on GitHub +5. Tag release: `git tag X.Y.Z && git push --follow-tags` +6. Manually trigger `.github/workflows/release.yml` to create GitHub release + +## Project Conventions + +### Macro-Generated Code Pattern +```swift +// User writes: +@Settings(prefix: "app_") +struct AppSettings { + @Setting static var username: String = "guest" +} + +// Macro generates: +extension AppSettings: __Settings_Container { + static let prefix = "app_" + enum __Attribute_username: __AttributeNonOptional { /* ... */ } + static var $username: __AttributeProxy<__Attribute_username> +} +``` + +**Key files:** `Sources/SettingsMacros/SettingsMacro.swift` (1495 lines), `SettingsMacro.swift` (268 lines) + +### SwiftUI Integration Pattern +- Use `AppSettingValues` container for app-wide settings (no custom `@Settings` needed) +- Inject mock stores via `.environment(\.userDefaultsStore, UserDefaultsStoreMock())` in previews +- Global prefix configuration: `AppSettingValues.prefix = "myapp_"` (auto-generates from bundle ID if not set) + +**Key file:** `Sources/Settings/SwiftUI/AppSetting.swift` + +### Thread-Safety Requirements +- Never use `@MainActor` on containers—breaks Sendable metatype requirements in Combine closures +- Use `OSAllocatedUnfairLock` for non-Sendable type access (iOS 17+, macOS 15+) +- Example: `AppSettingValues._config` protects both `store` and `prefix` with single lock + +### Encoding Strategy +- Native property list types (Int, String, Bool, Date, Data, URL, Array, Dictionary) stored directly +- Codable types require `@Setting(encoding: .json)` or `.propertyList` +- Custom encoders: `@Setting(encoder: JSONEncoder(), decoder: JSONDecoder())` + +**Key files:** `Sources/Settings/MacroSetting/Coding.swift`, protocol `PropertyListValue` in `Attribute.swift` + +### Testing Patterns +- Macro expansion tests: Use `SwiftSyntaxMacrosTestSupport` in `Tests/SettingsMacroExpansionTests/` +- Runtime tests: Standard XCTest in `Tests/SettingsTests/` +- Mock testing: `UserDefaultsStoreMock()` with `.recordingEnabled` for verification + +## Key Files Reference + +- `Sources/Settings/MacroSettings/Settings_Container.swift` - Core protocol with store operations +- `Sources/Settings/MacroSetting/Attribute.swift` - Base protocol for generated attribute enums +- `Sources/SettingsMacros/SettingMacro.swift` - Generates getters/setters + attribute enums (~1500 lines) +- `Sources/SettingsMacros/SettingsMacro.swift` - Generates container conformance + prefix +- `Package.swift` - Multi-target SPM package with macro plugin support + +## Common Pitfalls + +1. **Don't add `@MainActor` to containers** - Causes "Type 'X' does not conform to the 'Sendable' protocol" errors in Combine publishers +2. **Protocol existentials aren't Sendable** - `any UserDefaultsStore` requires `OSAllocatedUnfairLock` wrapper +3. **Prefix validation** - Must be KVC-compliant (no dots allowed) +4. **Optional vs non-optional** - Optional properties generate `__AttributeOptional`, non-optional generate `__AttributeNonOptional` with different reset behavior diff --git a/Sources/Settings/MacroSettings/UserDefaultsStore.swift b/Sources/Settings/MacroSettings/UserDefaultsStore.swift index 91de7a2..2fb74e1 100644 --- a/Sources/Settings/MacroSettings/UserDefaultsStore.swift +++ b/Sources/Settings/MacroSettings/UserDefaultsStore.swift @@ -4,7 +4,7 @@ public protocol Cancellable: Sendable { func cancel() } -public protocol UserDefaultsStore: Sendable { +public protocol UserDefaultsStore: Sendable, AnyObject { associatedtype Observer: Cancellable diff --git a/Sources/Settings/SwiftUI/AppSetting.swift b/Sources/Settings/SwiftUI/AppSetting.swift index a78bf1c..b65d696 100644 --- a/Sources/Settings/SwiftUI/AppSetting.swift +++ b/Sources/Settings/SwiftUI/AppSetting.swift @@ -83,7 +83,7 @@ public struct AppSettingValues: __Settings_Container { } set { _config.withLock { config in - config.prefix = newValue + config.prefix = newValue.replacing(".", with: "_") } } } @@ -185,10 +185,10 @@ where Attribute.Value: Sendable { public var projectedValue: Binding { Binding( get: { - value + wrappedValue }, set: { value in - Attribute.write(value: value) + wrappedValue = value } ) } @@ -252,10 +252,11 @@ where Attribute.Value: Sendable { /// Called by SwiftUI to set up the publisher subscription. public mutating func update() { - AppSettingValues.store = environmentStore - if observer.cancellable == nil { - observer.start(binding: $value) + if !(AppSettingValues.store === environmentStore) { + AppSettingValues.store = environmentStore } + // print("*** SwiftUI update: \(AppSettingValues.store.dictionaryRepresentation())") + observer.observe(binding: $value) } } @@ -264,19 +265,31 @@ extension AppSetting { @MainActor final class Observer { var cancellable: AnyCancellable? = nil + weak var usersDefaultsStore: (any UserDefaultsStore)? init() {} - func start(binding: Binding) { + func observe(binding: Binding) { + // When the store of the attribute has changed, we need + // to cancel the subscription and create a new one: + if !(usersDefaultsStore === Attribute.Container.store) { + cancel() + } if cancellable == nil { + usersDefaultsStore = Attribute.Container.store cancellable = Attribute.publisher .catch { _ in Just(Attribute.read()) } .receive(on: DispatchQueue.main) .sink { newValue in + print("Observer: \(Attribute.self) = \(newValue)") binding.wrappedValue = newValue } } } + + func cancel() { + cancellable = nil + } } } diff --git a/Sources/Settings/SwiftUI/UserDefaultsStoreEnvironment.swift b/Sources/Settings/SwiftUI/UserDefaultsStoreEnvironment.swift index d66b1cf..d6d381b 100644 --- a/Sources/Settings/SwiftUI/UserDefaultsStoreEnvironment.swift +++ b/Sources/Settings/SwiftUI/UserDefaultsStoreEnvironment.swift @@ -23,7 +23,24 @@ extension EnvironmentValues { /// } /// } /// ``` - @Entry public var userDefaultsStore: any UserDefaultsStore = UserDefaults.standard + public var userDefaultsStore: any UserDefaultsStore { + get { + self[__Key_userDefaultsStore.self] + } + set { + self[__Key_userDefaultsStore.self] = newValue + } + } + + private struct __Key_userDefaultsStore: SwiftUICore.EnvironmentKey { + + static var defaultValue: any UserDefaultsStore { + get { + UserDefaults.standard + } + } + } + } extension View { diff --git a/Sources/SettingsMock/UserDefaultsMock.swift b/Sources/SettingsMock/UserDefaultsMock.swift index 094a45e..2a82776 100644 --- a/Sources/SettingsMock/UserDefaultsMock.swift +++ b/Sources/SettingsMock/UserDefaultsMock.swift @@ -55,13 +55,13 @@ public final class UserDefaultsStoreMock: NSObject, UserDefaultsStore, Sendable } } - private func plistDeepCopy(_ value: Any) -> Any? { + private static func plistDeepCopy(_ value: Any) -> Any? { guard PropertyListSerialization.propertyList(value, isValidFor: .binary) else { return nil } guard let data = try? PropertyListSerialization.data(fromPropertyList: value, format: .binary, options: 0) else { return nil } return try? PropertyListSerialization.propertyList(from: data, options: [], format: nil) } - private func isEqualPlist(_ lhs: Any?, _ rhs: Any?) -> Bool { + private static func isEqualPlist(_ lhs: Any?, _ rhs: Any?) -> Bool { switch (lhs, rhs) { case (nil, nil): return true @@ -97,14 +97,14 @@ public final class UserDefaultsStoreMock: NSObject, UserDefaultsStore, Sendable public func set(_ value: Any?, forKey key: String) { if let value { // Only accept property-list-serializable values; mimic UserDefaults by ignoring unsupported values. - guard let stored = plistDeepCopy(value) else { + guard let stored = Self.plistDeepCopy(value) else { return } var oldEffective: Any? state.withLockUnchecked { state in oldEffective = state.values[key] ?? state.defaults[key] } - let shouldNotify = !isEqualPlist(oldEffective, stored) + let shouldNotify = !Self.isEqualPlist(oldEffective, stored) if shouldNotify { willChangeValue(forKey: key) } state.withLockUnchecked { state in state.values[key] = stored @@ -118,7 +118,7 @@ public final class UserDefaultsStoreMock: NSObject, UserDefaultsStore, Sendable oldEffective = state.values[key] ?? state.defaults[key] newEffective = state.defaults[key] } - let shouldNotify = !isEqualPlist(oldEffective, newEffective) + let shouldNotify = !Self.isEqualPlist(oldEffective, newEffective) if shouldNotify { willChangeValue(forKey: key) } state.withLock { state in _ = state.values.removeValue(forKey: key) @@ -190,10 +190,10 @@ public final class UserDefaultsStoreMock: NSObject, UserDefaultsStore, Sendable public func set(_ url: URL?, forKey key: String) { set(url as Any?, forKey: key) } public func register(defaults newDefaults: [String : Any]) { - print("register(defaults(\(newDefaults)") + print(Self.self, "register(defaults(\(newDefaults)") for (key, newDefault) in newDefaults { // Only accept property-list-serializable defaults - guard let copy = plistDeepCopy(newDefault) else { continue } + guard let copy = Self.plistDeepCopy(newDefault) else { continue } var hasUserValue = false var oldEffective: Any? @@ -204,7 +204,7 @@ public final class UserDefaultsStoreMock: NSObject, UserDefaultsStore, Sendable newEffective = hasUserValue ? state.values[key] : copy } - let shouldNotify = !hasUserValue && !isEqualPlist(oldEffective, newEffective) + let shouldNotify = !hasUserValue && !Self.isEqualPlist(oldEffective, newEffective) if shouldNotify { willChangeValue(forKey: key) } state.withLockUnchecked { state in state.defaults[key] = copy