Conversation
…ro for nested type support
…acro for nested type support
…acro for nested type support
Contributor
📝 WalkthroughWalkthroughRefactors three polymorphic codable macros from ExtensionMacro to MemberMacro, splitting generated member declarations (meta coding key, init, encode) from the empty extension conformance. Interface attributes and tests were updated; nested-enum expansion behavior was adjusted and covered by new tests. Changes
Sequence DiagramsequenceDiagram
participant Compiler
participant MacroDecl as Macro Declaration
participant MemberMacro as MemberMacro
participant ExtensionMacro as ExtensionMacro
participant Output as Generated Code
rect rgba(100,150,255,0.5)
Note over Compiler,Output: MemberMacro + ExtensionMacro expansion split
end
Compiler->>MacroDecl: Parse attribute on enum
activate MacroDecl
MacroDecl->>MemberMacro: invoke providingMembersOf expansion
activate MemberMacro
MemberMacro->>Output: emit `PolymorphicMetaCodingKey`, `init(from:)`, `encode(to:)` members
MemberMacro-->>MacroDecl: return [DeclSyntax]
deactivate MemberMacro
MacroDecl->>ExtensionMacro: invoke providingExtensionsOf expansion
activate ExtensionMacro
ExtensionMacro->>Output: emit `extension Type: Protocol {}` (empty body)
ExtensionMacro-->>MacroDecl: return [ExtensionDeclSyntax]
deactivate ExtensionMacro
deactivate MacroDecl
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumCodable.swift`:
- Around line 20-22: Finish the truncated doc comment for the
identifierCodingKey property in PolymorphicEnumCodable.swift: update the comment
that currently ends with "case of the enum during" so it completes the sentence
to mirror the Decodable variant's documentation (explain it's used to identify
the enum case during decoding/encoding), referencing the identifierCodingKey
property and the PolymorphicEnumCodable type so reviewers can locate and verify
the change.
In
`@Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumDecodable.swift`:
- Around line 19-21: The doc comment for the identifierCodingKey property in
PolymorphicEnumDecodable is truncated ("...case of the enum during"); update the
documentation to match the complete pattern used in the other interface files by
finishing the sentence to explain that this key is used to identify the specific
enum case during decoding, state the default value is "type", and mention it is
used when encoding/decoding polymorphic enums; locate the identifierCodingKey
documentation within the PolymorphicEnumDecodable declaration and replace the
truncated text with the full explanatory sentence.
In
`@Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumEncodable.swift`:
- Around line 19-21: The doc-comment for the identifierCodingKey property on
PolymorphicEnumEncodable is truncated mid-sentence; update the documentation to
complete the sentence and explain its purpose (e.g., "used to identify the
specific case of the enum during encoding and decoding"), ensuring the
doc-comment for identifierCodingKey and surrounding docs on the
PolymorphicEnumEncodable protocol/struct are complete and clear so consumers
understand how the type identifier is stored and used during encoding/decoding.
In
`@Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swift`:
- Around line 56-68: The expansion currently only checks
declaration.is(EnumDeclSyntax.self) and always emits extension \(type.trimmed):
Codable {}, so change PolymorphicEnumCodableMacro.expansion to run the same
preflight/validation used by the member role (the check that validates case
layout and fallback configuration) after confirming the declaration is an enum;
if that validation fails, swallow the error and return [] (do not emit the
Codable extension) so only the member role reports the diagnostic; ensure you
call the existing validation routine (the same function invoked by the member
role) rather than duplicating logic, and add a negative expansion test that
asserts no Codable extension is emitted for an invalid case shape.
In
`@Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumEncodableMacro.swift`:
- Around line 33-35: The macro currently declares a PolymorphicMetaCodingKey
attachment but only emits the encode(to:) method; update
PolymorphicEnumEncodableMacro to also generate the PolymorphicMetaCodingKey enum
(matching the Decodable variant) so the macro contract is fulfilled: locate the
macro implementation that returns ["\(raw: encodeToEncoderSyntax)"] and extend
it to emit the enum declaration named PolymorphicMetaCodingKey (with the same
cases/structure used by the Decodable macro), ensuring the generated symbol name
PolymorphicMetaCodingKey and the encode(to:) implementation reference the same
coding-key cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c74da97-52c6-4a63-b182-bd1d724951c2
📒 Files selected for processing (9)
Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumCodable.swiftSources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumDecodable.swiftSources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumEncodable.swiftSources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swiftSources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumDecodableMacro.swiftSources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumEncodableMacro.swiftTests/KarrotCodableMacrosTests/PolymorphicEnumCodableMacroTests/PolymorphicEnumCodableMacroTests.swiftTests/KarrotCodableMacrosTests/PolymorphicEnumCodableMacroTests/PolymorphicEnumDecodableMacroTests.swiftTests/KarrotCodableMacrosTests/PolymorphicEnumCodableMacroTests/PolymorphicEnumEncodableMacroTests.swift
Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumCodable.swift
Outdated
Show resolved
Hide resolved
Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumDecodable.swift
Outdated
Show resolved
Hide resolved
Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumEncodable.swift
Outdated
Show resolved
Hide resolved
Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swift
Show resolved
Hide resolved
Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumEncodableMacro.swift
Show resolved
Hide resolved
- Complete truncated identifierCodingKey doc comments in PolymorphicEnumCodable, PolymorphicEnumDecodable, and PolymorphicEnumEncodable interface files - Remove unused named(PolymorphicMetaCodingKey) from PolymorphicEnumEncodable macro declaration since encode(to:) does not use PolymorphicMetaCodingKey
Add enum and argument validation to the ExtensionMacro role of PolymorphicEnumCodable/Decodable/Encodable macros. Previously, when the MemberMacro failed validation, the ExtensionMacro could still emit protocol conformance without actual implementations, causing confusing secondary compiler errors. Now both roles validate consistently and throw on invalid input.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swift (1)
19-28: 🛠️ Refactor suggestion | 🟠 Major[HIGH] Extract a single preflight for both macro roles.
The enum/identifier/case/fallback preflight is copied in both expansions. That makes these roles easy to drift out of sync again, which can reintroduce the same member/extension desynchronization this PR is fixing. Move it into one helper or factory entry point and have both roles call it.
As per coding guidelines, "Evaluate separation of concerns between validation, generation, and expansion" and "Assess error handling and propagation across macro layers."
Also applies to: 64-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swift` around lines 19 - 28, The repeated enum/identifier/case/fallback preflight logic in PolymorphicEnumCodableMacro should be consolidated into a single helper on PolymorphicEnumCodableFactory (e.g., a new preflight(node:declaration:) that returns the identifierCodingKey, caseInfos, and fallbackCaseName) and both macro expansion paths should call that helper instead of duplicating calls to validateIdentifierCodingKey(in:), extractCaseInfos(from:), and validateFallbackCaseName(in:caseInfos:); update PolymorphicEnumCodableMacro to use the new factory entry point and remove the duplicated blocks so both roles share identical validation and error propagation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumEncodableMacro.swift`:
- Around line 39-55: Extract the duplicate validation into a shared helper
(e.g., PolymorphicEnumCodableFactory.validateAttachment(...) or
validatePolymorphicEnumAttachment(...)) that performs the enum check, calls
PolymorphicEnumCodableFactory.validateIdentifierCodingKey and
PolymorphicEnumCodableFactory.extractCaseInfos and returns the validated
EnumDeclSyntax (or cached result); then replace the current validation calls in
PolymorphicEnumEncodableMacro.expansion and the MemberMacro expansion with a
single call to that helper so diagnostics are emitted once and both roles still
prevent orphaned conformances.
---
Duplicate comments:
In
`@Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swift`:
- Around line 19-28: The repeated enum/identifier/case/fallback preflight logic
in PolymorphicEnumCodableMacro should be consolidated into a single helper on
PolymorphicEnumCodableFactory (e.g., a new preflight(node:declaration:) that
returns the identifierCodingKey, caseInfos, and fallbackCaseName) and both macro
expansion paths should call that helper instead of duplicating calls to
validateIdentifierCodingKey(in:), extractCaseInfos(from:), and
validateFallbackCaseName(in:caseInfos:); update PolymorphicEnumCodableMacro to
use the new factory entry point and remove the duplicated blocks so both roles
share identical validation and error propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73025f9a-efad-481b-aec7-ddfdcce12ee3
📒 Files selected for processing (9)
Sources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumCodable.swiftSources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumDecodable.swiftSources/KarrotCodableKit/PolymorphicCodable/Interface/PolymorphicEnumEncodable.swiftSources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumCodableMacro.swiftSources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumDecodableMacro.swiftSources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumEncodableMacro.swiftTests/KarrotCodableMacrosTests/PolymorphicEnumCodableMacroTests/PolymorphicEnumCodableMacroTests.swiftTests/KarrotCodableMacrosTests/PolymorphicEnumCodableMacroTests/PolymorphicEnumDecodableMacroTests.swiftTests/KarrotCodableMacrosTests/PolymorphicEnumCodableMacroTests/PolymorphicEnumEncodableMacroTests.swift
Comment on lines
+39
to
+55
| extension PolymorphicEnumEncodableMacro: ExtensionMacro { | ||
| public static func expansion( | ||
| of node: AttributeSyntax, | ||
| attachedTo declaration: some DeclGroupSyntax, | ||
| providingExtensionsOf type: some TypeSyntaxProtocol, | ||
| conformingTo _: [TypeSyntax], | ||
| in _: some MacroExpansionContext, | ||
| ) throws -> [ExtensionDeclSyntax] { | ||
| guard let enumDecl = declaration.as(EnumDeclSyntax.self) else { | ||
| throw CodableKitError.message("`@PolymorphicEnumEncodable` can only be attached to enums") | ||
| } | ||
|
|
||
| try PolymorphicEnumCodableFactory.validateIdentifierCodingKey(in: node) | ||
| _ = try PolymorphicEnumCodableFactory.extractCaseInfos(from: enumDecl) | ||
|
|
||
| return try [ExtensionDeclSyntax("extension \(type.trimmed): Encodable {}")] | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[MEDIUM] Both macro roles duplicate validation, causing duplicate diagnostics.
The MemberMacro and ExtensionMacro expansions independently perform the same validations (enum check, validateIdentifierCodingKey, extractCaseInfos). When validation fails, both roles throw, resulting in duplicate error messages shown to the user.
This is consistent with commit message noting "added validation to the ExtensionMacro role to prevent emitting orphaned protocol conformances when MemberMacro validation fails." However, consider extracting validation into a shared helper that caches results, or accept the duplicate diagnostics as an intentional design trade-off for preventing orphaned extensions.
The tests explicitly expect duplicate DiagnosticSpec entries, so this appears intentional—just flagging for awareness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Sources/KarrotCodableKitMacros/PolymorphicEnumCodableMacros/PolymorphicEnumEncodableMacro.swift`
around lines 39 - 55, Extract the duplicate validation into a shared helper
(e.g., PolymorphicEnumCodableFactory.validateAttachment(...) or
validatePolymorphicEnumAttachment(...)) that performs the enum check, calls
PolymorphicEnumCodableFactory.validateIdentifierCodingKey and
PolymorphicEnumCodableFactory.extractCaseInfos and returns the validated
EnumDeclSyntax (or cached result); then replace the current validation calls in
PolymorphicEnumEncodableMacro.expansion and the MemberMacro expansion with a
single call to that helper so diagnostics are emitted once and both roles still
prevent orphaned conformances.
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.
Background (Required)
@PolymorphicEnumCodable,@PolymorphicEnumDecodable,@PolymorphicEnumEncodablemacros on enums defined inside nested types (Fixes Compiler error when using @PolymorphicEnumCodable macro on enums defined inside nested types #19)Changes
@attached(extension)to@attached(member)+@attached(extension, conformances:)forPolymorphicEnumCodableMacro,PolymorphicEnumDecodableMacro,PolymorphicEnumEncodableMacroPolymorphicMetaCodingKey,init(from:),encode(to:)generation from extension body to member declarationsTesting Methods
Review Notes
@PolymorphicCodableStrategyProvidingmacro on protocols defined inside nested types #15 for@PolymorphicCodableStrategyProviding@attached(extension), generated code lives at top-level scope where sibling types within the parent namespace are not accessible by simple name. Converting to@attached(member)places the generated code inside the enum body, preserving the original type resolution scopeSummary by CodeRabbit
Refactor
Documentation
Tests