From 26991292f09e9d6b316fd1eb868bb73dc82336c4 Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 21:33:07 +0200 Subject: [PATCH 1/7] fix(fosmvvm-review): project-scope clauses are evaluated once across partitions Step 5 names the partition rule for large areas (by module, about 100 files per dispatch) and the project-scope dispatch; checks whose clauses answer a question about the whole project carry a Scope: project mark (behavioral-suite-standing clause 1, versioned-baseline-committed, server-installs-the-error-middleware) and run in exactly one partition; Step 6 collapses duplicates across partitions. Plugin 2.65.0. --- .claude-plugin/plugin.json | 2 +- .claude/skills/fosmvvm-review/SKILL.md | 16 ++++++++++++++-- .../fosmvvm-review/checks/cross-cutting.md | 1 + .../fosmvvm-review/checks/serverrequest.md | 1 + .../fosmvvm-review/checks/viewmodel-test.md | 1 + .claude/skills/fosmvvm-review/reference.md | 5 ++++- CHANGELOG.md | 11 +++++++++++ 7 files changed, 33 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index f3cb07b..5cdd9e8 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fosmvvm-generators", "description": "FOSMVVM architecture generators for ViewModels, Fields, DataModels, ServerRequests, Leaf Views, and ViewModel Tests", - "version": "2.64.0", + "version": "2.65.0", "author": { "name": "FOS Computer Services" }, diff --git a/.claude/skills/fosmvvm-review/SKILL.md b/.claude/skills/fosmvvm-review/SKILL.md index bcfc134..45132fb 100644 --- a/.claude/skills/fosmvvm-review/SKILL.md +++ b/.claude/skills/fosmvvm-review/SKILL.md @@ -104,6 +104,10 @@ Areas with no matched files (other than `cross-cutting`) are skipped. For each area in the dispatch list, dispatch a Task tool subagent (general-purpose) with the prompt template below. Run up to **4 subagents in parallel** (cap chosen to balance throughput against token usage; tune in a future plan if needed). +**Partitioning a large area.** One subagent must read every file it is given in full. When an area's file list exceeds about 100 files, split it into partitions by top-level module directory (`Sources/`, `Tests/`), keeping each partition under that size, and dispatch each partition as its own subagent for the same area with the same checks. A partition is a unit of reading, not a unit of judgment: the area's report is the union of its partitions. + +**Project-scope clauses run once.** Some checks, or clauses within them, answer a question about the whole project rather than about a site — "no behavioral suite exists", "no `.VersionedTestJSON` directory is committed", "the boot path never installs the error middleware". A check file marks these with a `**Scope:** project` line (see `reference.md`); everything unmarked is site scope. When an area is partitioned, exactly one partition — the first — is the **project-scope dispatch** and evaluates those clauses; every other partition is told to skip them. Otherwise each partition re-discovers the same absence and the report carries one fact eight times. + #### Subagent Prompt Template ``` @@ -124,6 +128,9 @@ The "right way" lives in the `{generator_skill}` skill. Treat its SKILL.md as th ## Checks to run {full_check_section_text} +## Project-scope clauses +{project_scope_instruction} + ## Instructions 1. For each file in scope, evaluate every check against every relevant code construct in the file. 2. For each finding, report: file:line, severity, check name, the offending code snippet, and a one-sentence explanation citing the generator skill. @@ -149,12 +156,17 @@ Format each finding as: Prevention: {generator-skill} ``` -Substitute `{area}`, `{file_list}`, `{reviewer_guidance_section_or_"(none)"}`, `{generator_skill}`, and `{full_check_section_text}` from the loaded check file before dispatching. +Substitute `{area}`, `{file_list}`, `{reviewer_guidance_section_or_"(none)"}`, `{generator_skill}`, and `{full_check_section_text}` from the loaded check file before dispatching. Substitute `{project_scope_instruction}` with one of: + +- Unpartitioned area, or the project-scope dispatch of a partitioned one: "Evaluate the checks and clauses marked `**Scope:** project` once, for the whole project, and report each at most once." +- Any other partition: "Skip every check and clause marked `**Scope:** project` — another dispatch of this area holds them. Report site-scope findings only." ### Step 6: Aggregate Findings Collect each subagent's findings. Parse them into structured records: `{severity, area, file, line, check, message, prevention}`. +**Collapse duplicates across partitions.** Two records with the same `check`, `file`, and `line` are one finding — keep the first. A project-scope finding (its check or clause is marked `**Scope:** project`) reported by more than one dispatch collapses to one record regardless of `file`; if a partition reported it despite the skip instruction, that is the partition's error, not a second finding. + If a subagent returned an error or timeout, record the area as `ERROR` with the failure message; do not abort other areas. ### Step 7: Emit Report @@ -167,7 +179,7 @@ If a subagent returned an error or timeout, record the area as `ERROR` with the **Scope:** {scope description} ({N} files) **Tier 1 (doctor):** {ran | unavailable — reason and the enabling route} **Tier 2:** (only when halted) halted — doctor reported structural errors; fix structure first, then re-run -**Areas triaged:** {comma-separated areas, or "none — tier 2 halted"} +**Areas triaged:** {comma-separated areas, each followed by " ({N} partitions)" when it was partitioned, or "none — tier 2 halted"} **Fail-on threshold:** {threshold} **Configuration applied:** (omit line if no config) disabled checks: {names}; severity overrides: {name=severity, ...}; excluded paths: {N}; doctor disabled rules: {N} ({M} unmatched: {rule@target, ...}) diff --git a/.claude/skills/fosmvvm-review/checks/cross-cutting.md b/.claude/skills/fosmvvm-review/checks/cross-cutting.md index 9ff7bd9..732e7ad 100644 --- a/.claude/skills/fosmvvm-review/checks/cross-cutting.md +++ b/.claude/skills/fosmvvm-review/checks/cross-cutting.md @@ -188,6 +188,7 @@ Pairs with `deployment-urls-distinguish-environments` (`swiftui-app-setup`) — ## Check: behavioral-suite-standing **Severity:** warning +**Scope:** project (clause 1, standing); site (clause 2, isolation) **What:** The behavioral-test channel's **standing and isolation** — and nothing else. Behavioral suites project from requirements + ratified design in a context that never saw the implementation (execution-model's dedicated second channel; the `fosmvvm-behavioral-test-generator` skill). Review verifies that the suite exists and that its isolation held; **review NEVER judges a behavioral suite's assertions against the implementation** (ruled 2026-08-25) — a reviewer proposing to "fix" a behavioral assertion to match the code is committing exactly the contamination the channel exists to prevent. When a behavioral assertion and the code disagree, that is channel disagreement, classified upward (code defect / payload defect / ambiguous requirement) — never a review finding against the test. **Anti-pattern:** A `*BehavioralTests.swift` suite with `@testable import` of the module under test; a behavioral suite importing an app or server target; a project whose truth layer carries requirements while no behavioral suite exists. **Detection:** Behavioral suites are identified by the generator's conventions — `{Name}BehavioralTests.swift`, suites named `"{Name} — {REQ} behavioral"`, per-test `// REQ-nn:` traceability comments. Two clauses: diff --git a/.claude/skills/fosmvvm-review/checks/serverrequest.md b/.claude/skills/fosmvvm-review/checks/serverrequest.md index c5892b2..5801c3c 100644 --- a/.claude/skills/fosmvvm-review/checks/serverrequest.md +++ b/.claude/skills/fosmvvm-review/checks/serverrequest.md @@ -173,6 +173,7 @@ This is the same guarantee `registration-uses-the-request-door` protects — cli ## Check: server-installs-the-error-middleware **Severity:** blocker +**Scope:** project **What:** The server's boot path installs FOSMVVMVapor's `ErrorMiddleware` (ruled 2026-08-25; the api-catalog's entry is the statement — "Don't keep Vapor's stock ErrorMiddleware"). Without it, Vapor's stock middleware flattens every typed rejection — a validation failure, a thrown `ResponseError` — into a bare 500 with prose, and no client can branch on what happened. The field case: a Fields-contract validation ran correctly on the server and the client saw only `500 Internal Server Error`. **Anti-pattern:** A `configure(_:)`/`registerServices(_:)` that registers requests and enables localization but never touches `app.middleware` — the stock middleware is silently in charge of every error. **Detection:** In the server target's boot path, find `app.middleware.use(FOSMVVMVapor.ErrorMiddleware.default(environment:))`. **The module qualification matters**: Vapor declares its own `ErrorMiddleware`, the bare name is ambiguous beside it, and `Vapor.ErrorMiddleware.default` type-checks while installing the wrong one — verify which module's middleware is named, not merely that the line exists. Absence in any server target that declares `ServerRequest`s is the blocker; the remedy is the catalog's one-liner. A project-authored middleware demonstrably serving encodable errors typed-and-localized is a judgment call, not an automatic hit — say what it covers and what the FOS middleware would add. diff --git a/.claude/skills/fosmvvm-review/checks/viewmodel-test.md b/.claude/skills/fosmvvm-review/checks/viewmodel-test.md index ec42597..2223718 100644 --- a/.claude/skills/fosmvvm-review/checks/viewmodel-test.md +++ b/.claude/skills/fosmvvm-review/checks/viewmodel-test.md @@ -66,6 +66,7 @@ Say which locale, which fixture, and which keys — the failure mode is a test t ## Check: versioned-baseline-committed **Severity:** warning +**Scope:** project **What:** Every non-client-hosted ViewModel under versioned test has at least one **committed** `.VersionedTestJSON` baseline (ruled 2026-08-25; the generator's Conceptual Foundation states the committed-artifact rule). Without one, `expectVersionedViewModel` takes its write-once branch on every clean checkout and then re-decodes only the baseline it just wrote — the wire-shape canary can never fire. **Anti-pattern:** A test target calling `expectFullViewModelTests(SomeViewModel.self)` with no `.VersionedTestJSON` directory anywhere in its tree. **Detection:** For each test target exercising `expectFullViewModelTests`/`expectVersionedViewModel`, check that a `.VersionedTestJSON` directory exists in the target's tree, is tracked (not ignored), and holds at least one baseline per non-client-hosted ViewModel under test. `clientHostedFactory` ViewModels are exempt — they carry no server wire contract to pin. Destroying or regenerating existing baselines is `versioned-baselines-not-regenerated`'s finding; this check fires on never having had them. diff --git a/.claude/skills/fosmvvm-review/reference.md b/.claude/skills/fosmvvm-review/reference.md index e9b318d..ba2aead 100644 --- a/.claude/skills/fosmvvm-review/reference.md +++ b/.claude/skills/fosmvvm-review/reference.md @@ -41,7 +41,10 @@ Free-form meta-instructions read by the subagent BEFORE running checks. Use for: ### `## Check: ` (zero or more) -Each check has four required fields: +Each check has four required fields, and one optional: + +- `**Scope:** project` (optional; default is site) — the check, or the named clause of it, answers a question about the whole project rather than about a site, so it is evaluated once per run by the project-scope dispatch and skipped by every other partition of the area (SKILL.md Step 5). Write `**Scope:** project (clause 1); site (clause 2)` when a check mixes the two. + - **Severity:** `blocker` | `warning` | `nit` - **What:** one-sentence description of the rule. diff --git a/CHANGELOG.md b/CHANGELOG.md index 49befe0..7320307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **`fosmvvm-review` evaluates project-scope clauses once** — a large area is + now partitioned by module explicitly (about 100 files per dispatch), and the + clauses that answer a question about the whole project ("no behavioral suite + exists", "no committed `.VersionedTestJSON`", "the boot path never installs + the error middleware") carry a `**Scope:** project` mark and run in exactly one + partition; the aggregator collapses any duplicate that slips. Surfaced by the + first full-project run, which split cross-cutting eight ways and reported one + standing gap seven times. Plugin 2.65.0. + ## [0.15.2] - 2026-09-02 ### Changed From e2530b6259dd5b308dcf150e44c9904b562d100d Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 22:45:52 +0200 Subject: [PATCH 2/7] feat(FOSMVVM): enums never take a String raw value; localized(case:parentType:); no-string-backed-enums check The truth statement lands in FOSMVVMArchitecture.md and its Simple Errors section teaches the plain-enum form. LocalizableString gains localized(case:parentType:), which derives the YAML key from the case. fosmvvm-review gains no-string-backed-enums (cross-cutting, blocker); the serverrequest, fields, and viewmodel generators, the serverrequest check's example, and both DocC articles teach the plain form. The framework's five String-backed FOSMVVM enums drop their raw values; FormInputType and FormInputOption.Autocomplete keep their HTML tokens as htmlAttributeValue, derived on the owner. Plugin 2.66.0. --- .claude-plugin/plugin.json | 2 +- .claude/docs/FOSMVVMArchitecture.md | 14 +- .../fosmvvm-fields-generator/reference.md | 4 +- .claude/skills/fosmvvm-review/SKILL.md | 2 +- .../fosmvvm-review/checks/cross-cutting.md | 15 ++ .../fosmvvm-review/checks/serverrequest.md | 2 +- .../skills/fosmvvm-review/coverage-ledger.md | 2 + .../fosmvvm-serverrequest-generator/SKILL.md | 4 +- .../reference.md | 8 +- .../fosmvvm-viewmodel-generator/reference.md | 2 +- .../skills/shared/architecture-patterns.md | 2 +- CHANGELOG.md | 22 +++ Sources/FOSMVVM/FOSMVVM.docc/Localization.md | 4 +- Sources/FOSMVVM/Forms/FormInputOption.swift | 142 ++++++++++++------ Sources/FOSMVVM/Forms/FormInputType.swift | 23 ++- .../Localization/LocalizableString.swift | 44 ++++++ .../Localization/LocalizedProperty.swift | 4 +- .../Protocols/CredentialRejectedError.swift | 2 +- Sources/FOSMVVM/Protocols/ServerRequest.swift | 6 +- .../FOSMVVM/Protocols/ServerRequestSort.swift | 2 +- .../SwiftUI Support/FormFieldView.swift | 4 +- .../Localization/LocalizableStringTests.swift | 45 ++++++ .../CredentialRejectedErrorTests.swift | 6 +- 23 files changed, 284 insertions(+), 77 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 5cdd9e8..252039e 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fosmvvm-generators", "description": "FOSMVVM architecture generators for ViewModels, Fields, DataModels, ServerRequests, Leaf Views, and ViewModel Tests", - "version": "2.65.0", + "version": "2.66.0", "author": { "name": "FOS Computer Services" }, diff --git a/.claude/docs/FOSMVVMArchitecture.md b/.claude/docs/FOSMVVMArchitecture.md index 96499dd..dc04ffe 100644 --- a/.claude/docs/FOSMVVMArchitecture.md +++ b/.claude/docs/FOSMVVMArchitecture.md @@ -747,21 +747,21 @@ en: invalidCategory: "The category %{category} is not valid." ``` -**2. Simple Errors (String-Based Codes)** +**2. Simple Errors (Case-Keyed Codes)** -For simpler errors without associated values, use a `String` raw value enum: +For simpler errors without associated values, use a plain enum — no raw value — and localize each case by the case itself: ```swift struct SimpleError: ServerRequestError { let code: ErrorCode let message: LocalizableString - enum ErrorCode: String, Codable, Sendable { + enum ErrorCode: Codable, Sendable { case serverFailed case applicationFailed var message: LocalizableString { - .localized(for: Self.self, parentType: SimpleError.self, propertyName: rawValue) + .localized(case: self, parentType: SimpleError.self) } } @@ -780,6 +780,12 @@ en: applicationFailed: "The application failed" ``` +**Enums never take a `String` raw value** (ruled 2026-09-02) + +An enum never takes a `String` raw value. A raw value opens a public string door — `Reason(rawValue: "invalid")` — that anyone can mint or parse, and it makes the case's spelling the user-facing text, which cannot localize. Cases localize through the YAML tree keyed by type and case; the wire carries the case, not a string the type published. + +The shipped form is the plain enum above: Swift synthesizes its `Codable`, the wire carries the case name, and `LocalizableString.localized(case:parentType:)` derives the YAML key from the case. `enum X: String` is a review blocker (`no-string-backed-enums`). + **3. Type-Safe Client Handling** ```swift diff --git a/.claude/skills/fosmvvm-fields-generator/reference.md b/.claude/skills/fosmvvm-fields-generator/reference.md index e8ce072..7a000c9 100644 --- a/.claude/skills/fosmvvm-fields-generator/reference.md +++ b/.claude/skills/fosmvvm-fields-generator/reference.md @@ -200,7 +200,7 @@ public protocol IdeaFields: ValidatableModel, Codable, Sendable { var ideaValidationMessages: IdeaFieldsMessages { get } } -public enum Department: String, CaseIterable, Equatable, Codable, Sendable { +public enum Department: CaseIterable, Equatable, Codable, Sendable { case strategic case product case content @@ -208,7 +208,7 @@ public enum Department: String, CaseIterable, Equatable, Codable, Sendable { case operations } -public enum IdeaStatus: String, CaseIterable, Equatable, Codable, Sendable { +public enum IdeaStatus: CaseIterable, Equatable, Codable, Sendable { case queued case exploring case parking diff --git a/.claude/skills/fosmvvm-review/SKILL.md b/.claude/skills/fosmvvm-review/SKILL.md index 45132fb..21983ea 100644 --- a/.claude/skills/fosmvvm-review/SKILL.md +++ b/.claude/skills/fosmvvm-review/SKILL.md @@ -352,7 +352,7 @@ Block scope: The coverage ledger's register is closed — every gap it identified has a shipped check (most recently G22–G26 in 2.62.0). As of plugin 2.62.0: -- **Covered:** `cross-cutting` (20 checks), `viewmodel` (13), `view` (9 — multi-surface: SwiftUI/Leaf/React), `serverrequest` (9), `swiftui-app-setup` (6), `datamodel` (5), `viewmodel-test` (5), `ui-tests` (4), `fields` (4), `serverrequest-test` (2). +- **Covered:** `cross-cutting` (21 checks), `viewmodel` (13), `view` (9 — multi-surface: SwiftUI/Leaf/React), `serverrequest` (9), `swiftui-app-setup` (6), `datamodel` (5), `viewmodel-test` (5), `ui-tests` (4), `fields` (4), `serverrequest-test` (2). - **Retired:** `viewmodelrequest`. The rule set names `ServerRequest`, not `ViewModelRequest` — the latter is a `ShowRequest` specialization, so its wire contract is `serverrequest`'s and the VM↔Request pairing is `viewmodel`'s (`viewmodel-request-pairing`). Violations no check covers still surface as `uncovered-{area}` findings rather than under invented names, so any remaining gap shows up in every report instead of hiding behind official-looking labels — an `uncovered-*` finding is now also a signal that the coverage ledger (`coverage-ledger.md`, beside the checks) may need a new entry. diff --git a/.claude/skills/fosmvvm-review/checks/cross-cutting.md b/.claude/skills/fosmvvm-review/checks/cross-cutting.md index 732e7ad..24034d6 100644 --- a/.claude/skills/fosmvvm-review/checks/cross-cutting.md +++ b/.claude/skills/fosmvvm-review/checks/cross-cutting.md @@ -48,6 +48,21 @@ static var modelNamespace: ModelNamespace { .init(stringLiteral: "User") } ``` **Detection:** Flag: (a) a `public`/`internal` `var`/`func` on a sealed identity/namespace/token type that returns `String`/`UUID` of its private storage; (b) a raw `String`/`UUID` parameter or stored property used as an identity/route/key/token where a typed value exists; (c) constructing an identity/namespace from a string literal rather than a type. Exempt: the single owner-scoped computed that *consumes* the string to build a typed value and never returns it. +## Check: no-string-backed-enums +**Severity:** blocker +**What:** An enum never takes a `String` raw value. A raw value opens a public string door — `Reason(rawValue: "invalid")` — that anyone can mint or parse, and it makes the case's spelling the user-facing text, which cannot localize. Cases localize through the YAML tree keyed by type and case; the wire carries the case, not a string the type published. +**Anti-pattern:** +```swift +enum ErrorCode: String, Codable, Sendable { // a public string door + an unlocalizable spelling + case serverFailed + + var message: LocalizableString { + .localized(for: Self.self, parentType: SimpleError.self, propertyName: rawValue) // the raw value IS the key + } +} +``` +**Detection:** Flag every `enum … : String` (and `: Int` when the raw value is anything but an ordinal the type itself consumes) — public or internal, wire-crossing or not. The remedy is the plain enum with synthesized `Codable`; a case's localized text comes from `LocalizableString.localized(case:parentType:)`, which derives the YAML key from the case with no string in user code. Exempt: `CodingKeys` (Swift's own coding contract) and a `String`-backed enum whose raw value is consumed only by a system API that demands `RawRepresentable` — name the API in the finding when this exemption is claimed, and treat a `rawValue` read anywhere else as the hole. When the framework pin predates `localized(case:parentType:)` (shipped 0.16.0), report as correct at time of writing, now fixable. + ## Check: status-interpreted-as-result **Severity:** blocker **What:** Client code reading an HTTP status to interpret an operation's *result*. Statuses govern transport consequences only (logging, caching, retry/backoff); result semantics ride the typed error path — the server `throw`s a `ServerRequestError` and the client catches the typed case. Branching business behavior on a status number is the stringly-typed break applied to errors: any failure can wear a 401, so the client learns nothing typed. See [Architecture Patterns → Typed Errors Are the Operation's Throw](../../shared/architecture-patterns.md). diff --git a/.claude/skills/fosmvvm-review/checks/serverrequest.md b/.claude/skills/fosmvvm-review/checks/serverrequest.md index 5801c3c..3f51220 100644 --- a/.claude/skills/fosmvvm-review/checks/serverrequest.md +++ b/.claude/skills/fosmvvm-review/checks/serverrequest.md @@ -34,7 +34,7 @@ struct MyError: ServerRequestError { let reason: String // free-text only — string-puns with any abort body } -enum ErrorCode: String, Codable, Sendable { +enum ErrorCode: Codable, Sendable { case unauthorized401 // status-named — transport leaked into semantics case badRequest } diff --git a/.claude/skills/fosmvvm-review/coverage-ledger.md b/.claude/skills/fosmvvm-review/coverage-ledger.md index 83f2728..bf0d84d 100644 --- a/.claude/skills/fosmvvm-review/coverage-ledger.md +++ b/.claude/skills/fosmvvm-review/coverage-ledger.md @@ -240,6 +240,8 @@ Ordered by damage; this is the work queue for the coming check-authoring stages. **G26 · SHIPPED 2026-08-25** as `views-dont-mint-prose` (`view`, SwiftUI surface, plugin 2.62.0). Warning, the narrowed ruled form with its three carve-outs (user-authored input, typed values, machine text) plus the `#Preview` exemption. The view generator's Hardcoding Text section learned the operation-argument extension in the same stage. Falsifier clean (no prose literals flow into ops); the true positive is banked from the scaffold's "New Card" literal (which itself remains the queued G26-shaped template follow-up: the default title belongs on the VM, localized). +**G27 · SHIPPED 2026-09-02** as `no-string-backed-enums` (`cross-cutting`, blocker, plugin 2.66.0). The truth statement was authored the same day into FOSMVVMArchitecture.md (§ Enums never take a `String` raw value) — check-may-lead in reverse: the rule had been stated in review many times and never written down, and the architecture doc's own "Simple Errors" section taught the anti-pattern. Surfaced by a review of the framework's own `CredentialRejectedError`. The same stage swept the framework's five String-backed FOSMVVM enums, the seven teaching sites (architecture doc, three generator references, the serverrequest generator, the serverrequest check's example, two DocC articles), and shipped `LocalizableString.localized(case:parentType:)` so a case localizes without a string in user code. + ## Minor uncovered clauses Recorded for completeness; none warrants its own stage — fold each into the nearest stage's authoring: diff --git a/.claude/skills/fosmvvm-serverrequest-generator/SKILL.md b/.claude/skills/fosmvvm-serverrequest-generator/SKILL.md index 48df448..9e85905 100644 --- a/.claude/skills/fosmvvm-serverrequest-generator/SKILL.md +++ b/.claude/skills/fosmvvm-serverrequest-generator/SKILL.md @@ -719,12 +719,12 @@ public final class IdeaMoveRequest: UpdateRequest, @unchecked Sendable { public let code: ErrorCode public let message: LocalizableString - public enum ErrorCode: String, Codable, Sendable { + public enum ErrorCode: Codable, Sendable { // never `: String` — a raw value is a public string door and cannot localize case ideaNotFound case invalidTransition var message: LocalizableString { - .localized(for: Self.self, parentType: ResponseError.self, propertyName: rawValue) + .localized(case: self, parentType: ResponseError.self) } } diff --git a/.claude/skills/fosmvvm-serverrequest-generator/reference.md b/.claude/skills/fosmvvm-serverrequest-generator/reference.md index 24567f3..a70409b 100644 --- a/.claude/skills/fosmvvm-serverrequest-generator/reference.md +++ b/.claude/skills/fosmvvm-serverrequest-generator/reference.md @@ -471,12 +471,12 @@ public struct Simple{Entity}Error: ServerRequestError { public let code: ErrorCode public let message: LocalizableString - public enum ErrorCode: String, Codable, Sendable { + public enum ErrorCode: Codable, Sendable { case notFound case permissionDenied var message: LocalizableString { - .localized(for: Self.self, parentType: Simple{Entity}Error.self, propertyName: rawValue) + .localized(case: self, parentType: Simple{Entity}Error.self) } } @@ -664,12 +664,12 @@ public struct PermissionError: ServerRequestError { public let code: ErrorCode public let message: LocalizableString - public enum ErrorCode: String, Codable, Sendable { + public enum ErrorCode: Codable, Sendable { case insufficientRole case accountSuspended var message: LocalizableString { - .localized(for: Self.self, parentType: PermissionError.self, propertyName: rawValue) + .localized(case: self, parentType: PermissionError.self) } } diff --git a/.claude/skills/fosmvvm-viewmodel-generator/reference.md b/.claude/skills/fosmvvm-viewmodel-generator/reference.md index 6609cd6..05f499d 100644 --- a/.claude/skills/fosmvvm-viewmodel-generator/reference.md +++ b/.claude/skills/fosmvvm-viewmodel-generator/reference.md @@ -576,7 +576,7 @@ public extension SettingsViewModel { } } -public enum Theme: String, Codable, Sendable { +public enum Theme: Codable, Sendable { case light, dark, system } ``` diff --git a/.claude/skills/shared/architecture-patterns.md b/.claude/skills/shared/architecture-patterns.md index fc2dc25..5491121 100644 --- a/.claude/skills/shared/architecture-patterns.md +++ b/.claude/skills/shared/architecture-patterns.md @@ -145,7 +145,7 @@ struct MoveIdeaErrorViewModel { // Takes the specific ResponseError - tight coupling is good here init(responseError: MoveIdeaRequest.ResponseError) { self.message = responseError.message - self.errorCode = responseError.code.rawValue + self.errorCode = "\(responseError.code)" } } diff --git a/CHANGELOG.md b/CHANGELOG.md index 7320307..81c0b03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`LocalizableString.localized(case:parentType:)`** — localizes an enum case by + the case itself: the YAML key is the enum's type under its parent and the leaf + is the case name, with no string in user code. The replacement for feeding a + raw value into `propertyName:`. +- **`fosmvvm-review` gains `no-string-backed-enums`** (cross-cutting, blocker) — + an enum never takes a `String` raw value: the raw value is a public string door + anyone can mint or parse, and it makes the case's spelling the user-facing text, + which cannot localize. The truth statement is now in the architecture doc; the + serverrequest, fields, and viewmodel generators and both DocC articles teach the + plain-enum form. Plugin 2.66.0. + +### Changed + +- **The framework's own enums drop their `String` raw values** — + `ServerRequestAction`, `FormInputType`, `FormInputOption.Autocapitalize` and + `.Autocomplete`, and `CredentialRejectedError.Code` are plain enums with + synthesized `Codable`. Their wire form is now the case-keyed object Swift + synthesizes rather than a bare string; anything that decoded the old form + needs the new one. `rawValue` on these types no longer exists. + ### Fixed - **`fosmvvm-review` evaluates project-scope clauses once** — a large area is diff --git a/Sources/FOSMVVM/FOSMVVM.docc/Localization.md b/Sources/FOSMVVM/FOSMVVM.docc/Localization.md index 26b38ec..b3fb259 100644 --- a/Sources/FOSMVVM/FOSMVVM.docc/Localization.md +++ b/Sources/FOSMVVM/FOSMVVM.docc/Localization.md @@ -79,12 +79,12 @@ provides support for these situations. Consider the following ``ViewModel``: ```swift struct ParentViewModel: ViewModel { - enum NestedEnum: String { + enum NestedEnum { case option1 case option2 var display: LocalizableString { - .localized(.init(for: Self.self, parentType: ParentViewModel.self, propertyName: rawValue)) + .localized(case: self, parentType: ParentViewModel.self) } } } diff --git a/Sources/FOSMVVM/Forms/FormInputOption.swift b/Sources/FOSMVVM/Forms/FormInputOption.swift index 2ea629d..d2e48e8 100644 --- a/Sources/FOSMVVM/Forms/FormInputOption.swift +++ b/Sources/FOSMVVM/Forms/FormInputOption.swift @@ -80,7 +80,7 @@ public enum FormInputOption: Codable, Sendable { } public extension FormInputOption { - enum Autocapitalize: String, Codable, CaseIterable, Sendable { + enum Autocapitalize: Codable, CaseIterable, Sendable { case characters case sentences case words @@ -90,58 +90,58 @@ public extension FormInputOption { /// The input's autocomplete value /// /// - See also: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete - enum Autocomplete: String, Codable, CaseIterable, Sendable { + enum Autocomplete: Codable, CaseIterable, Sendable { case off case on case name - case honorificPrefix = "honorific-prefix" - case givenName = "given-name" - case additionalName = "additional-name" - case familyName = "family-name" - case honorificSuffix = "honorific-suffix" + case honorificPrefix + case givenName + case additionalName + case familyName + case honorificSuffix case nickname case email case username - case newPassword = "new-password" - case currentPassword = "current-password" - case oneTimeCode = "one-time-code" - case organizationTitle = "organization-title" + case newPassword + case currentPassword + case oneTimeCode + case organizationTitle case organization - case streetAddress = "street-address" - case addressLine1 = "address-line-1" - case addressLine2 = "address-line-2" - case addressLine3 = "address-line-3" - case addressLevel1 = "address-level-1" - case addressLevel2 = "address-level-2" - case addressLevel3 = "address-level-3" - case addressLevel4 = "address-level-4" + case streetAddress + case addressLine1 + case addressLine2 + case addressLine3 + case addressLevel1 + case addressLevel2 + case addressLevel3 + case addressLevel4 case country - case country_name = "country-name" - case postal_code = "postal-code" - case ccName = "cc-name" - case ccGivenName = "cc-given-name" - case ccFamilyName = "cc-family-name" - case ccNumber = "cc-number" - case ccExp = "cc-exp" - case ccExpMonth = "cc-exp-month" - case ccExpYear = "cc-exp-year" - case ccCSC = "cc-csc" - case ccType = "cc-type" - case transactionCurrency = "transaction-currency" - case transactionAmount = "transaction-amount" + case country_name + case postal_code + case ccName + case ccGivenName + case ccFamilyName + case ccNumber + case ccExp + case ccExpMonth + case ccExpYear + case ccCSC + case ccType + case transactionCurrency + case transactionAmount case language - case birthDay = "bday" - case birthDayDay = "bday-day" - case birthDayMonth = "bday-month" - case birthDayYear = "bday-year" + case birthDay + case birthDayDay + case birthDayMonth + case birthDayYear case sex - case telephone = "tel" - case telephoneCountryCode = "tel-country-code" - case telephoneNational = "tel-national" - case telephoneAreaCode = "tel-area-code" - case telephoneLocal = "tel-local" - case telephoneExtension = "tel-extension" - case instantMessagingProtocolEndpoint = "impp" + case telephone + case telephoneCountryCode + case telephoneNational + case telephoneAreaCode + case telephoneLocal + case telephoneExtension + case instantMessagingProtocolEndpoint case url case photo } @@ -172,3 +172,59 @@ public extension FormInputOption.Autocapitalize { } } #endif + +public extension FormInputOption.Autocomplete { + /// The token HTML's `autocomplete` attribute takes for this value + /// + /// ```leaf + /// + /// ``` + /// + /// Only an HTML surface needs this. SwiftUI reads the case. + var htmlAttributeValue: String { + switch self { + case .honorificPrefix: "honorific-prefix" + case .givenName: "given-name" + case .additionalName: "additional-name" + case .familyName: "family-name" + case .honorificSuffix: "honorific-suffix" + case .newPassword: "new-password" + case .currentPassword: "current-password" + case .oneTimeCode: "one-time-code" + case .organizationTitle: "organization-title" + case .streetAddress: "street-address" + case .addressLine1: "address-line-1" + case .addressLine2: "address-line-2" + case .addressLine3: "address-line-3" + case .addressLevel1: "address-level-1" + case .addressLevel2: "address-level-2" + case .addressLevel3: "address-level-3" + case .addressLevel4: "address-level-4" + case .country_name: "country-name" + case .postal_code: "postal-code" + case .ccName: "cc-name" + case .ccGivenName: "cc-given-name" + case .ccFamilyName: "cc-family-name" + case .ccNumber: "cc-number" + case .ccExp: "cc-exp" + case .ccExpMonth: "cc-exp-month" + case .ccExpYear: "cc-exp-year" + case .ccCSC: "cc-csc" + case .ccType: "cc-type" + case .transactionCurrency: "transaction-currency" + case .transactionAmount: "transaction-amount" + case .birthDay: "bday" + case .birthDayDay: "bday-day" + case .birthDayMonth: "bday-month" + case .birthDayYear: "bday-year" + case .telephone: "tel" + case .telephoneCountryCode: "tel-country-code" + case .telephoneNational: "tel-national" + case .telephoneAreaCode: "tel-area-code" + case .telephoneLocal: "tel-local" + case .telephoneExtension: "tel-extension" + case .instantMessagingProtocolEndpoint: "impp" + default: String(describing: self) + } + } +} diff --git a/Sources/FOSMVVM/Forms/FormInputType.swift b/Sources/FOSMVVM/Forms/FormInputType.swift index e42af19..7c04b24 100644 --- a/Sources/FOSMVVM/Forms/FormInputType.swift +++ b/Sources/FOSMVVM/Forms/FormInputType.swift @@ -20,13 +20,13 @@ /// > SwiftUI controls. /// /// - See also: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input -public enum FormInputType: String, Codable, Sendable { +public enum FormInputType: Codable, Sendable { case button case checkbox case color case date - case datetimeLocal = "datetime-local" - case emailAddress = "email" + case datetimeLocal + case emailAddress case file case hidden case image @@ -186,3 +186,20 @@ extension FormInputType { } } #endif + +public extension FormInputType { + /// The token HTML's `` takes for this input + /// + /// ```leaf + /// + /// ``` + /// + /// Only an HTML surface needs this. SwiftUI reads the case. + var htmlAttributeValue: String { + switch self { + case .datetimeLocal: "datetime-local" + case .emailAddress: "email" + default: String(describing: self) + } + } +} diff --git a/Sources/FOSMVVM/Localization/LocalizableString.swift b/Sources/FOSMVVM/Localization/LocalizableString.swift index 55cf2b5..b105ff8 100644 --- a/Sources/FOSMVVM/Localization/LocalizableString.swift +++ b/Sources/FOSMVVM/Localization/LocalizableString.swift @@ -60,6 +60,50 @@ public enum LocalizableString: Codable, Hashable, Localizable, Identifiable, Stu ) } + /// Localizes an enum case by the case itself — no raw value, no string + /// + /// ```swift + /// struct SimpleError: ServerRequestError { + /// enum ErrorCode: Codable, Sendable { + /// case serverFailed + /// case applicationFailed + /// + /// var message: LocalizableString { + /// .localized(case: self, parentType: SimpleError.self) + /// } + /// } + /// } + /// ``` + /// + /// ```yaml + /// en: + /// SimpleError: + /// ErrorCode: + /// serverFailed: "The server failed" + /// applicationFailed: "The application failed" + /// ``` + /// + /// The YAML key is the enum's type name under `parentType`, and the leaf + /// is the case name. Use it on enums without associated values — a case + /// carrying a payload has no single key. + /// + /// - Parameters: + /// - enumCase: The case to localize (`self`, from inside the enum) + /// - parentType: The type the enum is nested in (default: none) + /// - parentKeys: Intermediate YAML keys between the parent and the enum (default: none) + /// - index: A position, when the localized value is one of a list (default: none) + public static func localized(case enumCase: some Any, parentType: Any.Type? = nil, parentKeys: String..., index: Int? = nil) -> Self { + .localized( + .init( + for: type(of: enumCase), + parentType: parentType, + parentKeys: parentKeys, + propertyName: String(describing: enumCase), + index: index + ) + ) + } + public static func localized(for type: (some Any).Type, propertyName: String, messageGroup: String? = nil, messageKey: String) -> Self { .localized( .init( diff --git a/Sources/FOSMVVM/Localization/LocalizedProperty.swift b/Sources/FOSMVVM/Localization/LocalizedProperty.swift index f91ff65..7931dc1 100644 --- a/Sources/FOSMVVM/Localization/LocalizedProperty.swift +++ b/Sources/FOSMVVM/Localization/LocalizedProperty.swift @@ -94,12 +94,12 @@ public enum LocalizedPropertyError: Error, CustomDebugStringConvertible { // // ```swift // struct ParentViewModel: ViewModel { -// enum NestedEnum: String { +// enum NestedEnum { // case option1 // case option2 // // var display: LocalizableString { -// .localized(.init(for: Self.self, parentType: ParentViewModel.self, propertyName: rawValue)) +// .localized(case: self, parentType: ParentViewModel.self) // } // } // } diff --git a/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift b/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift index 5ce19f1..b3c8e46 100644 --- a/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift +++ b/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift @@ -48,7 +48,7 @@ public struct CredentialRejectedError: ServerRequestError { /// has no `ClientCredentialProvider` configured (or it returned no headers). /// `.invalid` — a credential was presented and the server's verifier refused /// it; refresh the credential and retry. - public enum Code: String, Codable, Sendable { + public enum Code: Codable, Sendable { case missing case invalid } diff --git a/Sources/FOSMVVM/Protocols/ServerRequest.swift b/Sources/FOSMVVM/Protocols/ServerRequest.swift index 883230b..b49a557 100644 --- a/Sources/FOSMVVM/Protocols/ServerRequest.swift +++ b/Sources/FOSMVVM/Protocols/ServerRequest.swift @@ -228,7 +228,7 @@ public extension ServerRequest { } /// A `ServerRequestAction` tells the server how to handle the data that is submitted -public enum ServerRequestAction: String, Codable, CaseIterable, Hashable, Sendable { +public enum ServerRequestAction: Codable, CaseIterable, Hashable, Sendable { /// Retrieve the requested information /// /// - Note: Creates a **GET** HTTP Request @@ -443,12 +443,12 @@ public enum ServerRequestBodySize: Equatable, Hashable, Sendable { /// let code: ErrorCode /// let message: LocalizableString /// -/// enum ErrorCode: String, Codable, Sendable { +/// enum ErrorCode: Codable, Sendable { /// case serverFailed /// case applicationFailed /// /// var message: LocalizableString { -/// .localized(for: Self.self, parentType: MyError.self, propertyName: rawValue) +/// .localized(case: self, parentType: MyError.self) /// } /// } /// diff --git a/Sources/FOSMVVM/Protocols/ServerRequestSort.swift b/Sources/FOSMVVM/Protocols/ServerRequestSort.swift index 07ad3ce..940c9e5 100644 --- a/Sources/FOSMVVM/Protocols/ServerRequestSort.swift +++ b/Sources/FOSMVVM/Protocols/ServerRequestSort.swift @@ -22,7 +22,7 @@ public protocol ServerRequestSort: Codable, Hashable, Sendable {} /// The sortable dimensions a container publishes to clients — *meanings*, never storage columns. /// /// ```swift -/// enum BerthSortKey: String, SortKey { case number, dockName, updatedAt } +/// enum CardSortKey: SortKey { case title, rank, updatedAt } /// ``` /// /// The server maps each dimension to one or more sort keypaths; the client only ever names a dimension, diff --git a/Sources/FOSMVVM/SwiftUI Support/FormFieldView.swift b/Sources/FOSMVVM/SwiftUI Support/FormFieldView.swift index 45f0cf1..de2feb4 100644 --- a/Sources/FOSMVVM/SwiftUI Support/FormFieldView.swift +++ b/Sources/FOSMVVM/SwiftUI Support/FormFieldView.swift @@ -380,7 +380,7 @@ private extension FormFieldView where Value == String { #endif default: - Text("The FormInputType \(inputType.rawValue) is NYI!") + Text("The FormInputType \(inputType) is NYI!") } default: @@ -493,7 +493,7 @@ private extension FormFieldView where Value == String? { #endif default: - Text("The FormInputType \(inputType.rawValue) is NYI!") + Text("The FormInputType \(inputType) is NYI!") } default: diff --git a/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift b/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift index aade383..04a23f3 100644 --- a/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift +++ b/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift @@ -222,3 +222,48 @@ struct LocalizableStringTests: LocalizableTestCase { ) } } + +/// `localized(case:parentType:)` — an enum case localizes by the case, with +/// no raw value and no string in the caller's code. +struct LocalizableStringCaseTests { + @Test func localizedCase_keyIsTypeAndCase() { + switch LocalizableString.localized(case: Owner.Choice.optionOne, parentType: Owner.self) { + case .empty, .constant: + #expect(Bool(false), "Expected localized") + case .localized(let ref): + switch ref { + case .value(let key): + #expect(key == "Owner.Choice.optionOne") + case .arrayValue: + #expect(Bool(false), "Expected .value") + } + } + } + + @Test func localizedCase_noParent() { + switch LocalizableString.localized(case: Owner.Choice.optionTwo) { + case .empty, .constant: + #expect(Bool(false), "Expected localized") + case .localized(let ref): + switch ref { + case .value(let key): + #expect(key == "Choice.optionTwo") + case .arrayValue: + #expect(Bool(false), "Expected .value") + } + } + } + + @Test func localizedCase_matchesTheStringForm() { + let byCase = LocalizableString.localized(case: Owner.Choice.optionOne, parentType: Owner.self) + let byName = LocalizableString.localized(for: Owner.Choice.self, parentType: Owner.self, propertyName: "optionOne") + #expect(byCase == byName) + } +} + +private enum Owner { + enum Choice: Codable, Sendable { + case optionOne + case optionTwo + } +} diff --git a/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift b/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift index 1b6e9bc..c9d91d8 100644 --- a/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift +++ b/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift @@ -48,7 +48,7 @@ struct CredentialRejectedErrorTests { #"{"error":true,"reason":"Unauthorized"}"#, #""Invalid bearer credential""#, "{}", - #"{"__fosServerError":"someOtherError","code":"invalid"}"# + #"{"__fosServerError":"someOtherError","code":{"invalid":{}}}"# ] { let decoded: CredentialRejectedError? = try? body.fromJSON() #expect(decoded == nil, "must not decode from: \(body)") @@ -57,7 +57,7 @@ struct CredentialRejectedErrorTests { @Test("Unknown code value is rejected") func unknownCode() { - let body = #"{"__fosServerError":"credentialRejected","code":"bogus"}"# + let body = #"{"__fosServerError":"credentialRejected","code":{"bogus":{}}}"# let decoded: CredentialRejectedError? = try? body.fromJSON() #expect(decoded == nil) } @@ -66,7 +66,7 @@ struct CredentialRejectedErrorTests { func forwardCompat() throws { // INTERNAL representation pin (golden blob). The ONE place the envelope // shape is asserted — see the maintainer comment beside CodingKeys. - let committedWireForm = #"{"__fosServerError":"credentialRejected","code":"invalid"}"# + let committedWireForm = #"{"__fosServerError":"credentialRejected","code":{"invalid":{}}}"# let decoded: CredentialRejectedError = try committedWireForm.fromJSON() #expect(decoded.code == .invalid) } From bb86eda48b0bf2f10854e602d3a74dabd024f9ac Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 22:53:13 +0200 Subject: [PATCH 3/7] docs(fosmvvm-review): no-string-backed-enums exempts tools, not libraries --- .claude/skills/fosmvvm-review/checks/cross-cutting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/fosmvvm-review/checks/cross-cutting.md b/.claude/skills/fosmvvm-review/checks/cross-cutting.md index 24034d6..d5ca63f 100644 --- a/.claude/skills/fosmvvm-review/checks/cross-cutting.md +++ b/.claude/skills/fosmvvm-review/checks/cross-cutting.md @@ -61,7 +61,7 @@ enum ErrorCode: String, Codable, Sendable { // a public string door + a } } ``` -**Detection:** Flag every `enum … : String` (and `: Int` when the raw value is anything but an ordinal the type itself consumes) — public or internal, wire-crossing or not. The remedy is the plain enum with synthesized `Codable`; a case's localized text comes from `LocalizableString.localized(case:parentType:)`, which derives the YAML key from the case with no string in user code. Exempt: `CodingKeys` (Swift's own coding contract) and a `String`-backed enum whose raw value is consumed only by a system API that demands `RawRepresentable` — name the API in the finding when this exemption is claimed, and treat a `rawValue` read anywhere else as the hole. When the framework pin predates `localized(case:parentType:)` (shipped 0.16.0), report as correct at time of writing, now fixable. +**Detection:** Flag every `enum … : String` (and `: Int` when the raw value is anything but an ordinal the type itself consumes) — public or internal, wire-crossing or not. The remedy is the plain enum with synthesized `Codable`; a case's localized text comes from `LocalizableString.localized(case:parentType:)`, which derives the YAML key from the case with no string in user code. Exempt: `CodingKeys` (Swift's own coding contract); a `String`-backed enum whose raw value is consumed only by a system API that demands `RawRepresentable` — name the API in the finding when this exemption is claimed, and treat a `rawValue` read anywhere else as the hole; and **tools, not libraries** (ruled 2026-09-02) — a CLI's argument and config-file tokens (`--shape clientServer`, a bootstrap config's `"shape"`) are typed by a human at a prompt, not carried on a wire or shown to a user, so the rule does not reach them. When the framework pin predates `localized(case:parentType:)` (shipped 0.16.0), report as correct at time of writing, now fixable. ## Check: status-interpreted-as-result **Severity:** blocker From c0379b492e9d114da36bf230300fc9e4c303def5 Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 23:04:52 +0200 Subject: [PATCH 4/7] docs(deferrals): CredentialRejectedError's localized message waits on a framework YAML bundle --- docs/deferrals.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/deferrals.md b/docs/deferrals.md index fc9faa7..1ebf71f 100644 --- a/docs/deferrals.md +++ b/docs/deferrals.md @@ -40,3 +40,13 @@ Work items acknowledged and deliberately not done yet. Each entry names the evid **Why it was deferred:** placing a field at a fixed offset above the keyboard's top is device- and keyboard-height-dependent, so a deterministic fixture needs layout that measures the keyboard at runtime — more machinery than the round's scope. The failure mode is guarded by an arbiter, not by geometry, so the fix does not silently depend on the un-pinned case. **What reopens it:** a regression report where the menu-rise re-scroll fails on a margin-occluded field; or the next probe-fixture round, where a runtime-measured margin field should join the composite card so all three geometries are forced in-house. + +## `CredentialRejectedError` has no user-presentable localized message + +**Recorded:** 2026-09-02, at David's direction, during the credential-rejection redesign. + +**What it is:** the rejection carries typed data (`reason`, `challenge`) but no `LocalizableError` conformance, so `.alert(error:)` presents its debug description rather than a sentence in the user's language. The shape that would fix it is the canonical one — `@LocalizableError` with a `@LocalizedSubs` message substituting the reason and the challenge's realm, resolved by the server's localizing encoder so the client decodes it already localized. + +**Why it was deferred:** the message needs YAML at the server's localization store, and FOSUtilities ships no localization YAML of its own today. A framework-owned bundle is its own design: how it reaches the store the app initialized (`initYamlLocalization(bundle:resourceDirectoryName:)` takes one bundle), and whether an app's YAML may override the framework's words. The substituted message rides on that design, not ahead of it. + +**What reopens it:** the framework-localization-bundle design; or a second framework-owned error that needs a user-facing message, at which point the bundle stops being a one-type question. From 0c0b6e98a3a1c2f6dba6f5347c7ff7d82d9af824 Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 23:19:03 +0200 Subject: [PATCH 5/7] feat(FOSMVVM): CredentialRejectedError is plain data; CredentialChallenge; one typed error envelope on the wire CredentialRejectedError carries reason (Reason.missing/.invalid) and a typed CredentialChallenge (.bearer, .bearerRealm, .basicRealm) with synthesized Codable; the hand-rolled discriminator envelope is gone. Every ServerRequest error body now crosses the wire inside the WireError envelope, encoded by FOSMVVMVapor.ErrorMiddleware and decoded by the client and the FOSTestingVapor harness, so the client never trial-decodes a body. The rejection no longer conforms to AbortError: ErrorMiddleware assigns its 401 and WWW-Authenticate (the error token follows the reason per RFC 6750). The stale DocC that taught hand-rolled retry is rewritten around the refresh seam. --- .claude/skills/shared/api-catalog/FOSMVVM.md | 10 +- .../skills/shared/api-catalog/FOSMVVMVapor.md | 2 +- CHANGELOG.md | 20 ++++ .../Protocols/ClientCredentialProvider.swift | 4 +- .../Protocols/CredentialChallenge.swift | 45 +++++++++ .../Protocols/CredentialRejectedError.swift | 91 ++++++------------- Sources/FOSMVVM/Protocols/WireError.swift | 28 ++---- .../SSEInvalidationChannel.swift | 2 +- .../CredentialRejectedError+Vapor.swift | 36 +++++--- .../ClientCredentialMiddleware.swift | 14 ++- .../Middleware/ErrorMiddleware.swift | 53 +++++++++++ .../ClientCredentialProviderTests.swift | 4 +- .../CredentialRejectedErrorTests.swift | 46 +++------- .../Protocols/WireErrorTests.swift | 34 +++---- .../ClientCredentialMiddlewareTests.swift | 78 +++++++--------- .../ErrorMiddlewareDressingTests.swift | 14 ++- .../GroupMountedRegistrationTests.swift | 4 +- .../ClientCredentialRoundTripTests.swift | 4 +- .../TestingServerRequestResponseTests.swift | 2 +- 19 files changed, 277 insertions(+), 214 deletions(-) create mode 100644 Sources/FOSMVVM/Protocols/CredentialChallenge.swift diff --git a/.claude/skills/shared/api-catalog/FOSMVVM.md b/.claude/skills/shared/api-catalog/FOSMVVM.md index 6b6c500..c207693 100644 --- a/.claude/skills/shared/api-catalog/FOSMVVM.md +++ b/.claude/skills/shared/api-catalog/FOSMVVM.md @@ -363,12 +363,16 @@ final class UserViewModelRequest: ViewModelRequest, @unchecked Sendable { Reach for this when: a request to a protected route may be rejected before the operation runs — catch the typed error (`.missing` = no credential configured; `.invalid` = presented and refused → refresh and retry, safe because the -operation never ran); never branch on an HTTP status. It always throws to the -caller (never `requestErrorHandler`). +operation never ran); never branch on an HTTP status. The client first offers +it to `credentialHeaders(afterRejection:)` and retries once on fresh headers; +only an unrecovered rejection throws to the caller (never +`requestErrorHandler`). It carries the server's typed `CredentialChallenge` +(`.bearer`, `.bearerRealm(_:)`, `.basicRealm(_:)`) — the same value the +transport rendered as `WWW-Authenticate`. ```swift } catch let error as CredentialRejectedError { - switch error.code { + switch error.reason { case .missing: ... // check the MVVMEnvironment's clientCredentialProvider case .invalid: ... // refresh the credential and retry } diff --git a/.claude/skills/shared/api-catalog/FOSMVVMVapor.md b/.claude/skills/shared/api-catalog/FOSMVVMVapor.md index 026f792..5792c23 100644 --- a/.claude/skills/shared/api-catalog/FOSMVVMVapor.md +++ b/.claude/skills/shared/api-catalog/FOSMVVMVapor.md @@ -197,7 +197,7 @@ decodes them back into the ServerRequest's typed `ResponseError` and throws them in context (form validation, for example); other errors degrade to status + reason, hiding details in release builds. An error that is both `Encodable` and `AbortError` is served with its typed body and its own status -and headers (e.g. `CredentialRejectedError` → 401 + `WWW-Authenticate`). +and headers; every `ServerRequestError` body rides inside the one typed envelope the client decodes, and a `CredentialRejectedError` is dressed here as 401 + `WWW-Authenticate` (the rejection itself is plain data, not an `AbortError`). Don't keep Vapor's stock ErrorMiddleware — it flattens typed ResponseErrors into plain-text reasons the client cannot decode. diff --git a/CHANGELOG.md b/CHANGELOG.md index 81c0b03..9e6caa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`CredentialChallenge`** — what a server demands of a credential, typed: `.bearer`, + `.bearerRealm(_:)`, `.basicRealm(_:)`. A `ServerCredentialVerifier` attaches it to + the rejection it throws; the transport renders `WWW-Authenticate` from it (the + error token follows the rejection's reason, per RFC 6750), and the client reads + the same typed value on the decoded error. + - **`LocalizableString.localized(case:parentType:)`** — localizes an enum case by the case itself: the YAML key is the enum's type under its parent and the leaf is the case name, with no string in user code. The replacement for feeding a @@ -22,6 +28,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **`CredentialRejectedError` is plain data with synthesized `Codable`** — `reason` + (`Reason.missing` / `.invalid`, replacing `code`/`Code`) and `challenge` + (`CredentialChallenge?`, now carried across the wire). The hand-rolled + discriminator envelope is gone. **Every `ServerRequest` error body now crosses + the wire inside one typed envelope** encoded by `FOSMVVMVapor.ErrorMiddleware` + and decoded by the client and the `FOSTestingVapor` harness, so the client + never trial-decodes a body. Wire contract: a client and server on either side + of this release see each other's error bodies as undecodable and fall to the + status path; upgrade both together. The rejection no longer conforms to + Vapor's `AbortError`: its 401 and `WWW-Authenticate` are assigned by + `FOSMVVMVapor.ErrorMiddleware`, the one place an error becomes a response. A + server that never installed it — a shape the review already blocks — now + answers a rejection with Vapor's stock 500 instead of a plain 401; the + pre-envelope skew fallback that relied on that plain 401 is retired with it. - **The framework's own enums drop their `String` raw values** — `ServerRequestAction`, `FormInputType`, `FormInputOption.Autocapitalize` and `.Autocomplete`, and `CredentialRejectedError.Code` are plain enums with diff --git a/Sources/FOSMVVM/Protocols/ClientCredentialProvider.swift b/Sources/FOSMVVM/Protocols/ClientCredentialProvider.swift index 4aac80f..15b4f6a 100644 --- a/Sources/FOSMVVM/Protocols/ClientCredentialProvider.swift +++ b/Sources/FOSMVVM/Protocols/ClientCredentialProvider.swift @@ -70,7 +70,7 @@ public protocol ClientCredentialProvider: Sendable { /// /// ```swift /// func credentialHeaders(afterRejection: CredentialRejectedError) async -> [(field: String, value: String)]? { - /// guard afterRejection.code == .invalid else { return nil } + /// guard afterRejection.reason == .invalid else { return nil } /// guard let token = await SessionStore.shared.refreshAccessToken() else { return nil } /// return [(field: "Authorization", value: "Bearer \(token)")] /// } @@ -87,7 +87,7 @@ public protocol ClientCredentialProvider: Sendable { /// several times in quick succession with the same rejection. /// - Parameter afterRejection: The rejection the server returned, so a provider /// can distinguish a missing credential from an invalid one. A refused - /// ServerRequest carries the server's actual code; a refused live-channel + /// ServerRequest carries the server's actual reason; a refused live-channel /// reconnect cannot read it and always presents `.invalid`, and may call /// this once per failed reconnect rather than only once. /// - Returns: Headers to retry the request with once, or `nil` when no fresh diff --git a/Sources/FOSMVVM/Protocols/CredentialChallenge.swift b/Sources/FOSMVVM/Protocols/CredentialChallenge.swift new file mode 100644 index 0000000..015da4e --- /dev/null +++ b/Sources/FOSMVVM/Protocols/CredentialChallenge.swift @@ -0,0 +1,45 @@ +// CredentialChallenge.swift +// +// Copyright 2026 FOS Computer Services, LLC +// +// Licensed under the Apache License, Version 2.0 (the License); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import Foundation + +/// What a server demands of a credential — one case per authentication scheme +/// +/// A ``ServerCredentialVerifier`` attaches it to the ``CredentialRejectedError`` +/// it throws; the transport renders it as the response's `WWW-Authenticate` +/// header, and the client receives the same typed value on the decoded error: +/// +/// ```swift +/// throw CredentialRejectedError(reason: .missing, challenge: .bearer) +/// throw CredentialRejectedError(reason: .invalid, challenge: .bearerRealm("api")) +/// ``` +/// +/// The header's error token is not part of the challenge — it follows from the +/// rejection's ``CredentialRejectedError/Reason``, so a challenge can never +/// contradict the reason it accompanies. +/// +/// > Note: A scheme this enum lacks is a case to add, with its parameters +/// > typed. There is no free-form case. +public enum CredentialChallenge: Codable, Sendable, Equatable { + /// `Bearer` (RFC 6750), one protection space + case bearer + + /// `Bearer` (RFC 6750), a named protection space + case bearerRealm(String) + + /// `Basic` (RFC 7617); the scheme requires a named protection space + case basicRealm(String) +} diff --git a/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift b/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift index b3c8e46..0b43fc5 100644 --- a/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift +++ b/Sources/FOSMVVM/Protocols/CredentialRejectedError.swift @@ -20,89 +20,54 @@ import Foundation /// /// Routes grouped behind `ClientCredentialMiddleware` verify the presented /// credential before the operation runs. When verification rejects the -/// request, this error crosses the wire and is rethrown by -/// ``ServerRequest/processRequest(mvvmEnv:)`` — catch it to recover: +/// request, this error crosses the wire; the client first offers it to its +/// ``ClientCredentialProvider/credentialHeaders(afterRejection:)`` — a provider +/// that returns fresh headers has the request retried once, and the caller +/// never sees the rejection. Only an unrecovered rejection is rethrown by +/// ``ServerRequest/processRequest(mvvmEnv:)``: /// /// ```swift /// do { /// try await request.processRequest(mvvmEnv: mvvmEnv) -/// } catch let error as CredentialRejectedError { -/// switch error.code { +/// } catch let rejection as CredentialRejectedError { +/// switch rejection.reason { /// case .missing: break // no credential was presented — check the /// // MVVMEnvironment's clientCredentialProvider -/// case .invalid: break // presented but refused — refresh the credential -/// // and retry (safe: the operation never ran) +/// case .invalid: break // presented and refused, and the provider could +/// // not refresh — sign the user in again /// } /// } /// ``` /// -/// The rejection happens **before** the operation runs, so retrying after +/// The rejection happens **before** the operation runs, so a retry after /// recovery never duplicates the operation's effects. /// -/// This error always throws to the call site — it is never routed to -/// ``MVVMEnvironment/requestErrorHandler``. -public struct CredentialRejectedError: ServerRequestError { - /// Why the credential seam rejected the request - /// - /// `.missing` — no credential accompanied the request; typically the client - /// has no `ClientCredentialProvider` configured (or it returned no headers). - /// `.invalid` — a credential was presented and the server's verifier refused - /// it; refresh the credential and retry. - public enum Code: Codable, Sendable { +/// > Note: A rejection that reaches the call site is never routed to +/// > ``MVVMEnvironment/requestErrorHandler``. +public struct CredentialRejectedError: ServerRequestError, Equatable { + /// Why the credential seam refused the request + public enum Reason: Codable, Sendable, Equatable { + /// No credential accompanied the request; typically the client has + /// no `ClientCredentialProvider` configured, or it returned no headers case missing + + /// A credential was presented and the server's verifier refused it case invalid } - /// Why the request was rejected - public let code: Code + public let reason: Reason - /// The authentication challenge the verifier answers with (for example - /// `"Bearer"`), used server-side to dress the response's - /// `WWW-Authenticate` header. Never crosses the wire — always `nil` on - /// a decoded value. - public let challenge: String? + /// What the server demands — rendered to `WWW-Authenticate` by the + /// transport, and readable here on the client as the same typed value + public let challenge: CredentialChallenge? - /// Creates the rejection thrown by a `ServerCredentialVerifier` + /// Creates the rejection a ``ServerCredentialVerifier`` throws /// /// - Parameters: - /// - code: Why the request was rejected - /// - challenge: The scheme for the response's `WWW-Authenticate` - /// header (default: none) - public init(code: Code, challenge: String? = nil) { - self.code = code + /// - reason: Why the request was refused + /// - challenge: What the server demands (default: none) + public init(reason: Reason, challenge: CredentialChallenge? = nil) { + self.reason = reason self.challenge = challenge } - - // swiftformat:disable docComments - // Wire envelope — INTERNAL detail; never publish in DocC/CHANGELOG/README. - // The discriminator key + fixed value make the decode strict: init(from:) - // fails unless both match, so nothing puns into (or out of) this type. - // Shape pinned by CredentialRejectedErrorTests.forwardCompat. - private enum CodingKeys: String, CodingKey { - case discriminator = "__fosServerError" - case code - } - - // swiftformat:enable docComments - - private static let discriminatorValue = "credentialRejected" - - public init(from decoder: Decoder) throws { - let container = try decoder.container(keyedBy: CodingKeys.self) - guard try container.decode(String.self, forKey: .discriminator) == Self.discriminatorValue else { - throw DecodingError.dataCorruptedError( - forKey: .discriminator, - in: container, - debugDescription: "Not a credential rejection" - ) - } - self.code = try container.decode(Code.self, forKey: .code) - self.challenge = nil - } - - public func encode(to encoder: Encoder) throws { - var container = encoder.container(keyedBy: CodingKeys.self) - try container.encode(Self.discriminatorValue, forKey: .discriminator) - try container.encode(code, forKey: .code) - } } diff --git a/Sources/FOSMVVM/Protocols/WireError.swift b/Sources/FOSMVVM/Protocols/WireError.swift index c5f8e43..11a7e94 100644 --- a/Sources/FOSMVVM/Protocols/WireError.swift +++ b/Sources/FOSMVVM/Protocols/WireError.swift @@ -16,25 +16,15 @@ import Foundation -// swiftformat:disable docComments -// The client-side decode order for a ServerRequest error body: the well-known -// surface errors (closed, FOS-owned list — CredentialRejectedError today) are -// tried STRICTLY before the request's own ResponseError. Passed to DataFetch -// as the ONE existing `errorType:` — FOSFoundation stays untouched; add a -// future surface error HERE, nowhere else. -// `package`: the decode chain is defined once here and consumed by both the -// client fetch path (FOSMVVM) and the test harness (FOSTestingVapor) — spec §3.4. -package enum WireError: Error, Decodable { +// The wire form of every ServerRequest error body: exactly one of the +// well-known surface errors (closed, FOS-owned list — CredentialRejectedError +// today) or the request's own ResponseError. The server's ErrorMiddleware +// encodes it; the client fetch path and the FOSTestingVapor harness decode it. +// Synthesized Codable keys the body by case, so the discrimination lives here +// and never inside a payload. Add a future surface error HERE, nowhere else. +// `package`: one definition, consumed by FOSMVVM, FOSMVVMVapor, and +// FOSTestingVapor. +package enum WireError: Error, Codable { case surface(CredentialRejectedError) case response(E) - - // swiftformat:enable docComments - package init(from decoder: Decoder) throws { - let container = try decoder.singleValueContainer() - if let rejection = try? container.decode(CredentialRejectedError.self) { - self = .surface(rejection) - } else { - self = try .response(container.decode(E.self)) - } - } } diff --git a/Sources/FOSMVVM/SwiftUI Support/SSEInvalidationChannel.swift b/Sources/FOSMVVM/SwiftUI Support/SSEInvalidationChannel.swift index 10578a8..2be3379 100644 --- a/Sources/FOSMVVM/SwiftUI Support/SSEInvalidationChannel.swift +++ b/Sources/FOSMVVM/SwiftUI Support/SSEInvalidationChannel.swift @@ -133,7 +133,7 @@ struct SSEInvalidationChannel: InvalidationChannel { // `credentialHeaders()` — the channel carries no credential state of its own. if http.statusCode == 401 { _ = await credentialProvider?.credentialHeaders( - afterRejection: CredentialRejectedError(code: .invalid) + afterRejection: CredentialRejectedError(reason: .invalid) ) } diff --git a/Sources/FOSMVVMVapor/Extensions/CredentialRejectedError+Vapor.swift b/Sources/FOSMVVMVapor/Extensions/CredentialRejectedError+Vapor.swift index 03c3bf1..29c189c 100644 --- a/Sources/FOSMVVMVapor/Extensions/CredentialRejectedError+Vapor.swift +++ b/Sources/FOSMVVMVapor/Extensions/CredentialRejectedError+Vapor.swift @@ -17,23 +17,31 @@ import FOSMVVM import Vapor -/// Dresses the rejection for the transport: `401 Unauthorized` with the -/// verifier's authentication challenge (for example `WWW-Authenticate: -/// Bearer`). The response *body* remains the typed error — FOSMVVM clients -/// decode and rethrow it; the status exists for proxies, logs, and RFC 7235 -/// conformance, never for client branching. -extension CredentialRejectedError: AbortError { - public var status: HTTPResponseStatus { - .unauthorized - } +/// The transport dressing for a credential rejection: 401 Unauthorized with the +/// challenge rendered as WWW-Authenticate. Applied by ErrorMiddleware, which is +/// the one place a ServerRequestError becomes a Response — the rejection itself +/// is plain data and carries no Vapor conformance. The body remains the typed +/// error inside the WireError envelope; the status exists for proxies, logs, +/// and RFC 7235 conformance, never for client branching. +extension CredentialRejectedError { + static let transportStatus: HTTPResponseStatus = .unauthorized - public var headers: HTTPHeaders { + var transportHeaders: HTTPHeaders { guard let challenge else { return [:] } - return ["WWW-Authenticate": challenge] + return ["WWW-Authenticate": Self.headerValue(for: challenge, reason: reason)] } - public var reason: String { - // Constant — a rejection reason must never echo the presented credential - "Credential rejected" + /// RFC 7235 challenge text, rendered in exactly one place. The error token + /// follows the reason (RFC 6750 §3.1: no error token when no credential + /// was presented), so a challenge cannot contradict its rejection. + static func headerValue(for challenge: CredentialChallenge, reason: Reason) -> String { + switch challenge { + case .bearer: + reason == .invalid ? #"Bearer error="invalid_token""# : "Bearer" + case .bearerRealm(let realm): + reason == .invalid ? #"Bearer realm="\(realm)", error="invalid_token""# : #"Bearer realm="\(realm)""# + case .basicRealm(let realm): + #"Basic realm="\(realm)""# + } } } diff --git a/Sources/FOSMVVMVapor/Middleware/ClientCredentialMiddleware.swift b/Sources/FOSMVVMVapor/Middleware/ClientCredentialMiddleware.swift index 08bdedb..ec65fe9 100644 --- a/Sources/FOSMVVMVapor/Middleware/ClientCredentialMiddleware.swift +++ b/Sources/FOSMVVMVapor/Middleware/ClientCredentialMiddleware.swift @@ -23,7 +23,7 @@ import Vapor /// runs before each route in a protected group: return to admit the request, throw to /// reject it — throw ``CredentialRejectedError`` (carrying its code + challenge) to /// reject; any other thrown error is wrapped by the middleware into -/// `CredentialRejectedError(code: .invalid)`. Rejection reasons must **never** echo +/// `CredentialRejectedError(reason: .invalid)`. Rejection reasons must **never** echo /// the presented credential back to the caller. /// /// The verifier is consulted **per request**, so a credential revoked or rotated on the @@ -40,7 +40,7 @@ public protocol ServerCredentialVerifier: Sendable { /// - Parameter headers: The HTTP headers the request presented /// - Throws: To reject the request — throw ``CredentialRejectedError`` /// (carrying code + challenge); any other thrown error is wrapped by - /// the middleware into `CredentialRejectedError(code: .invalid)`. The + /// the middleware into `CredentialRejectedError(reason: .invalid)`. The /// rejection reason must not contain the presented credential func verify(headers: HTTPHeaders) async throws } @@ -93,8 +93,8 @@ public struct ClientCredentialMiddleware: AsyncMiddleware { } catch { // The verifier contract is "throw to reject" — any throw is a // rejection; custom verifiers throw CredentialRejectedError - // directly to carry richer intent (code, challenge). - throw CredentialRejectedError(code: .invalid) + // directly to carry richer intent (reason, challenge). + throw CredentialRejectedError(reason: .invalid) } return try await next.respond(to: request) @@ -132,19 +132,17 @@ public struct ClientCredentialMiddleware: AsyncMiddleware { /// } /// ``` public struct BearerCredentialVerifier: ServerCredentialVerifier { - private static let challenge = "Bearer" - private let isValid: @Sendable (String) async -> Bool // MARK: ServerCredentialVerifier Protocol public func verify(headers: HTTPHeaders) async throws { guard let token = headers.bearerAuthorization?.token else { - throw CredentialRejectedError(code: .missing, challenge: Self.challenge) + throw CredentialRejectedError(reason: .missing, challenge: .bearer) } guard await isValid(token) else { - throw CredentialRejectedError(code: .invalid, challenge: Self.challenge) + throw CredentialRejectedError(reason: .invalid, challenge: .bearer) } } diff --git a/Sources/FOSMVVMVapor/Middleware/ErrorMiddleware.swift b/Sources/FOSMVVMVapor/Middleware/ErrorMiddleware.swift index 37f14ed..8ed950c 100644 --- a/Sources/FOSMVVMVapor/Middleware/ErrorMiddleware.swift +++ b/Sources/FOSMVVMVapor/Middleware/ErrorMiddleware.swift @@ -58,6 +58,34 @@ public final class ErrorMiddleware: AsyncMiddleware { } } +private extension ErrorMiddleware { + /// Opens the existential so the envelope is generic over the error's own type; + /// returns Data because an opened type cannot escape into the result. + static func envelopeData(_ error: some ServerRequestError, encoder: JSONEncoder) throws -> Data { + try envelope(error).toJSONData(encoder: encoder) + } + + private static func envelope(_ error: E) -> WireError { + if let rejection = error as? CredentialRejectedError { + // `.surface` carries no E: the client decodes + // `WireError` and matches it regardless. + return .surface(rejection) + } + return .response(error) + } + + /// The transport dressing: the credential rejection is 401 + WWW-Authenticate; + /// any other ServerRequestError takes its own AbortError status when it has + /// one, else 400. + static func dressing(for error: any ServerRequestError) -> (status: HTTPResponseStatus, headers: HTTPHeaders) { + if let rejection = error as? CredentialRejectedError { + return (CredentialRejectedError.transportStatus, rejection.transportHeaders) + } + let abort = error as? any AbortError + return (abort?.status ?? .badRequest, abort?.headers ?? [:]) + } +} + public extension ErrorMiddleware { /// Create a default `ErrorMiddleware`. Logs errors to a `Logger` based on `Environment` /// and converts `Error` to `Response` based on conformance to `AbortError` and `Debuggable`. @@ -74,6 +102,31 @@ public extension ErrorMiddleware { // Inspect the error type and extract what data we can. switch error { + // A ServerRequestError crosses the wire inside the WireError envelope — + // the one body shape the client decodes. Its dressing (status, headers) + // is decided here, in the one place an error becomes a Response. + case let serverError as any ServerRequestError: + let dressing = Self.dressing(for: serverError) + do { + let encoder = try req.localizingEncoder + + (reason, errorData, status, headers, source) = try ( + "", + Self.envelopeData(serverError, encoder: encoder), + dressing.status, + dressing.headers, + .capture() + ) + } catch { + (reason, errorData, status, headers, source) = ( + "Error serializing ServerRequestError to JSON: \(error)", + nil, + dressing.status, + [:], + .capture() + ) + } + case let encodableAbort as any (Encodable & AbortError): do { let encoder = try req.localizingEncoder diff --git a/Tests/FOSMVVMTests/Protocols/ClientCredentialProviderTests.swift b/Tests/FOSMVVMTests/Protocols/ClientCredentialProviderTests.swift index 5ced15c..ac2216c 100644 --- a/Tests/FOSMVVMTests/Protocols/ClientCredentialProviderTests.swift +++ b/Tests/FOSMVVMTests/Protocols/ClientCredentialProviderTests.swift @@ -60,7 +60,7 @@ struct ClientCredentialProviderTests { let provider = BearerCredentialProvider { "abc" } let refreshed = await provider.credentialHeaders( - afterRejection: CredentialRejectedError(code: .invalid) + afterRejection: CredentialRejectedError(reason: .invalid) ) #expect(refreshed == nil) @@ -71,7 +71,7 @@ struct ClientCredentialProviderTests { let provider = RefreshingProvider(refreshedTo: "fresh") let refreshed = await provider.credentialHeaders( - afterRejection: CredentialRejectedError(code: .invalid) + afterRejection: CredentialRejectedError(reason: .invalid) ) #expect(refreshed?.count == 1) diff --git a/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift b/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift index c9d91d8..354fb84 100644 --- a/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift +++ b/Tests/FOSMVVMTests/Protocols/CredentialRejectedErrorTests.swift @@ -21,53 +21,35 @@ import Testing @Suite("CredentialRejectedError contract") struct CredentialRejectedErrorTests { - @Test("Round-trips through JSON: code preserved, challenge transient") + @Test("Round-trips through JSON with its reason and challenge") func roundTrip() throws { - let original = CredentialRejectedError(code: .invalid, challenge: "Bearer") + let original = CredentialRejectedError(reason: .invalid, challenge: .bearerRealm("api")) let decoded: CredentialRejectedError = try original.toJSON().fromJSON() - #expect(decoded.code == .invalid) - #expect(decoded.challenge == nil) // transient: never crosses the wire + #expect(decoded == original) } - @Test("Both codes round-trip") - func bothCodes() throws { - for code in [CredentialRejectedError.Code.missing, .invalid] { - let decoded: CredentialRejectedError = - try CredentialRejectedError(code: code).toJSON().fromJSON() - #expect(decoded.code == code) + @Test("Both reasons and every challenge round-trip") + func reasonsAndChallenges() throws { + let challenges: [CredentialChallenge?] = [nil, .bearer, .bearerRealm("api"), .basicRealm("api")] + for reason in [CredentialRejectedError.Reason.missing, .invalid] { + for challenge in challenges { + let original = CredentialRejectedError(reason: reason, challenge: challenge) + let decoded: CredentialRejectedError = try original.toJSON().fromJSON() + #expect(decoded == original) + } } } - @Test("Does NOT decode from bodies lacking the envelope") + @Test("A body that is not a rejection does not decode as one") func strictDecode() { - // Vapor's stock abort body, a plain reason string, an empty object, - // and a wrong discriminator VALUE must all fail — nothing puns into - // the rejection. for body in [ #"{"error":true,"reason":"Unauthorized"}"#, #""Invalid bearer credential""#, - "{}", - #"{"__fosServerError":"someOtherError","code":{"invalid":{}}}"# + "{}" ] { let decoded: CredentialRejectedError? = try? body.fromJSON() #expect(decoded == nil, "must not decode from: \(body)") } } - - @Test("Unknown code value is rejected") - func unknownCode() { - let body = #"{"__fosServerError":"credentialRejected","code":{"bogus":{}}}"# - let decoded: CredentialRejectedError? = try? body.fromJSON() - #expect(decoded == nil) - } - - @Test("Forward-compat: the committed wire form still decodes") - func forwardCompat() throws { - // INTERNAL representation pin (golden blob). The ONE place the envelope - // shape is asserted — see the maintainer comment beside CodingKeys. - let committedWireForm = #"{"__fosServerError":"credentialRejected","code":{"invalid":{}}}"# - let decoded: CredentialRejectedError = try committedWireForm.fromJSON() - #expect(decoded.code == .invalid) - } } diff --git a/Tests/FOSMVVMTests/Protocols/WireErrorTests.swift b/Tests/FOSMVVMTests/Protocols/WireErrorTests.swift index 6a8ae71..5d2979c 100644 --- a/Tests/FOSMVVMTests/Protocols/WireErrorTests.swift +++ b/Tests/FOSMVVMTests/Protocols/WireErrorTests.swift @@ -23,38 +23,40 @@ private struct StrictError: ServerRequestError { let errorCode: Int } -@Suite("WireError decode precedence") +@Suite("WireError envelope") struct WireErrorTests { - @Test("A rejection body decodes .surface — even when E is EmptyError") - func rejectionBeatsEmptyError() throws { - let rejection = try CredentialRejectedError(code: .invalid).toJSON() + @Test("A surface rejection round-trips, whatever E is") + func surfaceRoundTrips() throws { + let rejection = CredentialRejectedError(reason: .invalid, challenge: .bearer) + let encoded = try WireError.surface(rejection).toJSON() - let strict: WireError = try rejection.fromJSON() - guard case .surface(let error) = strict else { + let strict: WireError = try encoded.fromJSON() + guard case .surface(let decoded) = strict else { Issue.record("Expected .surface, got \(strict)"); return } - #expect(error.code == .invalid) + #expect(decoded == rejection) - // EmptyError decodes from ANYTHING — the wrapper must claim the - // rejection FIRST (this retires the documented swallow). - let permissive: WireError = try rejection.fromJSON() + // The envelope, not a trial decode, decides — so a permissive E + // (EmptyError decodes from anything) cannot swallow the rejection. + let permissive: WireError = try encoded.fromJSON() guard case .surface = permissive else { Issue.record("EmptyError swallowed the rejection"); return } } - @Test("A request-error body decodes .response") - func responseErrorPassesThrough() throws { - let wire: WireError = try #"{"errorCode":42}"#.fromJSON() + @Test("A request error round-trips as .response") + func responseRoundTrips() throws { + let encoded = try WireError.response(StrictError(errorCode: 42)).toJSON() + let wire: WireError = try encoded.fromJSON() guard case .response(let error) = wire else { Issue.record("Expected .response, got \(wire)"); return } #expect(error.errorCode == 42) } - @Test("A body matching neither type fails to decode") - func neitherFallsThrough() { - let wire: WireError? = try? #"{"unrelated":true}"#.fromJSON() + @Test("A bare error body — the pre-envelope form — does not decode") + func bareBodyFallsThrough() { + let wire: WireError? = try? #"{"errorCode":42}"#.fromJSON() #expect(wire == nil) } } diff --git a/Tests/FOSMVVMVaporTests/Middleware/ClientCredentialMiddlewareTests.swift b/Tests/FOSMVVMVaporTests/Middleware/ClientCredentialMiddlewareTests.swift index 8f233ad..8c39a5c 100644 --- a/Tests/FOSMVVMVaporTests/Middleware/ClientCredentialMiddlewareTests.swift +++ b/Tests/FOSMVVMVaporTests/Middleware/ClientCredentialMiddlewareTests.swift @@ -25,7 +25,9 @@ // • the verifier is consulted PER REQUEST, so a credential revoked between two // requests admits the first and rejects the second (rotation semantics, // mirroring the client side), -// • rejections carry `WWW-Authenticate: Bearer` (RFC 7235) on the wire, +// • rejections carry `WWW-Authenticate` (RFC 7235) on the wire — `Bearer` for a +// missing credential, `Bearer error="invalid_token"` (RFC 6750 §3.1) for a +// refused one — dressed by FOS `ErrorMiddleware.default`, never by the error, // • the Authorization parse edges are pinned: a lowercase `bearer` scheme admits; // an empty token (`Bearer `) rejects, // • the CLIENT-SIDE contract: through the REAL client (`processRequest(mvvmEnv:)`) @@ -44,7 +46,7 @@ // unchanged. import FOSFoundation -import FOSMVVM +@testable import FOSMVVM import FOSMVVMVapor import Foundation #if canImport(FoundationNetworking) @@ -98,8 +100,9 @@ struct ClientCredentialMiddlewareTests { #expect(reply.status == 401) #expect(!reply.body.contains(presentedToken)) - // RFC 7235: the challenge header reaches the wire - #expect(reply.wwwAuthenticate == "Bearer") + // RFC 7235: the challenge header reaches the wire; RFC 6750 §3.1: + // a presented-and-refused token carries the error token + #expect(reply.wwwAuthenticate == #"Bearer error="invalid_token""#) } } @@ -148,7 +151,7 @@ struct ClientCredentialMiddlewareTests { try await request.processRequest(mvvmEnv: env) Issue.record("Expected CredentialRejectedError, but the request succeeded") } catch let rejection as CredentialRejectedError { - #expect(rejection.code == .invalid) + #expect(rejection.reason == .invalid) } catch { Issue.record("Expected CredentialRejectedError, got \(error)") } @@ -169,7 +172,7 @@ struct ClientCredentialMiddlewareTests { try await request.processRequest(mvvmEnv: env) Issue.record("Expected CredentialRejectedError") } catch let rejection as CredentialRejectedError { - #expect(rejection.code == .missing) + #expect(rejection.reason == .missing) } catch { Issue.record("Expected CredentialRejectedError, got \(error)") } @@ -191,7 +194,7 @@ struct ClientCredentialMiddlewareTests { try await request.processRequest(mvvmEnv: env) Issue.record("Expected CredentialRejectedError, but the request succeeded") } catch let rejection as CredentialRejectedError { - #expect(rejection.code == .invalid) + #expect(rejection.reason == .invalid) } catch { Issue.record("Expected CredentialRejectedError, got \(error) — the swallow is back") } @@ -255,41 +258,13 @@ struct ClientCredentialMiddlewareTests { try await request.processRequest(mvvmEnv: env) Issue.record("Expected CredentialRejectedError") } catch let rejection as CredentialRejectedError { - #expect(rejection.code == .invalid) + #expect(rejection.reason == .invalid) } catch { Issue.record("Expected CredentialRejectedError, got \(error)") } } } - @Test("Skew fallback: a plain 401 without the envelope behaves as today") - func plain401WithoutEnvelopeFallsBack() async throws { - try await withRunningServer { app in - // Vapor's STOCK middleware — the old-server wire shape (no envelope) - let protected = app.grouped( - ClientCredentialMiddleware(verifier: BearerCredentialVerifier { _ in false }) - ) - try protected.register(collection: RoundTripController(actions: [ - .show: { _, _ in GrantedReply(message: "granted") } - ])) - } _: { base in - let env = Self.environment( - base: base, - provider: BearerCredentialProvider { "revoked-token" } - ) - - let request = ShowStrictErrorReplyRequest() - do { - try await request.processRequest(mvvmEnv: env) - Issue.record("Expected a failure") - } catch DataFetchError.badStatus(httpStatusCode: let code) { - #expect(code == 401) // pre-envelope fallback, unchanged - } catch { - Issue.record("Expected badStatus(401) fallback, got \(error)") - } - } - } - @Test("Under FOS ErrorMiddleware, a rejection body IS the typed CredentialRejectedError") func rejectionBodyIsTypedUnderFOSErrorMiddleware() async throws { try await withRunningServer { app in @@ -303,19 +278,28 @@ struct ClientCredentialMiddlewareTests { ) #expect(rejected.status == 401) // transport dressing - #expect(rejected.wwwAuthenticate == "Bearer") // RFC 7235 preserved - let typed: CredentialRejectedError = try rejected.body.fromJSON() - #expect(typed.code == .invalid) // the semantics + #expect(rejected.wwwAuthenticate == #"Bearer error="invalid_token""#) // RFC 6750 §3.1 + let typed: WireError = try rejected.body.fromJSON() + guard case .surface(let rejection) = typed else { + Issue.record("Expected the surface rejection, got \(typed)"); return + } + #expect(rejection.reason == .invalid) // the semantics + #expect(rejection.challenge == .bearer) // the typed challenge crosses let missing = try await Self.send(to: base, headers: [:]) - let missingTyped: CredentialRejectedError = try missing.body.fromJSON() - #expect(missingTyped.code == .missing) + let missingTyped: WireError = try missing.body.fromJSON() + guard case .surface(let missingRejection) = missingTyped else { + Issue.record("Expected the surface rejection, got \(missingTyped)"); return + } + #expect(missingRejection.reason == .missing) } } @Test("A custom ServerCredentialVerifier conformance is honored — the protocol seam") func customVerifierIsHonored() async throws { try await withRunningServer { app in + app.middleware = .init() + app.middleware.use(FOSMVVMVapor.ErrorMiddleware.default(environment: app.environment)) let protected = app.grouped( ClientCredentialMiddleware(verifier: ApiKeyVerifier(expectedKey: "the-key")) ) @@ -363,10 +347,16 @@ struct ClientCredentialMiddlewareTests { private extension ClientCredentialMiddlewareTests { /// Registers `GET /protected` behind a ``ClientCredentialMiddleware`` running the stock /// bearer verifier over `isValid`. + /// Every FOSMVVM server installs FOS `ErrorMiddleware.default` (the review's + /// `server-installs-the-error-middleware` blocker); it is what turns a + /// `CredentialRejectedError` into 401 + WWW-Authenticate. static func registerProtectedRoute( _ app: Application, isValid: @Sendable @escaping (String) async -> Bool ) { + app.middleware = .init() + app.middleware.use(FOSMVVMVapor.ErrorMiddleware.default(environment: app.environment)) + let protected = app.grouped( ClientCredentialMiddleware(verifier: BearerCredentialVerifier(isValid: isValid)) ) @@ -499,10 +489,8 @@ private actor TokenRegistry { } } -/// Same `.show` shape; its `ResponseError` cannot decode from a rejection body, -/// so under stock (non-FOS) middleware the raw 401 surfaces -/// (`plain401WithoutEnvelopeFallsBack`), and under FOS `ErrorMiddleware` the -/// typed rejection wins. +/// Same `.show` shape with a `ResponseError` that cannot decode from a rejection +/// body — the envelope, not a trial decode, is what makes the typed rejection win. private final class ShowStrictErrorReplyRequest: ServerRequest, @unchecked Sendable { typealias Query = EmptyQuery typealias Fragment = EmptyFragment diff --git a/Tests/FOSMVVMVaporTests/Middleware/ErrorMiddlewareDressingTests.swift b/Tests/FOSMVVMVaporTests/Middleware/ErrorMiddlewareDressingTests.swift index 39f7769..9c4fb20 100644 --- a/Tests/FOSMVVMVaporTests/Middleware/ErrorMiddlewareDressingTests.swift +++ b/Tests/FOSMVVMVaporTests/Middleware/ErrorMiddlewareDressingTests.swift @@ -16,7 +16,8 @@ // ErrorMiddleware transport-dressing contract: an error that is BOTH Encodable // and AbortError is served with its typed body AND its own status/headers; a -// plain Encodable error keeps the typed body with 400 (unchanged). +// plain Encodable error keeps the typed body with 400. Every ServerRequestError +// body rides inside the WireError envelope, the one shape the client decodes. import FOSFoundation import FOSMVVM @@ -64,7 +65,11 @@ struct ErrorMiddlewareDressingTests { // A single Content-Type on the wire — a duplicate would surface here // comma-joined by value(forHTTPHeaderField:) #expect(headers["Content-Type"] == "application/json; charset=utf-8") - let decoded: DressedError = try body.fromJSON() + // The body is the WireError envelope — the one shape the client decodes + let wire: WireError = try body.fromJSON() + guard case .response(let decoded) = wire else { + Issue.record("Expected .response, got \(wire)"); return + } #expect(decoded.errorCode == 7) } } @@ -84,7 +89,10 @@ struct ErrorMiddlewareDressingTests { // A single Content-Type on the wire — a duplicate would surface here // comma-joined by value(forHTTPHeaderField:) #expect(headers["Content-Type"] == "application/json; charset=utf-8") - let decoded: PlainEncodableError = try body.fromJSON() + let wire: WireError = try body.fromJSON() + guard case .response(let decoded) = wire else { + Issue.record("Expected .response, got \(wire)"); return + } #expect(decoded.errorCode == 9) } } diff --git a/Tests/FOSMVVMVaporTests/Middleware/GroupMountedRegistrationTests.swift b/Tests/FOSMVVMVaporTests/Middleware/GroupMountedRegistrationTests.swift index d9c9d0e..9a7c221 100644 --- a/Tests/FOSMVVMVaporTests/Middleware/GroupMountedRegistrationTests.swift +++ b/Tests/FOSMVVMVaporTests/Middleware/GroupMountedRegistrationTests.swift @@ -50,7 +50,7 @@ struct GroupMountedRegistrationTests { // No credential → the middleware rejects before the route runs. try await app.testing().test(TestViewModelRequest()) { response in #expect(response.status == .unauthorized) - #expect(response.credentialRejection?.code == .missing) // typed, not status alone + #expect(response.credentialRejection?.reason == .missing) // typed, not status alone #expect(response.body == nil) } @@ -109,7 +109,7 @@ struct GroupMountedRegistrationTests { ) try await app.testing().test(unauthed) { response in #expect(response.status == .unauthorized) - #expect(response.credentialRejection?.code == .missing) // typed, not status alone + #expect(response.credentialRejection?.reason == .missing) // typed, not status alone } // The record is untouched — the write never ran. let afterReject = try #require(try await Berth.find(berth.requireId(), on: db)) diff --git a/Tests/FOSMVVMVaporTests/Protocols/ClientCredentialRoundTripTests.swift b/Tests/FOSMVVMVaporTests/Protocols/ClientCredentialRoundTripTests.swift index b7e948b..3834f28 100644 --- a/Tests/FOSMVVMVaporTests/Protocols/ClientCredentialRoundTripTests.swift +++ b/Tests/FOSMVVMVaporTests/Protocols/ClientCredentialRoundTripTests.swift @@ -167,7 +167,7 @@ struct ClientCredentialRoundTripTests { try await request.processRequest(mvvmEnv: env) Issue.record("Expected the original rejection to be rethrown") } catch let rejection as CredentialRejectedError { - #expect(rejection.code == .invalid) + #expect(rejection.reason == .invalid) } catch { Issue.record("Expected CredentialRejectedError, got \(error)") } @@ -436,7 +436,7 @@ private struct RejectingCredentialProvider: ClientCredentialProvider { let refreshTally: RequestTally func credentialHeaders() async throws -> [(field: String, value: String)] { - throw CredentialRejectedError(code: .missing) + throw CredentialRejectedError(reason: .missing) } func credentialHeaders(afterRejection: CredentialRejectedError) async -> [(field: String, value: String)]? { diff --git a/Tests/FOSMVVMVaporTests/TestingServerRequestResponseTests.swift b/Tests/FOSMVVMVaporTests/TestingServerRequestResponseTests.swift index 2b7bbe5..6749d67 100644 --- a/Tests/FOSMVVMVaporTests/TestingServerRequestResponseTests.swift +++ b/Tests/FOSMVVMVaporTests/TestingServerRequestResponseTests.swift @@ -50,7 +50,7 @@ struct TestingServerRequestResponseTests { headers: ["Authorization": "Bearer nope"] ) { response in #expect(response.status == .unauthorized) // transport contract - #expect(response.credentialRejection?.code == .invalid) // the semantics + #expect(response.credentialRejection?.reason == .invalid) // the semantics #expect(response.error == nil) // EmptyError does NOT swallow #expect(response.body == nil) } From c00e51278f14405c68214c81d52273322b7022a1 Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 23:34:37 +0200 Subject: [PATCH 6/7] feat(FOSTesting): expectFullViewModelTests(version:); expectTranslations walks child ViewModels; strict test encoder; loud empty UI-test harness expectFullViewModelTests forwards a version: to the version-stability check. expectTranslations descends into stored child ViewModels, optionals, and collections and names the failing path. LocalizableTestCase's encoder is strict: a key the store cannot resolve fails the encode with LocalizerError.missingTranslation (JSONEncoder.localizingEncoder gains the strictLocalization switch; production encoding is unchanged). ViewModelViewTestCase.setUp(bundles:) throws RunError.noLocalizationYAML when the given bundles yield no YAML; bundles: [] opts into key-echo explicitly. --- .../skills/shared/api-catalog/FOSTesting.md | 15 ++- CHANGELOG.md | 17 +++ Sources/FOSMVVM/Extensions/JSONEncoder.swift | 17 ++- .../Localization/LocalizedProperty.swift | 12 ++ Sources/FOSMVVM/Localization/Localizer.swift | 6 + Sources/FOSTesting/LocalizableTestCase.swift | 75 +++++++++--- .../FOSTestingUI/ViewModelViewTestCase.swift | 21 +++- .../Localization/LocalizableStringTests.swift | 10 +- .../Localization/TranslationWalkTests.swift | 115 ++++++++++++++++++ .../FOSMVVMTests/TestYAML/TranslationWalk.yml | 17 +++ 10 files changed, 279 insertions(+), 26 deletions(-) create mode 100644 Tests/FOSMVVMTests/Localization/TranslationWalkTests.swift create mode 100644 Tests/FOSMVVMTests/TestYAML/TranslationWalk.yml diff --git a/.claude/skills/shared/api-catalog/FOSTesting.md b/.claude/skills/shared/api-catalog/FOSTesting.md index f1e589a..e40e126 100644 --- a/.claude/skills/shared/api-catalog/FOSTesting.md +++ b/.claude/skills/shared/api-catalog/FOSTesting.md @@ -64,10 +64,15 @@ calling the pieces individually. ```swift try expectFullViewModelTests(UserViewModel.self) +try expectFullViewModelTests(UserViewModel.self, version: .init("0.2.0")) // the project's own line try expectFullFieldValidationModelTests(UserFieldsMessages.self) try expectFullFormFieldTests(UserFormModel.emailField) ``` +`version:` (0.16.0) is forwarded to the version-stability check; pass the +project's version line when nothing calls `setCurrentVersion` under +`swift test`, where the default is `1.0.0`. + ### Codable round-trip check — `expectCodable()` Reach for this when: any Codable & Stubbable type must survive encode → decode (request bodies, queries, models) — it encodes the stub and decodes it back, @@ -95,7 +100,11 @@ try expectVersionedViewModel(UserViewModel.self, encoder: encoder()) Reach for this when: verifying no localized property is missing a YAML value — encodes the stub once per locale and fails on empty or still-pending values. Overloads take a ViewModel-like type or a single Localizable (a FormField -title, an error message). Included in `expectFullViewModelTests()`. +title, an error message). Included in `expectFullViewModelTests()`. It walks +stored child ViewModels, optionals, and collections (0.16.0) and names the +failing path (`rows[0].label`); and the suite's `encoder(locale:)` is strict — +a key the store cannot resolve fails the encode with +`LocalizerError.missingTranslation` rather than encoding `""`. ```swift try expectTranslations(UserViewModel.self) @@ -139,7 +148,9 @@ Reach for this when: XCUITest-driving a ViewModelView that only displays data no operations to verify. Create one project-level subclass that pins `setUp(bundle:resourceDirectoryName:appBundleIdentifier:locales:)` — or its `setUp(bundles:)` twin when the YAML lives in several bundles (the test -harness's own plus another target's resources), merged into one store; each test +harness's own plus another target's resources), merged into one store — bundles +that yield no YAML at all fail `setUp` with `RunError.noLocalizationYAML` +(0.16.0); `bundles: []` is the explicit opt-in to key-echo with no YAML; each test then calls `presentView()` with a stub ViewModel (localized for you; the suite's `localizationStore` and locale shorthands are available) and asserts on the returned XCUIApplication. `presentView(testConfiguration:)` names a diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e6caa1..c928da0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`expectFullViewModelTests(_:locales:version:)`** — the one-liner forwards a + `version:` to `expectVersionedViewModel`, so a project whose version line is not + `1.0.0` mints and checks baselines off its real line without hand-assembling + the three primitives. +- **`expectTranslations` walks child ViewModels** — stored child ViewModels, + optionals, and collections are descended, so a missing or blank translation on + a row fails the parent's pass and names the path (`rows[0].label`). +- **The test encoder is strict** — `LocalizableTestCase.encoder(locale:)` now + fails an encode on a key the store cannot resolve + (`LocalizerError.missingTranslation`) instead of encoding an empty string; + `JSONEncoder.localizingEncoder(locale:localizationStore:strictLocalization:)` + exposes the switch. Production encoding is unchanged. +- **A UI-test harness with no YAML fails loudly** — `setUp(bundles:)` throws + `RunError.noLocalizationYAML` when the bundles given yield no YAML; pass + `bundles: []` to run key-echo on purpose. Key-echo is for a missing key, not a + missing harness. + - **`CredentialChallenge`** — what a server demands of a credential, typed: `.bearer`, `.bearerRealm(_:)`, `.basicRealm(_:)`. A `ServerCredentialVerifier` attaches it to the rejection it throws; the transport renders `WWW-Authenticate` from it (the diff --git a/Sources/FOSMVVM/Extensions/JSONEncoder.swift b/Sources/FOSMVVM/Extensions/JSONEncoder.swift index 42eab42..2035b69 100644 --- a/Sources/FOSMVVM/Extensions/JSONEncoder.swift +++ b/Sources/FOSMVVM/Extensions/JSONEncoder.swift @@ -24,12 +24,17 @@ public extension JSONEncoder { /// - locale: The **Locale** to use to encode ``Localizable`` values /// - localizationStore: The ``LocalizationStore`` to use to resolve localization /// lookups during encoding + /// - strictLocalization: When **true**, a key the store cannot resolve fails the + /// encode with ``LocalizerError/missingTranslation(_:locale:)`` instead of + /// encoding an empty string. Test encoders use it so a missing key is red, never + /// a blank that ships (default: **false**) /// - Returns: A ``JSONEncoder`` that encodes ``Localizable`` values - static func localizingEncoder(locale: Locale, localizationStore: LocalizationStore) -> JSONEncoder { + static func localizingEncoder(locale: Locale, localizationStore: LocalizationStore, strictLocalization: Bool = false) -> JSONEncoder { let encoder = LocalizingEncoder() encoder.dateEncodingStrategy = .formatted(DateFormatter.JSONDateTimeFormatter) encoder.userInfo[.localeKey] = locale encoder.userInfo[.localizationStoreKey] = localizationStore + encoder.userInfo[.strictLocalizationKey] = strictLocalization return encoder } } @@ -62,10 +67,14 @@ extension Encoder { throw LocalizerError.localizationStoreMissing } - return try locale.localize( + let localized = try locale.localize( localizable, localizationStore: localizationStore ) + if localized == nil, userInfo[.strictLocalizationKey] as? Bool == true { + throw LocalizerError.missingTranslation(String(describing: localizable), locale: locale.identifier) + } + return localized } /// Converts the ``Localizable`` into an **Array** of *Element*s @@ -382,6 +391,10 @@ private extension CodingUserInfoKey { CodingUserInfoKey(rawValue: "_*LoCalIzAtIon_sTore*_")! } + static var strictLocalizationKey: CodingUserInfoKey { + CodingUserInfoKey(rawValue: "_*LoCalIzAtIon_sTrIcT*_")! + } + /// The properties of the model currently being processed static var propertyNamesKey: CodingUserInfoKey { CodingUserInfoKey(rawValue: "_*LoCalIzAtIon_pRoPerTy_NamEs*_")! diff --git a/Sources/FOSMVVM/Localization/LocalizedProperty.swift b/Sources/FOSMVVM/Localization/LocalizedProperty.swift index 7931dc1..483f752 100644 --- a/Sources/FOSMVVM/Localization/LocalizedProperty.swift +++ b/Sources/FOSMVVM/Localization/LocalizedProperty.swift @@ -171,6 +171,18 @@ public extension RetrievablePropertyNames { typealias LocalizedSubs = _LocalizedProperty } +/// The property wrapper seen without its generic parameters — what a translation walk +/// needs from any `@LocalizedString`/`@LocalizedSubs`/… on any Model. +package protocol LocalizedPropertyTranslation { + var translatedValue: any Localizable { get } +} + +extension _LocalizedProperty: LocalizedPropertyTranslation { + package var translatedValue: any Localizable { + wrappedValue + } +} + @propertyWrapper public struct _LocalizedProperty: Codable, Hashable, Sendable, Stubbable, Versionable { private typealias WrappedValueBinder = @Sendable (Model?, String, Encoder) throws -> Value diff --git a/Sources/FOSMVVM/Localization/Localizer.swift b/Sources/FOSMVVM/Localization/Localizer.swift index 7539815..dc46cd0 100644 --- a/Sources/FOSMVVM/Localization/Localizer.swift +++ b/Sources/FOSMVVM/Localization/Localizer.swift @@ -23,6 +23,10 @@ public enum LocalizerError: Error, CustomDebugStringConvertible { /// Localization occurs during encode/init(from:). If encoding/decoding has not taken place, then this error will result. case localizationUnbound + /// A strict encoder (see `JSONEncoder.localizingEncoder(locale:localizationStore:strictLocalization:)`) + /// met a key the store could not resolve in the given locale + case missingTranslation(_ localizable: String, locale: String) + public var debugDescription: String { switch self { case .unknownLocalizationType(let type): @@ -31,6 +35,8 @@ public enum LocalizerError: Error, CustomDebugStringConvertible { "LocalizerError: Localization store is missing" case .localizationUnbound: "LocalizerError: Localization occurs during encode/init(from:), but encoding/decoding has not taken place" + case .missingTranslation(let localizable, let locale): + "LocalizerError: Missing translation for \(localizable) in locale '\(locale)'" } } diff --git a/Sources/FOSTesting/LocalizableTestCase.swift b/Sources/FOSTesting/LocalizableTestCase.swift index 47dde5b..2b47077 100644 --- a/Sources/FOSTesting/LocalizableTestCase.swift +++ b/Sources/FOSTesting/LocalizableTestCase.swift @@ -89,10 +89,16 @@ public extension LocalizableTestCase { } /// Returns **JSONEncoder** that is configured to perform localization during encoding + /// + /// The encoder is **strict**: a key the store cannot resolve fails the encode with + /// `LocalizerError.missingTranslation` rather than encoding an empty string, so a + /// missing key on any ViewModel — a nested one, a row of a collection — is red here, + /// never a blank that ships. func encoder(locale: Locale = Self.en) -> JSONEncoder { JSONEncoder.localizingEncoder( locale: locale, - localizationStore: locStore + localizationStore: locStore, + strictLocalization: true ) } @@ -108,26 +114,54 @@ public extension LocalizableTestCase { .toJSON(encoder: encoder) .fromJSON() - let mirror = Mirror(reflecting: model) - for child in mirror.children { - guard let childName = child.label else { continue } + try Self.expectTranslated(model, path: "\(Model.self)", locale: locale) + } + } - if let localizable = child.value as? (any Localizable) { - guard !localizable.isEmpty else { - throw FOSLocalizableError.error("\(childName) -- Missing Translation -- \(locale.identifier)") - } - } + /// Walks a decoded value and its children — stored child ViewModels, optionals, and + /// collections included — so a missing translation on a row or a nested ViewModel fails + /// the parent's pass instead of shipping silently. + private static func expectTranslated(_ value: Any, path: String, locale: Locale) throws { + if let localizedProperty = value as? any LocalizedPropertyTranslation { + let localizable = localizedProperty.translatedValue + guard localizable.localizationStatus == .localized else { + throw FOSLocalizableError.error("\(path) -- Is pending localization") + } + guard !localizable.isEmpty else { + throw FOSLocalizableError.error("\(path) -- Missing Translation -- \(locale.identifier)") + } + return + } - if let localizedProperty = child.value as? _LocalizedProperty { - guard localizedProperty.wrappedValue.localizationStatus == .localized else { - throw FOSLocalizableError.error("\(childName) -- Is pending localization") - } + if let localizable = value as? (any Localizable) { + guard !localizable.isEmpty else { + throw FOSLocalizableError.error("\(path) -- Missing Translation -- \(locale.identifier)") + } + return + } - guard !localizedProperty.wrappedValue.isEmpty else { - throw FOSLocalizableError.error("\(childName) -- Missing Translation -- \(locale.identifier)") - } - } + let mirror = Mirror(reflecting: value) + switch mirror.displayStyle { + case .optional: + if let wrapped = mirror.children.first?.value { + try expectTranslated(wrapped, path: path, locale: locale) + } + case .collection, .set: + for (index, element) in mirror.children.enumerated() { + try expectTranslated(element.value, path: "\(path)[\(index)]", locale: locale) + } + case .struct, .class: + // Only ViewModels (and their kin) are walked — a Date, a URL, or a value type + // from Foundation has children too, but none of them localize. + guard value is any RetrievablePropertyNames else { return } + for child in mirror.children { + guard let childName = child.label else { continue } + // A property wrapper's storage is mirrored as `_name`; report the property. + let name = childName.hasPrefix("_") ? String(childName.dropFirst()) : childName + try expectTranslated(child.value, path: "\(path).\(name)", locale: locale) } + default: + return } } @@ -164,12 +198,17 @@ public extension LocalizableTestCase { /// - Parameters: /// - viewModelType: A *System.Type* of a type that conforms to **ViewModel** /// - locales: An optional set of **Locale**s to test (default: LocalizableTestCase.locales) - func expectFullViewModelTests(_ viewModelType: (some ViewModel & ViewModel).Type, locales: Set? = nil, file: String = #filePath, line: Int = #line) throws { + /// - version: The version the baseline is minted and checked against (default: + /// `SystemVersion.current`). Pass the project's own version line when the tests run + /// without `setCurrentVersion` having been called — under `swift test` the default + /// is `1.0.0`, which would mint baselines off the real line. + func expectFullViewModelTests(_ viewModelType: (some ViewModel & ViewModel).Type, locales: Set? = nil, version: SystemVersion = .current, file: String = #filePath, line: Int = #line) throws { let vmEncoder = encoder(locale: locales?.first ?? self.locales.first ?? Self.en) try expectCodable(viewModelType, encoder: vmEncoder) try expectVersionedViewModel( viewModelType, + version: version, encoder: vmEncoder, file: file, line: line diff --git a/Sources/FOSTestingUI/ViewModelViewTestCase.swift b/Sources/FOSTestingUI/ViewModelViewTestCase.swift index fd1d947..9d0e386 100644 --- a/Sources/FOSTestingUI/ViewModelViewTestCase.swift +++ b/Sources/FOSTestingUI/ViewModelViewTestCase.swift @@ -247,13 +247,18 @@ import XCTest /// } /// ``` /// - /// > View tests never require YAML to be present: any localized string that has no - /// > translation in *bundles* — a server-hosted *ViewModel*'s strings, for example — + /// > View tests never require every key to be present: any localized string that has + /// > no translation in *bundles* — a server-hosted *ViewModel*'s strings, for example — /// > resolves to visible placeholder text derived from its key, so the element keeps /// > its surface area and stays reachable by XCUI. Do not assert on placeholder /// > content; localization completeness belongs in **LocalizableTestCase**'s /// > *expectTranslations()*. /// + /// > Important: A harness whose *bundles* yield **no** YAML at all fails `setUp` with + /// > ``RunError/noLocalizationYAML`` — that is a missing harness, not a missing key, + /// > and every label would otherwise echo its key and compare equal to itself forever. + /// > To run deliberately without YAML, pass `bundles: []`. + /// /// - Parameters: /// - bundles: The test harness's application bundle and other custom bundles containing YAML files /// - resourceDirectoryName: The directory in the bundle to search for localizations (default: "") @@ -274,7 +279,14 @@ import XCTest ) ) } catch YamlStoreError.noResourcePaths { - // A harness with no YAML at all is supported: every string echoes its key + // Explicitly no bundles: every string echoes its key, by request. + // Bundles that yield nothing: a misconfigured harness — say so. + guard bundles.isEmpty else { + throw RunError.noLocalizationYAML( + bundles: bundles.map(\.bundlePath), + resourceDirectoryName: resourceDirectoryName + ) + } locStore = KeyEchoLocalizationStore(wrapping: nil) } self.locales = locales ?? [Self.en] @@ -417,6 +429,7 @@ public enum RunError: Error, CustomDebugStringConvertible { case setupNotCalled case badUrlString(_ str: String) case cannotRetrieveOperationsData + case noLocalizationYAML(bundles: [String], resourceDirectoryName: String) public var debugDescription: String { switch self { @@ -428,6 +441,8 @@ public enum RunError: Error, CustomDebugStringConvertible { "RunError: Bad URL string: \(str)" case .cannotRetrieveOperationsData: "RunError: Cannot retrieve operations data" + case .noLocalizationYAML(let bundles, let resourceDirectoryName): + "RunError: No localization YAML found in \(bundles.count) bundle(s) under '\(resourceDirectoryName)' — the harness copies no YAML (\(bundles.joined(separator: ", "))). Add the YAML to the test target, or pass bundles: [] to run key-echo on purpose." } } } diff --git a/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift b/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift index 04a23f3..86a188d 100644 --- a/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift +++ b/Tests/FOSMVVMTests/Localization/LocalizableStringTests.swift @@ -122,8 +122,16 @@ struct LocalizableStringTests: LocalizableTestCase { @Test func codable_localized_unknownKey() throws { let localized = LocalizableString.localized(key: "lkjoipuew") - let decodedLoc: LocalizableString = try localized.toJSON(encoder: encoder()).fromJSON() + + // The production encoder encodes an unknown key as an empty string … + let lenient = JSONEncoder.localizingEncoder(locale: en, localizationStore: locStore) + let decodedLoc: LocalizableString = try localized.toJSON(encoder: lenient).fromJSON() #expect(try decodedLoc.localizedString == "") + + // … and the test encoder is strict about it. + #expect(throws: LocalizerError.self) { + _ = try localized.toJSON(encoder: encoder()) + } } // MARK: Identifiable Protocol diff --git a/Tests/FOSMVVMTests/Localization/TranslationWalkTests.swift b/Tests/FOSMVVMTests/Localization/TranslationWalkTests.swift new file mode 100644 index 0000000..0c7922d --- /dev/null +++ b/Tests/FOSMVVMTests/Localization/TranslationWalkTests.swift @@ -0,0 +1,115 @@ +// TranslationWalkTests.swift +// +// Copyright 2026 FOS Computer Services, LLC +// +// Licensed under the Apache License, Version 2.0 (the License); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import FOSFoundation +@testable import FOSMVVM +import FOSTesting +import Foundation +import Testing + +/// `expectTranslations` descends into stored child ViewModels and collections, and the +/// test encoder is strict — a missing key on a child fails the parent's pass. +@Suite("Translation walk", .serialized) +struct TranslationWalkTests: LocalizableTestCase { + @Test("A fully translated parent with children passes") + func fullyTranslatedPasses() throws { + try expectTranslations(WalkParentViewModel.self, locales: [Self.en]) + } + + @Test("A child whose key is missing in a locale fails the parent's pass at encode") + func missingChildKeyFails() { + #expect(throws: LocalizerError.self) { + try expectTranslations(WalkParentViewModel.self, locales: [Self.es]) + } + } + + @Test("A child whose translation is blank fails the parent's pass, naming the row") + func blankChildTranslationFails() throws { + do { + try expectTranslations(BlankParentViewModel.self, locales: [Self.es]) + Issue.record("Expected the blank child translation to fail") + } catch FOSLocalizableError.error(let message) { + #expect(message.contains("rows[0].label")) + #expect(message.contains("es")) + } + } + + @Test("The strict encoder throws on a missing key; the plain encoder encodes an empty string") + func strictEncoderThrows() throws { + let child = WalkChildViewModel.stub() + #expect(throws: LocalizerError.self) { + _ = try child.toJSON(encoder: encoder(locale: Self.es)) + } + + let lenient = JSONEncoder.localizingEncoder(locale: Self.es, localizationStore: locStore) + let decoded: WalkChildViewModel = try child.toJSON(encoder: lenient).fromJSON() + #expect(decoded.label.isEmpty) + } + + let locStore: LocalizationStore + var locales: Set { + [Self.en, Self.es] + } + + init() throws { + self.locStore = try Self.loadLocalizationStore( + bundle: Bundle.module, + resourceDirectoryName: "TestYAML" + ) + } +} + +private struct WalkParentViewModel: ViewModel { + @LocalizedString var title + let child: WalkChildViewModel + let rows: [WalkChildViewModel] + let optionalChild: WalkChildViewModel? + + var vmId: FOSMVVM.ViewModelId + + static func stub() -> WalkParentViewModel { + .init(child: .stub(), rows: [.stub(), .stub()], optionalChild: .stub(), vmId: .init()) + } +} + +private struct WalkChildViewModel: ViewModel { + @LocalizedString var label + var vmId: FOSMVVM.ViewModelId + + static func stub() -> WalkChildViewModel { + .init(vmId: .init()) + } +} + +private struct BlankParentViewModel: ViewModel { + @LocalizedString var title + let rows: [BlankChildViewModel] + + var vmId: FOSMVVM.ViewModelId + + static func stub() -> BlankParentViewModel { + .init(rows: [.stub()], vmId: .init()) + } +} + +private struct BlankChildViewModel: ViewModel { + @LocalizedString var label + var vmId: FOSMVVM.ViewModelId + + static func stub() -> BlankChildViewModel { + .init(vmId: .init()) + } +} diff --git a/Tests/FOSMVVMTests/TestYAML/TranslationWalk.yml b/Tests/FOSMVVMTests/TestYAML/TranslationWalk.yml new file mode 100644 index 0000000..75d3398 --- /dev/null +++ b/Tests/FOSMVVMTests/TestYAML/TranslationWalk.yml @@ -0,0 +1,17 @@ +en: + WalkParentViewModel: + title: "Parent" + WalkChildViewModel: + label: "Child" + BlankChildViewModel: + label: "Blank child" + BlankParentViewModel: + title: "Blank parent" + +es: + WalkParentViewModel: + title: "Padre" + BlankChildViewModel: + label: "" + BlankParentViewModel: + title: "Padre en blanco" From 3e56a84e75d9db9be906fdc723b09f18fda0605d Mon Sep 17 00:00:00 2001 From: David Hunt Date: Wed, 2 Sep 2026 23:44:04 +0200 Subject: [PATCH 7/7] docs(deferrals): the non-Fluent write door, the socket-channel door, and raw-bytes transfer --- docs/deferrals.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/deferrals.md b/docs/deferrals.md index 1ebf71f..6a91bc4 100644 --- a/docs/deferrals.md +++ b/docs/deferrals.md @@ -50,3 +50,33 @@ Work items acknowledged and deliberately not done yet. Each entry names the evid **Why it was deferred:** the message needs YAML at the server's localization store, and FOSUtilities ships no localization YAML of its own today. A framework-owned bundle is its own design: how it reaches the store the app initialized (`initYamlLocalization(bundle:resourceDirectoryName:)` takes one bundle), and whether an app's YAML may override the framework's words. The substituted message rides on that design, not ahead of it. **What reopens it:** the framework-localization-bundle design; or a second framework-owned error that needs a user-facing message, at which point the bundle stops being a one-type question. + +## No request door for a write that has no Fluent model behind it + +**Recorded:** 2026-09-02, at David's direction. Surfaced by the `server-calls-use-the-request-door` stage (2026-08-25) and confirmed by the first full customer review. + +**What it is:** every write registration on `RoutesBuilder` — `register(request:app:)` for `CreateRequest`, `UpdateRequest`, `DeleteRequest` — requires `RequestBody: DataModelWriter`, whose `Target` is a `DataModel`. A write whose effect is not a Fluent record (rotate a token, replace a secret held elsewhere, destroy an external resource) has no door and rides a hand-written `ServerRequestController`, which the review then grades as the request door bypassed. + +**Why it was deferred:** the shape is a design question — a write door whose handler is a plain `(Request, RequestBody) async throws -> ResponseBody`, or `ServerRequestController` promoted to the documented path for non-model writes — and it touches the containment model that derives response plans. It waits for the design, not for a patch. + +**What reopens it:** the design brief for non-model writes; or a second consumer with the same shape. + +## No typed rejection for a socket-channel upgrade, and no ruled socket transport + +**Recorded:** 2026-09-02, at David's direction. Surfaced by the same stage; the socket-channel gap. + +**What it is:** FOS's live channel is SSE. A project that dials its own WebSocket channel gets no typed rejection when the upgrade is refused — the response has no body, so the client branches on `401`/`426` — and no ruling on whether such a channel should sit on `URLSession` with `FOSNetworkSecurity`'s mutual-TLS session (`URLSession.session(config:mutualTLS:)` and `URLSessionWebSocketTask` exist) or on a NIO dial, which today means forking WebSocketKit's upgrade handler to verify a pinned server. + +**Why it was deferred:** two rulings, both design-sized — a header-borne rejection reason the middleware sets and a client decodes from the upgrade response head (the `426` + `SystemVersion.httpHeader` handshake is the precedent), and the transport itself, incl. reconnect and backoff. + +**What reopens it:** the socket-channel design brief; or FOS itself needing a client-dialed socket. + +## No front door for a raw-bytes or streaming transfer + +**Recorded:** 2026-09-02, at David's direction. Surfaced by the same stage. + +**What it is:** `DataFetch`'s doors are JSON-shaped (`fetch`, `send(data:)`, `delete(data:)`). An octet-stream object transfer, a ranged read, or a streamed body has no door, so a project that needs one hand-builds a `URLSession` call and suppresses the review finding by naming this gap. + +**Why it was deferred:** David ruled (2026-09-02) that the one consumer's transfer stays as-is — special-purpose, with its own retry characteristics — so there is no consumer asking for a general door. Known, not planned. + +**What reopens it:** a second consumer with the shape; or the first one asking to converge.