Replace any-Error existentials with per-client leaf error types - #20
Merged
Conversation
The five @witness client structs (Repositories, OAuth, Traffic, Collaborators, Stargazers) declared closures typed throws(any Swift.Error), tripping no_any_protocol_existential and typed_throws_required. Each client now declares a concrete, nested Error enum with one case per operation and its closures are typed throws(Client.Error) against it, per the L3 client-modularization leaf-error doctrine. This is source-breaking for any out-of-repo Live implementation that assigns closures typed against the old any-Error signature or propagates arbitrary errors through these witnesses — such implementations must now throw the client's own Error type (or map into it) instead.
The Repositories, Collaborators, and Stargazers convenience-method extensions still declared untyped async throws even though they forward directly to the now-typed @witness closures; type them throws(Client.Error) too. Also reflows the Collaborators closure signatures and applies swift-format so the file matches the repo's .swift-format configuration.
coenttb
commented
Aug 3, 2026
coenttb
left a comment
coenttb
left a comment
Collaborator
Author
There was a problem hiding this comment.
Independent review — verdict: APPROVE (posted as a COMMENT; formal approval is the engine's job). Independent review lane of coordinator session e25a1d74, reviewing exactly head e24ceb5.
Checked:
- All 10 original violation sites are cleared; at this head no
throws(any Swift.Error)or untypedasync throwsremains anywhere in the fiveGitHub * Typesclient files, including every convenience wrapper (list(owner:repo:),listAll, etc., nowthrows(Client.Error)). Zero new lint suppressions in the diff. - Leaf-error shape: each
Erroris a concreteSwift.Error & Sendable & Equatableenum nested asClient.Error— nocase underlying(any Error)smuggling the existential back in. Doctrine-conformant on the axes that matter. - Per-client vs per-operation: the issue (#19) sanctions "per-operation/per-client". I verified the claim in the PR body:
Package.swiftcompiles only theGitHub Standardtarget, and the five changed source directories are not members of any target, so the operation namespaces (List,Create, …) these files reference are indeed uncompiled scaffold. Minting operation-namespace host types just to hang per-operation errors on them would be fabrication; per-client is the right minimal granularity here. - API-break disclosure: the body enumerates every retyped closure and states no migration shim — honest and complete. Pre-release breaking posture covers it.
- Unblocking #15/#17: those PRs add wire values in the compiled
GitHub Standardtarget; this PR touches only the uncompiled client scaffold and clears the lint leg that was failing their runs. No overlap or conflict expected.
What the green actually proves: CI run 30851838572 (conclusion=success, exactly this head) proves lint-clean and a successful build of the GitHub Standard target — it does not compile the five changed files, because they are not wired into any target. The green is real evidence for the lint objective (which is this PR's stated goal) but is silent on whether these files typecheck; that residual risk predates this PR and will be discharged when the L3 modularization programme wires these directories into targets.
Non-blocking observations for the follow-up that wires these files in: (1) the case <operation>(reason: String) shape names the operation, not the failure condition — /swift error doctrine wants cases like invalidHeader(expected:found:); a stringly reason: payload is acceptable interim scaffolding but should become structured payloads (status, decoding failure, etc.) when Live implementations exist. (2) GitHub.OAuth.Client remains @unchecked Sendable (pre-existing, untouched here).
Nothing here blocks landing. Refs #19.
![swift-institute-bot[bot]](https://avatars.githubusercontent.com/in/3543256?s=60&v=4){kind=small}
coenttb
added a commit
to swift-foundations/swift-identities-types
that referenced
this pull request
Aug 4, 2026
…t.Error) Identity.Authentication.Client.Error and Identity.Authentication.Token.Client.Error replace the existential throws(any Swift.Error) spelling with concrete, Sendable, Equatable leaf error enums per the L3 client-modularization leaf-error doctrine (swift-standards/swift-github-standard#20 precedent). Both conform to Witness.Unimplemented.Representable so the @witness macro's .unimplemented() test-default codegen keeps compiling under typed throws.
coenttb
added a commit
to swift-foundations/swift-identities-types
that referenced
this pull request
Aug 4, 2026
…ed + no_any_protocol_existential) (#12) * Resolve remaining SwiftLint findings on issue #9 (typed_throws_required + no_any_protocol_existential) typed_throws_required (47 sites, 17 files): convert untyped `async throws` / `async throws -> T` wrapper functions to `async throws(any Swift.Error) -> T`, matching the error type already declared on the underlying stored-closure witness property they forward to. Real, non-suppressing conversion — no behavior change, since these wrappers already propagate whatever the witness closure throws. no_any_protocol_existential (111 sites after the above conversion surfaces new matches on the wrapper functions, 64 pre-existing + 47 newly typed): these are Dependencies-style pluggable Client witness structs (and one OAuth.Provider protocol) whose stored closures / requirements are constructed per-backend by TestDependencyKey/liveValue conformances elsewhere in the package graph. Threading a concrete or generic Failure parameter through would require a cross-package architecture change to every enclosing Identity.* namespace and every downstream @dependency consumer — out of scope for issue #9. Disposed as rule-sanctioned block disables (`swiftlint:disable`/`enable no_any_protocol_existential`) bracketing each declaration, with a reason citing the [API-ERR-006]/[#219] Encoder/Decoder witness carve-out precedent already documented in the rule's own custom_rules entry. Both SwiftLint (--strict) and swift-format (lint --strict) are clean locally after this change. No config edits, no rule weakening. Part of #9. * Revert "Resolve remaining SwiftLint findings on issue #9 (typed_throws_required + no_any_protocol_existential)" This reverts commit 4e26396. * Authentication: per-client leaf Error enums replacing throws(any Swift.Error) Identity.Authentication.Client.Error and Identity.Authentication.Token.Client.Error replace the existential throws(any Swift.Error) spelling with concrete, Sendable, Equatable leaf error enums per the L3 client-modularization leaf-error doctrine (swift-standards/swift-github-standard#20 precedent). Both conform to Witness.Unimplemented.Representable so the @witness macro's .unimplemented() test-default codegen keeps compiling under typed throws. * Creation: per-client leaf Error enum replacing throws(any Swift.Error) * Deletion: per-client leaf Error enum replacing throws(any Swift.Error) * Email.Change: per-client leaf Error enum replacing throws(any Swift.Error) * Logout: per-client leaf Error enum replacing throws(any Swift.Error) * MFA.BackupCodes: per-client leaf Error enum replacing throws(any Swift.Error) * MFA.Email: per-client leaf Error enum replacing throws(any Swift.Error) * MFA.SMS: per-client leaf Error enum replacing throws(any Swift.Error) * MFA.Status: per-client leaf Error enum replacing throws(any Swift.Error) * MFA.TOTP: promote ClientError to Error, add unimplemented case + Representable Renames the pre-existing ClientError enum to the doctrine-standard Client.Error name, adds Sendable conformance, and types all 12 witness closures with it, replacing throws(any Swift.Error). * MFA.WebAuthn: per-client leaf Error enum replacing throws(any Swift.Error) * OAuth: extend Identity.OAuth.Client.Error with .unimplemented case Fixes the compile break isolated by review 4849676698: the @witness macro's .unimplemented() test-default codegen requires the leaf error to conform to Witness.Unimplemented.Representable. Also retypes the remaining throws(any Swift.Error) witness closures (provider, providers, authorizationURL, callback, connection, disconnect, getValidToken, getAllConnections) to the same leaf error, which pre-existed at merge-base and was not part of PR #12's original diff. * Password: per-client leaf Error enums (Reset, Change) replacing throws(any Swift.Error) * Reauthorization: per-client leaf Error enum replacing throws(any Swift.Error) * Identity: type require and all forwarding conveniences with leaf errors Identity.require and Identity+conveniences.swift's forwarding wrappers previously called their underlying witness closures under bare/existential throws. Now that every client exposes a concrete leaf Error, the forwarding layer is typed to match exactly the client it forwards to. Identity.Error itself gains Sendable + Equatable. * _Testing: fix withDependencies typed-throws inference in TestHelper Fixes the compile break isolated by review 4849676698 at TestHelper.swift:20: withDependencies's trailing operation closure literal needs an explicit throws(Failure) annotation, or Swift infers any Error for the closure and the outer throws(Failure) call site fails to typecheck. * OAuth: type Provider protocol with a leaf error; suppress the registry's genuine existential Identity.OAuth.Provider's four requirements (authorizationURL, exchangeCode, getUserInfo, refreshToken) get a shared Identity.OAuth.Provider.Error leaf type instead of untyped throws, caught by lint once the existential-throws revert restored the pre-4e26396 baseline signatures. Identity.OAuth.Client's registerProvider/provider/providers witness closures carry a genuine any Identity.OAuth.Provider existential: the client is a runtime registry of heterogeneous, dynamically-registered provider conformances keyed by string identifier, which a generic parameter cannot express. These are the rework's residual disable:next sites — three total, each with an inline reason distinct from the #219 Encoder/Decoder carve-out the prior head misapplied. * _Testing: drop redundant -> Void from the typed-throws closure fix * swift-format: reflow lines that exceeded the 100-column limit Mechanical wrapping (swift-format format --in-place) of the longer leaf-error type names introduced across the leaf-error rework; no semantic change. * Fix invalid redeclaration: rename unimplemented case to unimplementedWitness Ubuntu 6.3/6.4.x release compile logs at 04152f6 show 'invalid redeclaration of unimplemented' across all 14 leaf-error enums: an enum case named unimplemented implicitly synthesizes a same-named, same-signature initializer that collides with the Witness.Unimplemented.Representable requirement 'static func unimplemented(_:) -> Self' — exactly the trap documented on Witness.Unimplemented.Representable itself (swift-witnesses), which this rework's first pass missed. Renames the wrapping case to unimplementedWitness everywhere; the Representable conformance's static func keeps the required name and constructs the renamed case. * Fix Identity.OAuth.Provider.Error: Swift forbids nesting a type in a protocol extension Same compile pass: 'type Error cannot be nested in protocol extension of Provider' plus the cascading Equatable-derivation errors it caused. Renames the leaf type from Identity.OAuth.Provider.Error (illegal) to Identity.OAuth.ProviderError, declared on the Identity.OAuth namespace instead of the Provider protocol, and retypes all four protocol requirements plus the default refreshToken implementation to match. * Fix typed-throws closure inference in test doubles and JWT wrapping Test-double closures need an explicit throws(LeafError) annotation on the closure signature; without it Swift infers the closure's thrown type as any Error even when the body's catch clause only ever throws the leaf case, so it fails to convert to the client's typed-throws closure slot. Also wrap RFC_7519.Error from JWT.compactSerialization() into Identity.Authentication.Token.Client.Error at the two call sites that forward it untranslated. * swift-format: reflow line exceeding 100-column limit The typed-throws closure annotation added for the request closure in Identity.Password.Change's test double pushed the parameter list past the line-length limit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementation lane of coordinator session e25a1d74.
What
Replaces
throws(any Swift.Error)/any Swift.Errorexistentials in the five@Witnessclient structs with concrete, per-client leaf error types, per the ruled L3 client-modularization leaf-error doctrine (typed throws via per-operation leaf errors):Sources/GitHub Repositories Types/GitHub.Repositories.Client.swift— addsGitHub.Repositories.Client.Error(cases:list,get,create,update,delete)Sources/GitHub OAuth Types/GitHub.OAuth.Client.swift— addsGitHub.OAuth.Client.Error(cases:exchangeCode,getAuthenticatedUser,getUserEmails)Sources/GitHub Traffic Types/GitHub.Traffic.Client.swift— addsGitHub.Traffic.Client.Error(cases:views,clones,paths,referrers)Sources/GitHub Collaborators Types/GitHub.Collaborators.Client.swift— addsGitHub.Collaborators.Client.Error(cases:list,check,add,remove,getPermission,listInvitations,updateInvitation,deleteInvitation)Sources/GitHub Stargazers Types/GitHub.Stargazers.Client.swift— addsGitHub.Stargazers.Client.Error(cases:list)Each
Erroris a concreteSwift.Error & Sendable & Equatableenum nested under itsClient, with one case per closure property (currently areason: Stringpayload). Closures in each@Witnessstruct now readasync throws(Client.Error) -> ...instead ofasync throws(any Swift.Error) -> ....Clears all 10 SwiftLint violation sites (
no_any_protocol_existential+typed_throws_required) on these files with no new suppressions.Why per-client rather than per-operation
Issue #19 sanctions either granularity ("per-operation/per-client leaf error types"). These client files reference operation namespaces (
List,Create,Update, …) that are not currently defined anywhere in this repository's compiled sources — theGitHub *Typessource tree here predates full L3 modularization and is not wired into the singleGitHub StandardSwiftPM target inPackage.swift. Introducing new operation-namespace types to host per-operation errors was out of scope for a lint-fix; a per-clientErrorenum is the minimal concrete leaf type that satisfies the doctrine without fabricating unreferenced scaffolding.Source-breaking, honestly
This is source-breaking for any out-of-repo Live implementation:
list,get,create,update,delete,exchangeCode,getAuthenticatedUser,getUserEmails,views,clones,paths,referrers,check,add,remove,getPermission,listInvitations,updateInvitation, ordeleteInvitationon any of the five clients must now throw the client's ownErrortype instead of an arbitraryany Swift.Error.throws an HTTP/decoding error directly must now map it into the correspondingClient.Errorcase (e.g.Client.Error.list(reason: "\(underlyingError)")) before throwing.Refs #19.