Upgrade swift version to 6 for Data layer target #192
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to various Swift files to enhance concurrency support and update protocol conformances. Key updates include the addition of Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
pantelisss
changed the base branch from
main
to
feature/fe-1352-upgrade-swift-version-to-6-for-widget-target
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (22)
wxm-ios/Toolkit/Toolkit/Utils/Combine+.swift (1)
9-9: 💡 Codebase verification
Swift 6 migration: Add @preconcurrency to all Combine imports
Based on the search results, there are numerous files importing Combine without the @preconcurrency attribute. Since this is a Swift 6 migration, all Combine imports should consistently use @preconcurrency to properly mark potentially unsafe concurrent APIs.
Key locations requiring the same change:
wxm-ios/AppCore/MainApp.swiftwxm-ios/Toolkit/Toolkit/Utils/WXMLocationManager.swiftwxm-ios/Toolkit/Toolkit/Utils/Cancellables.swift- All files in
wxm-ios/DataLayer/- All files in
wxm-ios/DomainLayer/- All files in
PresentationLayer/🔗 Analysis chain
LGTM: Appropriate use of @preconcurrency attribute
The addition of @preconcurrency to the Combine import is correct for Swift 6 migration, as it properly marks potentially unsafe concurrent APIs.
Let's verify consistent usage across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other Combine imports that might need @preconcurrency rg "^import\s+Combine" --type swiftLength of output: 8614
station-widget/Views/ErrorView.swift (1)
9-9: 💡 Codebase verification
Inconsistent usage of @preconcurrency attribute with DomainLayer imports detected
The verification reveals inconsistent usage of the
@preconcurrencyattribute across the codebase:
- Only 20 files use
@preconcurrency import DomainLayer- Over 120 files use plain
import DomainLayerThis inconsistency could lead to potential thread-safety issues during the Swift 6 migration. All imports of DomainLayer should be marked with
@preconcurrencyas part of the concurrency improvements strategy.Key locations requiring updates:
- DataLayer module files (e.g.,
KeychainHelperService.swift,ApiClient.swift)- PresentationLayer module files (e.g.,
HistoryViewModel.swift,WeatherUnitsConverter.swift)- Core app files (e.g.,
MainApp.swift)🔗 Analysis chain
LGTM! Consistent with Swift 6 upgrade strategy.
The addition of
@preconcurrencyto the DomainLayer import aligns with the project's Swift 6 upgrade objectives and broader concurrency improvements.Let's verify consistent usage across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent @preconcurrency usage with DomainLayer imports # Expected: All DomainLayer imports should use @preconcurrency # Search for DomainLayer imports without @preconcurrency echo "Checking for non-preconcurrency DomainLayer imports:" rg "^import\s+DomainLayer\s*$" --type swift # Search for DomainLayer imports with @preconcurrency echo -e "\nChecking for preconcurrency DomainLayer imports:" rg "^@preconcurrency\s+import\s+DomainLayer\s*$" --type swiftLength of output: 17971
wxm-ios/Toolkit/Toolkit/Analytics/WXMAnalytics.swift (1)
10-10: 🛠️ Refactor suggestion
Consider adding synchronization for providers array
While marking the class as
@unchecked Sendable, it's important to ensure thread-safe access to shared mutable state.Consider adding synchronization to protect the
providersarray:public class WXMAnalytics: @unchecked Sendable { public static let shared = WXMAnalytics() - private var providers: [AnalyticsProviderImplementation] = [] + private let queue = DispatchQueue(label: "com.wxm.analytics", attributes: .concurrent) + private var _providers: [AnalyticsProviderImplementation] = [] + private var providers: [AnalyticsProviderImplementation] { + get { queue.sync { _providers } } + set { queue.async(flags: .barrier) { self._providers = newValue } } + }This ensures thread-safe access to the providers array while maintaining good performance through a concurrent queue with barrier writes.
Committable suggestion skipped: line range outside the PR's diff.
PresentationLayer/UIComponents/Modifiers/SizeObserver.swift (1)
47-47: 💡 Codebase verification
Warning: Inconsistent thread-safety annotations in the codebase
The verification revealed two different uses of
nonisolated(unsafe):
- In
SizeObserver.swift: Used safely with an immutable CGRect value- In
MockProtocol.swift: Used with a mutable dictionary[String: String], which is not thread-safeThe
MockProtocolusage is concerning because:
- The static dictionary can be modified at runtime
- Dictionary type is not thread-safe for concurrent access
- Mutable state should not be marked as
nonisolated(unsafe)🔗 Analysis chain
LGTM! The
nonisolated(unsafe)annotation is appropriate here.The addition of
nonisolated(unsafe)todefaultValueis safe because:
- The value is an immutable CGRect.zero
- CGRect is a value type, making it thread-safe
- The property is only read, never modified after initialization
Let's verify similar patterns across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for similar PreferenceKey implementations to ensure consistent concurrency annotations # Search for PreferenceKey conformances ast-grep --pattern 'struct $NAME: PreferenceKey { $$$ static var defaultValue$$$ $$$ }' # Search for other nonisolated(unsafe) usage patterns rg "nonisolated\(unsafe\) static var"Length of output: 385
Script:
#!/bin/bash # Let's check the context of both nonisolated(unsafe) usages and other PreferenceKey implementations # Check MockProtocol.swift context rg -B 5 -A 5 "nonisolated\(unsafe\) static var responses" # Find all PreferenceKey implementations and their defaultValue ast-grep --pattern 'struct $NAME: PreferenceKey { $$$ static var defaultValue: $TYPE = $VALUE $$$ }'Length of output: 1557
wxm-ios/DataLayer/DataLayer/Networking/Mock/MockProtocol.swift (1)
20-20:
⚠️ Potential issueAdd proper synchronization for shared dictionary
The
nonisolated(unsafe)modifier correctly indicates thread-safety concerns, but the staticresponsesdictionary needs proper synchronization as it's accessed from multiple threads.Consider implementing one of these solutions:
- nonisolated(unsafe) static var responses: [String: String] = [:] + private static let responsesQueue = DispatchQueue(label: "com.weatherxm.mockprotocol.responses") + private static var _responses: [String: String] = [:] + + nonisolated static var responses: [String: String] { + get { + responsesQueue.sync { _responses } + } + set { + responsesQueue.sync { _responses = newValue } + } + }Or using
actorfor Swift concurrency:- nonisolated(unsafe) static var responses: [String: String] = [:] + private static let responsesActor = ResponsesActor() + + private actor ResponsesActor { + var responses: [String: String] = [:] + + func get() -> [String: String] { + responses + } + + func set(_ newValue: [String: String]) { + responses = newValue + } + } + + static var responses: [String: String] { + get async { + await responsesActor.get() + } + set { + Task { + await responsesActor.set(newValue) + } + } + }Committable suggestion skipped: line range outside the PR's diff.
station-widget/StationTimelineEntry.swift (1)
10-10: 💡 Codebase verification
Several DomainLayer imports need @preconcurrency attribute
The verification revealed inconsistent usage of
@preconcurrencyattribute across the codebase. Many files importing DomainLayer lack the required attribute for Swift 6 compatibility:
- Most files in
wxm-ios/DataLayer/need the attribute- All files in
PresentationLayer/need the attribute- Only a few files like
station-widget/*and some repository implementations correctly use@preconcurrency🔗 Analysis chain
LGTM! Verify other DomainLayer imports.
The
@preconcurrencyattribute is correctly applied to handle potential data races when importing DomainLayer.Let's verify consistent usage across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of @preconcurrency with DomainLayer imports # Expect: All DomainLayer imports should use @preconcurrency # Search for DomainLayer imports without @preconcurrency echo "=== Checking for non-preconcurrency DomainLayer imports ===" rg "^import\s+DomainLayer\s*$" --type swift # Search for DomainLayer imports with @preconcurrency echo -e "\n=== Checking for preconcurrency DomainLayer imports ===" rg "^@preconcurrency\s+import\s+DomainLayer\s*$" --type swiftLength of output: 17999
wxm-ios/DataLayer/DataLayer/Networking/Interceptors/AuthInterceptor.swift (1)
14-14:
⚠️ Potential issueCritical: Unsafe concurrent access to mutable state
Using
@unchecked Sendablebypasses important thread-safety checks, which is risky for this class because:
- It contains mutable state (
cancellableSet)- It handles sensitive data (authentication tokens)
- It performs network operations
Consider these safer alternatives:
- Make the class thread-safe by wrapping mutable state in an actor:
private actor CancellableStorage { var cancellables: Set<AnyCancellable> = [] func store(_ cancellable: AnyCancellable) { cancellables.insert(cancellable) } }
- Or implement proper
Sendableconformance by synchronizing access to shared state:private let queue = DispatchQueue(label: "com.weatherxm.authinterceptor") private var _cancellableSet: Set<AnyCancellable> = [] private func storeCancellable(_ cancellable: AnyCancellable) { queue.sync { _cancellableSet.insert(cancellable) } }PresentationLayer/Utils/Weather/WeatherUnitsManager.swift (1)
13-17:
⚠️ Potential issueConsider safer alternatives to
nonisolated(unsafe)The
nonisolated(unsafe)attribute on the singleton property bypasses Swift's actor isolation checks, which could lead to race conditions when accessing shared state (UserDefaults, analytics) from multiple threads. Consider these safer alternatives:
- Use an actor to synchronize access:
actor WeatherUnitsManagerActor { static let shared = WeatherUnitsManagerActor() private let manager: WeatherUnitsManager private init() { let useCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MainUseCase.self)! self.manager = WeatherUnitsManager(mainUseCase: useCase) } func getManager() -> WeatherUnitsManager { return manager } }
- Or use a serial dispatch queue:
static let `default`: WeatherUnitsManager = { let queue = DispatchQueue(label: "com.weatherxm.unitsmanager") return queue.sync { let useCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MainUseCase.self)! return WeatherUnitsManager(mainUseCase: useCase) } }()wxm-ios/Toolkit/Toolkit/FirebaseManager/FirebaseManager.swift (1)
15-15:
⚠️ Potential issueFix typo in protocol name
FirbaseManagerImplementationThere appears to be a typo in the protocol name. It should be
FirebaseManagerImplementationinstead ofFirbaseManagerImplementation.- private let firebaseManagerImpl: FirbaseManagerImplementation + private let firebaseManagerImpl: FirebaseManagerImplementationThis should be fixed across all usages in the file.
Committable suggestion skipped: line range outside the PR's diff.
wxm-ios/Toolkit/Toolkit/FirebaseManager/RemoteConfigManager.swift (1)
157-157: 💡 Codebase verification
⚠️ Potential issueCritical: Thread-safety violation confirmed in RemoteConfigManager
The verification confirms critical thread-safety concerns:
- The class has 17
@Publishedproperties that must be updated on the main thread per SwiftUI requirementsupdateProperties()is called from Firebase callbacks throughshouldUpdateCallback- The current implementation removes the main thread dispatch, which can lead to SwiftUI runtime warnings or crashes
The changes must be reverted to maintain thread-safety:
func updateProperties() { - shouldUpdateCallback?() + DispatchQueue.main.async { [weak self] in + guard let self else { return } + self.shouldUpdateCallback?() + } }🔗 Analysis chain
Critical: Potential thread-safety violation in updateProperties
The removal of
DispatchQueue.main.asyncis concerning because:
RemoteConfigManageris anObservableObjectand its@Publishedproperties must be updated on the main thread- This method is called from Firebase callbacks which may execute on background threads
- The removal of weak self handling could introduce retain cycles
Consider restoring the previous implementation:
func updateProperties() { - shouldUpdateCallback?() + DispatchQueue.main.async { [weak self] in + guard let self else { return } + self.shouldUpdateCallback?() + } }Let's verify where this callback is used:
Let me gather more context about the class and its properties to verify the thread-safety concerns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for shouldUpdateCallback usage rg -A 5 "shouldUpdateCallback.*="Length of output: 617
Script:
#!/bin/bash # Search for RemoteConfigManager class definition and properties ast-grep --pattern 'class RemoteConfigManager: ObservableObject { $$$ }' # Search for @Published properties rg "@Published.*var" wxm-ios/Toolkit/Toolkit/FirebaseManager/RemoteConfigManager.swift # Search for where updateProperties is called rg -A 3 "updateProperties\(\)"Length of output: 2362
wxm-ios/Swinject/SwinjectHelper.swift (1)
8-9: 💡 Codebase verification
Inconsistent usage of @preconcurrency attribute across imports
The verification reveals inconsistent usage of the @preconcurrency attribute across the codebase:
- DataLayer: Only 1 out of 3 imports uses @preconcurrency
- DomainLayer: Only 19 out of 100+ imports use @preconcurrency
- Swinject: Only 1 out of 2 imports uses @preconcurrency
This inconsistency could lead to potential concurrency issues in Swift 6. The attribute should be consistently applied across all imports of these modules to ensure proper backward compatibility during the Swift 6 migration.
🔗 Analysis chain
LGTM! Appropriate use of @preconcurrency attribute.
The addition of @preconcurrency to these imports is correct for the Swift 6 upgrade, as it helps maintain backward compatibility while adopting Swift's new concurrency model.
Let's verify consistent usage across the codebase:
Also applies to: 11-11
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of @preconcurrency with these imports # Expected: All imports of these modules should use @preconcurrency echo "Checking DataLayer imports..." rg "^import DataLayer" --type swift rg "^@preconcurrency import DataLayer" --type swift echo "Checking DomainLayer imports..." rg "^import DomainLayer" --type swift rg "^@preconcurrency import DomainLayer" --type swift echo "Checking Swinject imports..." rg "^import Swinject" --type swift rg "^@preconcurrency import Swinject" --type swiftLength of output: 18487
wxm-ios/Toolkit/Toolkit.xcodeproj/project.pbxproj (1)
264-264: 💡 Codebase verification
Deployment target inconsistencies detected across modules
The project upgrade to Xcode 16.1.0 (build 1610) has revealed inconsistent deployment targets across different modules:
- Main app targets iOS 14.0-17.0 in different configurations
- Toolkit module targets iOS 15.0-16.2
- DataLayer, DomainLayer, and PresentationLayer modules target iOS 14.0-15.0
These inconsistencies could lead to runtime issues. Consider:
- Aligning deployment targets across all modules
- Setting a consistent minimum iOS version based on your app's requirements
- Updating any deprecated APIs if targeting newer iOS versions
🔗 Analysis chain
Project settings updated to latest Xcode version.
The LastUpgradeCheck has been updated from 1530 to 1610, indicating a project-wide upgrade to support the latest Xcode features and improvements.
Let's verify the minimum deployment target compatibility:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check deployment target settings rg "IPHONEOS_DEPLOYMENT_TARGET"Length of output: 3672
wxm-ios/DataLayer/DataLayer.xcodeproj/project.pbxproj (1)
722-723: 💡 Codebase verification
⚠️ Inconsistent Swift version configuration across modulesThe verification reveals inconsistencies in Swift version settings across the project modules:
- DataLayer: Swift 6.0 ✓
- Toolkit: Swift 6.0 ✓
- DomainLayer: Still on Swift 5.0
⚠️ - PresentationLayer: Still on Swift 5.0
⚠️ - Main project (wxm-ios): Mixed Swift 5.0 and 6.0 settings
⚠️ While strict concurrency checking is properly configured where Swift 6.0 is used, the inconsistent Swift versions across modules could lead to compatibility issues. All modules should be upgraded to Swift 6.0 to maintain consistency and ensure proper concurrency checking throughout the codebase.
🔗 Analysis chain
Swift 6 upgrade with strict concurrency checking
The changes appropriately configure Swift 6 and enable complete concurrency checking across all build configurations (Mock, Debug, Release). This is a good practice as it:
- Ensures consistent behavior across all configurations
- Helps catch potential concurrency issues at compile time
- Aligns with modern Swift concurrency best practices
Let's verify the Swift version requirements are consistent across the codebase:
Also applies to: 881-882, 917-918
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Swift version settings across all xcodeproj files # Expected: Should find SWIFT_VERSION = 6.0 consistently # Search for Swift version settings rg "SWIFT_VERSION = " --type xmlLength of output: 183
Script:
#!/bin/bash # Let's try a different approach to find Swift version settings in project files # First, let's find all xcodeproj files that might contain the configuration fd "project.pbxproj" # Then search for Swift version settings without type filter rg "SWIFT_VERSION = " $(fd "project.pbxproj") # Also search for strict concurrency settings rg "SWIFT_STRICT_CONCURRENCY = " $(fd "project.pbxproj")Length of output: 3728
wxm-ios/DataLayer/DataLayer/Networking/Interceptors/RequestHeadersAdapter.swift (1)
37-38:
⚠️ Potential issueRemove unnecessary
awaiton synchronous property accessProperties like
UIDevice.current.systemVersionandUIDevice.modelNameare synchronous and do not requireawait. Usingawaiton synchronous properties may cause compiler warnings or unnecessary overhead.Apply this diff to remove the unnecessary
awaitkeywords:- let systemVersion = await UIDevice.current.systemVersion + let systemVersion = UIDevice.current.systemVersion - let deviceInfo = "Device: \(await UIDevice.modelName)" + let deviceInfo = "Device: \(UIDevice.modelName)"Also applies to: 41-41
wxm-ios/Toolkit/Toolkit/FirebaseManager/NotificationsHandler.swift (1)
22-22:
⚠️ Potential issueReview the use of
nonisolated(unsafe)forcancellableSetThe attribute
nonisolated(unsafe)bypasses concurrency safety checks, which may lead to data races. It's crucial to confirm that accessingcancellableSetfrom concurrent contexts is thread-safe. Consider using concurrency-safe constructs like actors or locks to manage this mutable state.wxm-ios/DataLayer/DataLayer/RepositoryImplementations/DeviceInfoRepositoryImpl.swift (1)
45-53: 🛠️ Refactor suggestion
Handle potential
nilselfwhen using[weak self]inTaskclosureIn the
rebootStationmethod, theTaskclosure capturesselfweakly and usesself?when accessingbluetoothWrapper. Ifselfbecomesnil, the asynchronous operations will be skipped, and no further state will be emitted tovalueSubject, potentially leaving subscribers waiting indefinitely. Consider handling thenilcase explicitly by sending an appropriate error state tovalueSubjectto prevent indefinite waiting.Apply the following change to ensure proper handling:
Task { @MainActor [weak self, valueSubject] in valueSubject.send(.connect) + guard let self = self else { + valueSubject.send(.failed(.unknown)) + return + } if let error = await self.bluetoothWrapper.connect(device: device) { valueSubject.send(.failed(error.toRebootError)) return } valueSubject.send(.rebooting) if let error = await self.bluetoothWrapper.reboot(device: device) { valueSubject.send(.failed(error.toRebootError)) return } valueSubject.send(.finished) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Task { @MainActor [weak self, valueSubject] in valueSubject.send(.connect) guard let self = self else { valueSubject.send(.failed(.unknown)) return } if let error = await self.bluetoothWrapper.connect(device: device) { valueSubject.send(.failed(error.toRebootError)) return } valueSubject.send(.rebooting) if let error = await self.bluetoothWrapper.reboot(device: device) {station-widget/Station_widget.swift (2)
59-59:
⚠️ Potential issueAvoid using
nonisolated(unsafe)to prevent data racesUsing
nonisolated(unsafe)onstationcan lead to unsafe concurrent access. It's advisable to find a safer alternative to accessconfiguration.selectedStationwithout bypassing isolation checks.Suggested Change:
Access
stationwithin theTaskto maintain isolation:- nonisolated(unsafe) let station = configuration.selectedStation Task { @MainActor in + let station = configuration.selectedStation let entry = await getEntry(for: station, displaySize: displaySize) let timeline = Timeline(entries: [entry], policy: .after(nextUpdate)) completion(timeline) }Committable suggestion skipped: line range outside the PR's diff.
13-13: 💡 Codebase verification
⚠️ Potential issueBased on the findings, I can now provide the final response:
@unchecked Sendableusage requires attention for thread safetyThe verification reveals potential thread-safety issues:
CancellableWrappercontains a mutableSet<AnyCancellable>property but doesn't conform toSendableWidgetUseCasecontains aCancellableWrapperinstance and dependencies (meRepository,keychainRepository) but doesn't conform toSendable- Multiple other components in the codebase use
CancellableWrapperfor managing cancellables, suggesting this is a systemic concernTo ensure thread safety:
- Make
CancellableWrapperthread-safe by using appropriate synchronization and conforming toSendable- Verify that
MeRepositoryandKeychainRepositoryareSendable-compliant- Consider using
@MainActorif the widget only needs to run on the main thread🔗 Analysis chain
Verify the safety of
@unchecked SendableconformanceMarking
Provideras@unchecked Sendablebypasses the compiler's safety checks for concurrency. Ensure that all properties withinProvider, such asuseCaseandcancellableWrapper, are safe to use across concurrency domains or are themselvesSendable.Consider reviewing the properties for
Sendableconformance:private let useCase: WidgetUseCase private let cancellableWrapper: CancellableWrapper = .init()Run the following script to check
Sendableconformance of properties:
Let me gather more information about these types to better understand their implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that WidgetUseCase and CancellableWrapper conform to Sendable. # Check for Sendable conformance in WidgetUseCase ast-grep --pattern $'struct WidgetUseCase: Sendable { $$$ }' --json # Check for Sendable conformance in CancellableWrapper ast-grep --pattern $'final class CancellableWrapper: Sendable { $$$ }' --jsonLength of output: 153
Script:
#!/bin/bash # Check the implementation of WidgetUseCase ast-grep --pattern 'struct WidgetUseCase { $$$ }' # Check the implementation of CancellableWrapper ast-grep --pattern 'class CancellableWrapper { $$$ }' # Search for any other references to these types rg "WidgetUseCase|CancellableWrapper" -A 3Length of output: 9385
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/Bluetooth/BTActionsWrapper.swift (1)
14-14:
⚠️ Potential issue
BTActionWrappercontains non-Sendable properties but is marked as@unchecked SendableMarking
BTActionWrapperas@unchecked Sendableasserts that it's safe to share instances across concurrency domains without compiler verification. However, the class contains several non-Sendable properties:
cancellables: Set<AnyCancellable>:AnyCancellabledoes not conform toSendable.scanningTimeoutWorkItem: DispatchWorkItem?:DispatchWorkItemis a reference type and notSendable.rebootStationWorkItem: DispatchWorkItem?: Same as above.performCommandDelegate: BTPerformCommandDelegate?: UnlessBTPerformCommandDelegateconforms toSendable, this could be unsafe.bluetoothManager: BluetoothManager: Ensure thatBluetoothManagerand its properties areSendable.Using these non-Sendable types without proper synchronization can lead to data races and undefined behavior. Consider one of the following:
- Refactor the class to remove or safely handle non-Sendable properties, possibly by enforcing thread confinement or adding synchronization mechanisms.
- Avoid marking the class as
@unchecked Sendableuntil it can be safely shared across concurrency domains.Apply this diff to remove the
@unchecked Sendableannotation for now:-class BTActionWrapper: @unchecked Sendable { +class BTActionWrapper {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.class BTActionWrapper {wxm-ios/DataLayer/UserDevicesService.swift (2)
19-19:
⚠️ Potential issueEnsure Thread Safety when using
@unchecked SendableBy declaring
UserDevicesServiceas@unchecked Sendable, you're asserting that it is safe for use across concurrency domains without compiler enforcement. Please ensure that all properties and methods within this class are thread-safe to prevent potential data races or concurrency issues.
184-184:
⚠️ Potential issueUse
.storeThreadSafeinstead of.storefor thread safetyReplace
.store(in: &cancellableSet)with.storeThreadSafe(in: &cancellableSet)to ensure thread-safe access tocancellableSet, consistent with its usage elsewhere in the class.Apply this diff to fix the issue:
- .store(in: &cancellableSet) + .storeThreadSafe(in: &cancellableSet)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements..storeThreadSafe(in: &cancellableSet)wxm-ios.xcodeproj/project.pbxproj (1)
3535-3536: 💡 Codebase verification
Swift concurrency safety needs attention -
@unchecked Sendableusage detectedSeveral classes and structs are marked with
@unchecked Sendable, which bypasses the compiler's strict concurrency checking. This could lead to data races and thread safety issues. Key locations that need review:
- Core Services:
WXMAnalytics,Logger,FirebaseManager,RemoteConfigManager- Data Layer:
UserInfoService,UserDevicesService,DeviceInfoRepositoryImpl,FirmwareUpdateImpl- Network components:
AuthInterceptor,RequestHeadersAdapter,MockProtocol- Bluetooth:
BTActionWrapper- Widget Components:
Provider,StationTimelineEntry- Domain Models:
DeviceDetailsConsider:
- Implementing proper thread-safe mechanisms instead of using
@unchecked- Ensuring all mutable state is properly synchronized
- Using actors or other Swift concurrency features where appropriate
🔗 Analysis chain
Verify the strict concurrency setting across the codebase.
Setting
SWIFT_STRICT_CONCURRENCYtocompleteis a good practice for ensuring safe concurrency. However, please ensure that all the code in the project adheres to this strict setting to avoid any potential runtime issues.Run the following script to verify strict concurrency adherence:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify strict concurrency adherence across Swift files. # Test: Search for occurrences of `@unchecked` attributes that bypass strict concurrency checks. # Expect: No concerning usages of `@unchecked`. rg --type swift $'@unchecked\s+(Sendable|MainActor)'Length of output: 1856
pantelisss
changed the base branch from
feature/fe-1352-upgrade-swift-version-to-6-for-widget-target
to
feature/swift_6_upgrade
pantelisss
deleted the
feature/fe-1351-upgrade-swift-version-to-6-for-datalayer-target
branch
Why?
Upgrade Data layer to Swif 6
How?
Fixed all errors regarding the Swift 6
Testing
Ensure the app builds and works as expected
Additional context
Fixes FE-1351
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores