Skip to content

Conversation

@cbaker6
Copy link
Member

@cbaker6 cbaker6 commented Dec 10, 2025

New Pull Request Checklist

Issue Description

Closes: FILL_THIS_OUT

Approach

TODOs before merging

  • Add tests
  • Add entry to changelog
  • Add changes to documentation (guides, repository pages, in-code descriptions)

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved concurrency safety for encoding and storage operations.
    • Strengthened type constraints on storage methods to enforce thread-safe value handling in concurrent contexts.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

These updates strengthen Swift concurrency safety by adding nonisolated(unsafe) isolation to a formatter singleton in ParseEncoder and enforcing Sendable constraints on generic storage accessors in ParseStorage. No behavioral logic changes; purely concurrency annotation refinements.

Changes

Cohort / File(s) Summary
Concurrency Isolation
Sources/ParseSwift/Coding/ParseEncoder.swift
Property _iso8601Formatter declaration updated with nonisolated(unsafe) attribute (2 occurrences) to adjust concurrency isolation semantics
Sendable Constraints
Sources/ParseSwift/Storage/ParseStorage.swift
Methods get<T>(valueFor:) and set(_:for:) generic signatures tightened to require T: Sendable alongside existing codec constraints

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • ParseEncoder.swift: Verify that nonisolated(unsafe) annotation is intentional for the shared formatter singleton and won't introduce data races
  • ParseStorage.swift: Ensure all callers of these methods can satisfy the added Sendable constraint; check for any type breakage at call sites

Poem

Plus Ultra on concurrency, true! 💪
A formatter stands alone, safe and sound,
Storage now demands what's thread-bound,
Swift's isolation grows strong today,
Sendable constraints light the way! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description contains only the template with unfilled placeholders. The 'Closes: FILL_THIS_OUT' and empty 'Approach' section lack essential information about the changes and their justification. Complete the 'Closes' field with the linked issue number, describe the approach explaining why Sendable constraints were added for concurrency safety, and address the TODOs checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix async warnings' is vague and generic, lacking specificity about what async warnings are being addressed or how they relate to the concrete changes made. Consider a more descriptive title that specifies the concurrency changes, such as 'Add nonisolated(unsafe) and Sendable constraints for async safety' or 'Mark ISO8601Formatter as nonisolated(unsafe) and add Sendable constraints'.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fixAsyncWarnings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cbaker6 cbaker6 changed the title Fix async warnings fix: Atorage methods return Sendable Dec 10, 2025
@cbaker6 cbaker6 changed the title fix: Atorage methods return Sendable fix: Storage methods return Sendable Dec 10, 2025
@codecov
Copy link

codecov bot commented Dec 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.32%. Comparing base (18a7f9c) to head (b97e50a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #206      +/-   ##
==========================================
- Coverage   91.95%   91.32%   -0.63%     
==========================================
  Files         179      179              
  Lines       15977    12241    -3736     
==========================================
- Hits        14691    11179    -3512     
+ Misses       1286     1062     -224     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18a7f9c and afe3ae0.

📒 Files selected for processing (2)
  • Sources/ParseSwift/Coding/ParseEncoder.swift (1 hunks)
  • Sources/ParseSwift/Storage/ParseStorage.swift (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Sources/ParseSwift/Storage/ParseStorage.swift (1)
Sources/ParseSwift/Storage/ParsePrimitiveStorable.swift (2)
  • get (46-48)
  • set (50-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: test (platform=macOS, test)
  • GitHub Check: test (platform=visionOS\ Simulator,OS=2.4,name=Apple\ Vision\ Pro, build)
  • GitHub Check: spm-test
  • GitHub Check: linux
🔇 Additional comments (2)
Sources/ParseSwift/Storage/ParseStorage.swift (2)

60-63: A POWERFUL Sendable constraint! Your encoding is now HEROICALLY thread-safe!

Just like with the get method above, this Sendable constraint ensures that objects passed into the actor are safe to transfer across isolation boundaries! This is EXACTLY the right approach for actor-isolated storage!

The same verification above will confirm whether the ParsePrimitiveStorable protocol's set method signature has been updated to match!


55-58: The async wrapper pattern is correct; no protocol inconsistency exists.

ParseStorage doesn't implement ParsePrimitiveStorable. Instead, it uses composition—wrapping a ParsePrimitiveStorable backing store (like KeychainStore) and exposing an async interface. The synchronous protocol methods and the async actor methods serve different purposes: the protocol defines sync implementations, while ParseStorage provides an async actor boundary with Sendable constraints for safe cross-isolation calls. This is intentional design, not a mismatch requiring alignment.

Likely an incorrect or invalid review comment.

@cbaker6 cbaker6 merged commit e4ca272 into main Dec 10, 2025
13 of 15 checks passed
@cbaker6 cbaker6 deleted the fixAsyncWarnings branch December 10, 2025 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants