-
Notifications
You must be signed in to change notification settings - Fork 1
Add support for sendable conformance argument #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Parsing no longer assumes fixed argument positions and allows partial argument lists.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
==========================================
+ Coverage 68.64% 68.78% +0.13%
==========================================
Files 69 71 +2
Lines 4538 4558 +20
==========================================
+ Hits 3115 3135 +20
Misses 1423 1423 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial thoughts:
- Implementation looks good - no real issues there.
- Some suggested improvements to documentation, code formatting, naming, etc.
I still need to look closer at the README and tests, so I may have some more comments later, but I figured I'd give you these to start.
Appreciate the work you've done on this!
Sources/Mocking/Models/MacroArguments/MockSendableConformance.swift
Outdated
Show resolved
Hide resolved
...ces/MockingMacros/Extensions/InheritedTypeSyntax/InheritedTypeSyntax+UncheckedSendable.swift
Outdated
Show resolved
Hide resolved
Sources/MockingMacros/Macros/MockedMacro/MockedMacro+MacroArguments.swift
Outdated
Show resolved
Hide resolved
Sources/MockingMacros/Macros/MockedMacro/MockedMacro+MacroArguments.swift
Outdated
Show resolved
Hide resolved
Sources/MockingMacros/Models/MacroArguments/MockSendableConformance.swift
Show resolved
Hide resolved
Sources/MockingMacros/Models/MacroArguments/MockSendableConformance.swift
Outdated
Show resolved
Hide resolved
Sources/MockingMacros/Models/MacroArguments/MockSendableConformance.swift
Outdated
Show resolved
Hide resolved
@graycampbell I've addressed your initial feedback and re-requested review. Thanks! |
📝 Summary
Added support for sendable conformance (
.checked
|.unchecked
) on generated mocks. This is provided as an argument on the@Mocked
macro:.checked
(also the Default) is a no-op right now since the mock's sendability is inherited through the implementation and implicitly checked by the compiler already..unchecked
conforms the generated mock to@unchecked Sendable
, supporting a fix for the issue described in [Feature] Support unchecked Sendable mocks #86.To accomplish this, this pull request:
SendableConformance
macro argument.MacroArgument
protocol and refactors argument parsing to allow for partial arguments (ex., havingsendableConformance
but notcompilationCondition
), giving us greater flexibility for future arguments.@unchecked Sendable
a reusable extension ofInheritedTypeSyntax
.@unchecked Sendable
to the inheritance clause when.unchecked
is passed in for thesendableConformance
argument.🛠️ Type of Change
🧪 How Has This Been Tested?
I replicated the scenario and error described in #86 and verified it is resolved when conforming the mock to
@unchecked Sendable
. Unit tests verifying the macro's expansion passed. Please see screenshots of the expansions below:.checked
).checked
.unchecked
🔗 Related PRs or Issues
✅ Checklist
SwiftFormat