-
Notifications
You must be signed in to change notification settings - Fork 0
v1.0.0-alpha.1 #2
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
WalkthroughThis update introduces Changes
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift (1)
17-17: Add missing trailing newline.The file should end with a single trailing newline as indicated by SwiftLint.
} +Sources/XMLCoder/Encoder/XMLEncoder.swift (1)
42-42: Fix colon spacing and approve Sendable conformance.The
Sendableconformance forNodeEncodingis appropriate since it's a simple enum with no associated values.- public enum NodeEncoding : Sendable { + public enum NodeEncoding: Sendable {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Sources/XMLCoder/Auxiliaries/Box/DateBox.swift(3 hunks)Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift(1 hunks)Sources/XMLCoder/Encoder/DynamicNodeEncoding.swift(1 hunks)Sources/XMLCoder/Encoder/XMLEncoder.swift(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Sources/XMLCoder/Auxiliaries/Box/DateBox.swift (1)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift (1)
xmlCoderFormatter(12-16)
Sources/XMLCoder/Encoder/XMLEncoder.swift (8)
Sources/XMLCoder/Encoder/DynamicNodeEncoding.swift (2)
nodeEncoding(48-50)nodeEncoding(54-56)Tests/XMLCoderTests/NodeEncodingStrategyTests.swift (2)
nodeEncoding(46-48)nodeEncoding(72-74)Tests/XMLCoderTests/AdvancedFeatures/AttributedIntrinsicLegacyTest.swift (4)
nodeEncoding(89-96)nodeEncoding(116-123)nodeEncoding(135-142)nodeEncoding(178-185)Tests/XMLCoderTests/AdvancedFeatures/DynamicNodeDecodingLegacyTest.swift (2)
nodeEncoding(134-139)nodeEncoding(151-158)Tests/XMLCoderTests/AdvancedFeatures/AttributedIntrinsicTest.swift (1)
nodeEncoding(143-150)Tests/XMLCoderTests/EscapedCharactersTest.swift (1)
nodeEncoding(60-62)Tests/XMLCoderTests/AdvancedFeatures/DynamicNodeEncodingTest.swift (3)
nodeEncoding(124-129)nodeEncoding(141-148)nodeEncoding(289-296)Tests/XMLCoderTests/RootLevetExtraAttributesTests.swift (1)
nodeEncoding(55-60)
🪛 SwiftLint (0.57.0)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift
[Warning] 17-17: Files should have a single trailing newline
(trailing_newline)
Sources/XMLCoder/Encoder/XMLEncoder.swift
[Warning] 42-42: Colons should be next to the identifier when specifying a type and next to the key in dictionary literals
(colon)
🔇 Additional comments (7)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift (1)
11-17: Excellent refactoring for thread safety!The change from a shared global
ISO8601DateFormatterinstance to a static factory method is a great improvement for concurrency safety. Each call creates a fresh formatter instance, eliminating any potential race conditions from shared state.Sources/XMLCoder/Auxiliaries/Box/DateBox.swift (2)
47-47: LGTM: Proper usage of the new factory method.The replacement of the global formatter with
ISO8601DateFormatter.xmlCoderFormatter()is correct and maintains the same functionality while improving thread safety.
73-73: LGTM: Consistent usage of the new factory method.The replacement maintains consistency with the other usage site and correctly applies the thread-safe pattern.
Sources/XMLCoder/Encoder/DynamicNodeEncoding.swift (1)
43-43: LGTM: Appropriate Sendable conformance for protocol.Adding
Sendableconformance toDynamicNodeEncodingis a good improvement for concurrency safety. This ensures that all conforming types will be required to be thread-safe, which aligns with the overall goal of making the XMLCoder library concurrency-safe.Sources/XMLCoder/Encoder/XMLEncoder.swift (3)
16-16: LGTM: Appropriate Sendable conformance for OutputFormatting.Adding
Sendableconformance toOutputFormattingis correct since it's anOptionSetwith aUIntraw value, making it inherently thread-safe.
236-237: LGTM: Proper @sendable annotations for closure typealiases.The addition of
@Sendableannotations to the closure typealiases is correct and ensures thread safety for encoding operations.
265-271: Smart compiler version handling for @sendable closures.The conditional compilation approach is well-implemented:
- For Swift 6.1+: Uses direct method reference which is automatically
@Sendable- For older versions: Explicitly wraps in
@SendableclosureThis ensures compatibility across compiler versions while maintaining thread safety.
| import Foundation | ||
|
|
||
| struct DateBox: Equatable { | ||
| struct DateBox: Equatable, Sendable { |
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.
💡 Verification agent
🧩 Analysis chain
Verify Sendable compliance with DateFormatter case.
While adding Sendable conformance to DateBox is good for concurrency safety, the Format.formatter(DateFormatter) case may be problematic since DateFormatter is not Sendable. This could lead to compiler warnings or errors in strict concurrency mode.
Let me verify if DateFormatter conforms to Sendable:
🏁 Script executed:
#!/bin/bash
# Check if DateFormatter conforms to Sendable in the codebase
ast-grep --pattern '$_: DateFormatter' | head -20
rg -A 5 -B 5 "DateFormatter.*Sendable\|Sendable.*DateFormatter"Length of output: 117
Let’s inspect the Format enum in DateBox.swift to confirm its Sendable conformance and associated cases:
🏁 Script executed:
#!/bin/bash
rg -n "enum Format" -A 10 Sources/XMLCoder/Auxiliaries/Box/DateBox.swiftLength of output: 345
Fix the non-Sendable DateFormatter case in DateBox.Format.
The Format.formatter(DateFormatter) associated value makes Format (and thus DateBox) fail strict-Sendable checks, since DateFormatter isn’t Sendable. To resolve this, either:
- Replace the
DateFormatter–based case with a thread-safe alternative (e.g. a format identifier or closure that’sSendable), - Or explicitly opt out of automatic checking by marking the enum as
@unchecked Sendable.
For example, at the bottom of Sources/XMLCoder/Auxiliaries/Box/DateBox.swift:
enum Format: Equatable {
case secondsSince1970
case millisecondsSince1970
case iso8601
case formatter(DateFormatter)
}
+
+ // DateFormatter is not @Sendable; opt out of checking here
+ @unchecked
+ extension DateBox.Format: Sendable {}• Location to update:
- Sources/XMLCoder/Auxiliaries/Box/DateBox.swift (lines 12–17)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Sources/XMLCoder/Auxiliaries/Box/DateBox.swift around lines 11 to 17, the
Format enum inside DateBox uses a DateFormatter in one of its cases, which is
not Sendable and causes strict Sendable conformance to fail. To fix this, either
replace the DateFormatter case with a thread-safe, Sendable alternative like a
format identifier or closure, or mark the Format enum with @unchecked Sendable
to opt out of automatic Sendable checking. This change will ensure DateBox
conforms to Sendable without errors.
Summary by CodeRabbit
New Features
Sendable.Refactor