-
Notifications
You must be signed in to change notification settings - Fork 11
fix(patch): Optimize AST traversal in EquatableMacro #19
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
Conversation
- Refactored property extraction loop for single-pass AST traversal - Removed redundant isStoredProperty check - No functional behavior change; focused on performance and code clarity
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
+ Coverage 95.76% 96.97% +1.21%
==========================================
Files 8 11 +3
Lines 1013 1122 +109
==========================================
+ Hits 970 1088 +118
+ Misses 43 34 -9
Continue to review 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.
Pull Request Overview
This PR optimizes the AST traversal logic in EquatableMacro.expansion by refactoring the stored property extraction from a functional compactMap approach to an imperative loop with early exits. The changes aim to improve compile-time performance and code clarity while maintaining identical runtime behavior.
- Replaced
compactMapwith a for-loop andcontinuestatements for early exits - Consolidated closure detection logic into a single boolean expression
- Removed redundant
isStoredPropertycheck that duplicated existing conditions
Comments suppressed due to low confidence (1)
Sources/EquatableMacros/EquatableMacro.swift:140
- [nitpick] The function name
isClosureshould beisClosure(type:)to match Swift naming conventions for functions that take parameters.
let isClosureProperty = (binding.typeAnnotation?.type).map(isClosure) == true ||
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
Sorry for the repeated lint failures. I’ll make sure to run lint locally next time 🥲 |
supersonicbyte
left a comment
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.
awesome, thanks!
Description
This change optimizes the
EquatableMacro.expansionfunction by refactoring the stored property extraction logic to a single-pass AST traversal with early exit conditions. It also removes an unnecessaryisStoredPropertycheck that duplicated an existing condition.These changes improve code clarity and offer minor compile-time performance improvements when expanding macros, without altering runtime behavior or output.
(remove SwiftLint disable for
cyclomatic_complexity)How Has This Been Tested?
EquatableandHashableconformances continue to match expectationsMinimal checklist:
DocCcode-level documentation for any public interfaces exported by the package