Conversation
- Remove conventional-name fallback hook detection; only @CodableHook is invoked. - Warn when conventional hook methods exist without @CodableHook. - Update README and macro expansion tests.
Document that hooks are annotation-only in v2 and provide before/after examples.
Conventional hook method names are no longer invoked in v2; make this a compile-time error and update docs/tests.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a breaking change to CodableKit v2 that makes lifecycle hooks explicit: only methods annotated with @CodableHook will be recognized and invoked. The macro now emits compile-time errors when conventional hook method names are detected without the required annotation.
- Removed implicit hook detection by conventional method names (
willDecode,didDecode,willEncode,didEncode) - Added compile-time diagnostics to catch unannotated conventional hooks
- Updated documentation and added migration guide for v1 to v2 upgrades
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
Sources/CodableKitMacros/CodeGenCore.swift |
Removed fallback logic for conventional hook names; added diagnostic generation for unannotated hooks with conventional names |
Tests/EncodableKitTests/CodableMacroTests+hooks.swift |
Added diagnostic expectations for unannotated willEncode and didEncode methods |
Tests/DecodableKitTests/CodableMacroTests+hooks.swift |
Added diagnostic expectation for unannotated didDecode method |
Tests/CodableKitTests/CodableMacroTests+class_hooks.swift |
Added diagnostic expectations for three unannotated hook methods in class context |
README.md |
Updated hook documentation to clarify explicit annotation requirement; added reference to migration guide |
MIGRATION.md |
New file providing comprehensive migration guidance from v1 to v2 with examples for each hook type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
This pull request introduces breaking changes to how Codable lifecycle hooks are detected and invoked in CodableKit v2. Hooks are now explicit: only methods annotated with
@CodableHookwill be recognized and invoked, and using conventional method names without the annotation will result in a compile-time error. The documentation and tests have been updated to reflect this new behavior, and migration guidance has been added.Codable Hook Detection Changes:
didDecode,willEncode,didEncode,willDecode); only@CodableHook-annotated methods are now invoked. [1] [2]@CodableHook, a clear error is emitted indicating the method will not be invoked.Documentation Updates:
MIGRATION.mdfile detailing the breaking changes and providing migration steps for upgrading from v1 to v2.README.mdto clarify that hooks are now explicit and to direct users to the migration guide. [1] [2]Test Suite Updates: