[#701] Add foundation for analytics tracking#702
[#701] Add foundation for analytics tracking#702thinh2k1310 wants to merge 3 commits intodevelopfrom
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive analytics framework for the iOS template. It adds core abstractions ( Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (Startup)
participant Analytics as Analytics.shared
participant ConsoleTracker as ConsoleAnalyticsTracker
participant ViewModel as LandingViewModel
participant Trackers as Registered Trackers
App->>Analytics: configure(trackers: [consoleTracker], parameters)
Analytics->>ConsoleTracker: setUp(additionalParameters)
ConsoleTracker-->>Analytics: prints setup message
ViewModel->>ViewModel: continueWithDemoSession()
ViewModel->>Analytics: trackEvent(UserLoginEvent)
Analytics->>Trackers: trackEvent(name: "user_login", parameters)
ConsoleTracker-->>ConsoleTracker: prints event with parameters
Trackers-->>ViewModel: event recorded
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
thinh2k1310
changed the title
thinh2k1310
requested review from
Shayokh144,
Thieurom,
donmartial,
dquangit,
ducbm051291,
kevinhoangpq,
markgravity,
minhnimble,
nhgia,
nkhanh44,
nmint8m,
patcharapon-j,
phongvhd93,
roman-nimble,
suho and
vnntsu
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (12)
template/Tuist/Interfaces/SwiftUI/Sources/Application/Dependencies/Container+Application.swift (1)
8-10: Optional:.singletonis redundant here.Since
Analytics.sharedis already a process-wide singleton, the.singletonscope on the factory adds no behavior. Harmless, but you could drop it for clarity. Keeping it is also fine if you want consistency with other container scopes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Tuist/Interfaces/SwiftUI/Sources/Application/Dependencies/Container`+Application.swift around lines 8 - 10, The analytics Factory currently wraps Analytics.shared and then applies .singleton which is redundant; update the factory provider for analytics (the var analytics: Factory<AnalyticsProtocol> block) to return self { Analytics.shared } without the .singleton scope (or leave as-is if you prefer consistency), so remove the trailing .singleton from the factory definition referencing Analytics.shared.template/Tuist/Interfaces/SwiftUI/Sources/Application/App.swift (1)
7-16: Release builds ship with zero trackers — document the follow-up.The
#elsebranch replaces trackers with[], so Release builds emit no analytics at all. That’s reasonable for a foundation PR (no real SDK adapters yet), but please either:
- Add a
// TODO:here pointing to the follow-up that wires real trackers (Firebase/AppsFlyer/Facebook per the PR description), or- Drop the
configure(trackers: [], …)call entirely and let Release default to “no trackers” implicitly, so the intent is obvious.As-is, a future change that adds a production tracker only to the
#if DEBUGbranch could easily ship a silent-analytics Release without catching it in review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Tuist/Interfaces/SwiftUI/Sources/Application/App.swift` around lines 7 - 16, The Release branch currently calls Analytics.shared.configure(trackers: [], additionalParameters: nil) which explicitly sets zero trackers and can be confusing; either remove that call so Release implicitly has no trackers or add a clear TODO comment pointing to the follow-up work to wire production trackers (Firebase/AppsFlyer/Facebook) — update the init() near Analytics.shared.configure (and ConsoleAnalyticsTracker reference) to either delete the `#else` configure call or replace it with a single-line // TODO: wire production trackers (link-to-issue/PR) so reviewers know this is intentional.template/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swift (1)
1-5: Framework and filename inconsistent with the rest of the Analytics test suite.Based on learnings, project convention is
.Spec.swiftfor Quick/Nimble suites and.Tests.swiftfor Swift Testing suites. This file is namedMockAnalyticsTrackerSpec.swiftbut uses Swift Testing (@Suite,@Test,#expect). Per the AI summary, the neighboringAnalyticsSpec.swiftis Quick-based.Pick one to stay consistent:
- Rename to
MockAnalyticsTrackerTests.swiftif you want to keep Swift Testing here, and consider migratingAnalyticsSpec.swiftas well for uniformity; or- Rewrite this as a
QuickSpecwith Nimble matchers to matchAnalyticsSpec.swift.Mixing both frameworks inside a single module makes it harder for contributors to know which convention applies when adding new specs.
Based on learnings: “Quick/Nimble specs use
.Spec.swift, while Swift Testing files use.Tests.swift.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swift` around lines 1 - 5, The file MockAnalyticsTrackerSpec.swift is using Swift Testing annotations (`@Suite`, `@Test`, `#expect`) but is named with the .Spec.swift convention; rename the file to MockAnalyticsTrackerTests.swift and keep the Swift Testing constructs (retain `@Suite`("MockAnalyticsTracker Tests") and any `@Test` methods) so the filename matches the framework; ensure imports/testing annotations remain unchanged and run the test target to verify discovery; if you prefer to standardize on Quick/Nimble instead, rewrite the suite as a QuickSpec (matching AnalyticsSpec.swift) rather than renaming.template/Tuist/Interfaces/SwiftUI/Sources/Presentation/Modules/Landing/LandingViewModel.swift (1)
49-64: Optional: consolidate the tracking call to avoid duplication.The two branches only differ in the
isSuccessfulflag. Emitting once after the do/catch keeps the tracking concern in one place and makes it harder to forget to track when another code path is added later.♻️ Suggested refactor
func continueWithDemoSession() async { - do { - try await sessionRepository.save(tokenSet: DemoTokenSet()) - state = .signedIn - - // Track successful login event - let loginEvent = UserLoginEvent(loginMethod: "demo", isSuccessful: true) - analytics.trackEvent(loginEvent) - } catch { - state = .signedOut - - // Track failed login event - let loginEvent = UserLoginEvent(loginMethod: "demo", isSuccessful: false) - analytics.trackEvent(loginEvent) - } + let isSuccessful: Bool + do { + try await sessionRepository.save(tokenSet: DemoTokenSet()) + state = .signedIn + isSuccessful = true + } catch { + state = .signedOut + isSuccessful = false + } + analytics.trackEvent(UserLoginEvent(loginMethod: "demo", isSuccessful: isSuccessful)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Tuist/Interfaces/SwiftUI/Sources/Presentation/Modules/Landing/LandingViewModel.swift` around lines 49 - 64, In continueWithDemoSession(), avoid duplicating analytics.trackEvent by computing a success flag inside the do/catch: set state = .signedIn and success = true in the do block after await sessionRepository.save(tokenSet: DemoTokenSet()), set state = .signedOut and success = false in the catch, then create one UserLoginEvent(loginMethod: "demo", isSuccessful: success) and call analytics.trackEvent(...) once after the do/catch; this keeps tracking centralized and prevents forgetting to emit the event in future paths.template/Modules/Analytics/Sources/AnalyticsTrackerType.swift (1)
1-4: AddSendableconformance toAnalyticsTrackerType.This public enum crosses actor boundaries and requires explicit
Sendableconformance under strict concurrency checking. Public non-frozen enums in Swift 6 don't automatically inferSendableeven with Sendable raw values likeString.-public enum AnalyticsTrackerType: String, CaseIterable { +public enum AnalyticsTrackerType: String, CaseIterable, Sendable {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Sources/AnalyticsTrackerType.swift` around lines 1 - 4, The public enum AnalyticsTrackerType must explicitly declare Sendable conformance because it crosses actor boundaries; update the declaration for AnalyticsTrackerType to add Sendable (e.g., public enum AnalyticsTrackerType: String, CaseIterable, Sendable) so the type is explicitly Sendable under strict concurrency checking, and ensure any related usages respect Sendable requirements.template/{PROJECT_NAME}Tests/Sources/Specs/Presentation/Modules/Landing/LandingViewModelTests.swift (1)
193-226: Consider extractingAnalyticsProtocolMockto a shared test utility.This mock will likely be needed by any ViewModel test that tracks analytics. Defining it locally per test file will lead to duplication as more screens get analytics integration. Consider hoisting it into a shared test helpers module (e.g. alongside the other
*Mocktypes used here) so future tests can reuse it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/`{PROJECT_NAME}Tests/Sources/Specs/Presentation/Modules/Landing/LandingViewModelTests.swift around lines 193 - 226, Extract the AnalyticsProtocolMock class into the shared test helpers module used by other ViewModel tests: create a new test helper file containing AnalyticsProtocolMock (preserving trackedStructEvents, trackEvent(_:) and trackEvent(_:on:)), give it appropriate access level (internal/public) so tests can use it, add it to the test target and import the helpers where needed, and remove the local definition from LandingViewModelTests to avoid duplication.template/Modules/Analytics/Sources/Events/UserLoginEvent.swift (1)
6-12: Consider a type-safeloginMethodto align with issue#701's AnalyticsParameter goal.Issue
#701's acceptance criteria calls for a type-safeAnalyticsParameterwrapper. Since this event is meant as the canonical example for consumers, modellingloginMethodas an enum (e.g..demo,.social) — with arawValueused inparameters— would both guide future events and avoid typo-prone string constants in call sites likeLandingViewModel.continueWithDemoSession().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Sources/Events/UserLoginEvent.swift` around lines 6 - 12, Change the loginMethod property on UserLoginEvent from String to a type-safe enum (e.g., enum LoginMethod: String { case demo, email, social, ... }) and update the public init(loginMethod:isSuccessful:) to accept LoginMethod instead of String; keep isSuccessful default as before. Update any places that construct analytics parameters (e.g., parameters building code or AnalyticsParameter usage) to pass loginMethod.rawValue when a String is required by the analytics API so call sites use UserLoginEvent(loginMethod: .demo) rather than string literals. Ensure the new LoginMethod enum is public and documented near UserLoginEvent so consumers can use the typed values.template/Modules/Analytics/Sources/AnalyticsProtocol.swift (1)
1-30: Consider makingAnalyticsProtocolclass-bound.The only production implementation (
Analytics) is afinal classholding mutable tracker state and exposed as a singleton — it's inherently reference-semantic. Constraining the protocol toAnyObjectwould:
- Allow identity comparisons (
===) that the test suite already wants to do (seeAnalyticsSpec.swift's singleton test).- Prevent accidental value-type conformers that would silently copy the tracker list on every call site.
-public protocol AnalyticsProtocol { +public protocol AnalyticsProtocol: AnyObject {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Sources/AnalyticsProtocol.swift` around lines 1 - 30, Make AnalyticsProtocol class-bound by adding the class-only constraint (conform to AnyObject) so implementations like the final class Analytics remain reference types; update the protocol declaration (AnalyticsProtocol) to inherit from AnyObject and adjust any test/mocks (e.g., AnalyticsSpec.swift singleton expectations or mock types) to conform to the new class-bound protocol if needed so identity comparisons (===) and reference semantics work as intended.template/Modules/Analytics/Sources/ConsoleAnalyticsTracker.swift (1)
17-17:additionalParameters?.descriptionproduces unstable output.
Dictionary.descriptionhas no guaranteed key order and formats values viaString(describing:), which is fine for debugging but noisy. Matching thetrackEventformatting (key: valuepairs joined by,) here would make setup logs consistent with event logs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Sources/ConsoleAnalyticsTracker.swift` at line 17, Replace the unstable use of additionalParameters?.description with a deterministic, trackEvent-consistent formatting: if additionalParameters is non-nil, sort its keys, map each (key, value) to "key: value" using String(describing: value), join pairs with ", " and assign that to paramsString; otherwise use "none". Update the code around the paramsString assignment (referencing additionalParameters and paramsString) so logs match the formatting used by trackEvent.template/Modules/Analytics/Tests/Sources/Events/AnalyticsEventsSpec.swift (1)
1-23: Filename suffix is inconsistent with the test framework.This file uses Swift Testing (
@Suite/@Test) but is namedAnalyticsEventsSpec.swift. Per project convention, Quick/Nimble specs use.Spec.swiftwhile Swift Testing files use.Tests.swift. Consider renaming toUserLoginEventTests.swift(orAnalyticsEventsTests.swift) to match the framework in use — the same applies to the companionAnalyticsSpec.swiftandMockAnalyticsTrackerSpec.swiftfiles.Based on learnings: "Quick/Nimble specs use
.Spec.swift, while Swift Testing files use.Tests.swift."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Tests/Sources/Events/AnalyticsEventsSpec.swift` around lines 1 - 23, The test file is using Swift Testing annotations (`@Suite`, `@Test`) but has a .Spec.swift suffix; rename AnalyticsEventsSpec.swift to a .Tests.swift name (e.g., UserLoginEventTests.swift or AnalyticsEventsTests.swift) and likewise rename companion files (AnalyticsSpec.swift -> AnalyticsTests.swift, MockAnalyticsTrackerSpec.swift -> MockAnalyticsTrackerTests.swift) so the framework and file suffix match; update any test target references or import entries that reference the old filenames and keep the struct UserLoginEventTests and its test methods createEventWithCorrectParameters and handleFailedLogin unchanged.template/Modules/Analytics/Sources/Analytics.swift (2)
23-30: Minor: redundant filter when dispatching to all tracker types.The no-
on:overloads passAnalyticsTrackerType.allCasesand thenfilter { trackerTypes.contains($0.type) }— every tracker matches by construction, so the filter is pure overhead (and allocates an intermediate array). Consider either forwarding directly totrackers.forEachin the unfiltered path, or, if you prefer the single funnel, making the filtered overload accept anOptional<[AnalyticsTrackerType]>wherenilmeans "all". Applies equally totrackScreen,setUserProperty, andsetUserId.♻️ Proposed refactor
- public func trackEvent(name: String, parameters: [String: Any]?) { - trackEvent(name: name, parameters: parameters, on: AnalyticsTrackerType.allCases) - } - - public func trackEvent(name: String, parameters: [String: Any]?, on trackerTypes: [AnalyticsTrackerType]) { - let targetTrackers = trackers.filter { trackerTypes.contains($0.type) } - targetTrackers.forEach { $0.trackEvent(name: name, parameters: parameters) } - } + public func trackEvent(name: String, parameters: [String: Any]?) { + trackers.forEach { $0.trackEvent(name: name, parameters: parameters) } + } + + public func trackEvent(name: String, parameters: [String: Any]?, on trackerTypes: [AnalyticsTrackerType]) { + let allowed = Set(trackerTypes) + trackers + .filter { allowed.contains($0.type) } + .forEach { $0.trackEvent(name: name, parameters: parameters) } + }Using
Setalso drops the per-trackercontainscost from O(m) to O(1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Sources/Analytics.swift` around lines 23 - 30, The current overload trackEvent(name:parameters:) forwards AnalyticsTrackerType.allCases into trackEvent(name:parameters:on:) and then filters trackers with trackerTypes.contains($0.type), causing an unnecessary allocation and loop when all trackers are intended; update the implementations to skip filtering when trackerTypes == AnalyticsTrackerType.allCases by directly calling trackers.forEach { $0.trackEvent(...) } in that case, or change the filtered overload signature to accept trackerTypes: [AnalyticsTrackerType]? (nil == all) and branch to trackers.forEach when nil; apply the same pattern to trackScreen, setUserProperty, and setUserId, and if you keep a contains-based path convert trackerTypes to a Set for O(1) lookups to reduce cost.
11-14:configurereplaces trackers without tearing down the previous set.If
configure(trackers:)is ever called more than once (e.g., environment switch, test setup), the previously-registered trackers are silently dropped with no symmetricaltearDown/resethook. Given the current app flow only configures once at launch this is low risk, but worth either documenting as "call once" or adding a teardown step so third-party SDKs (Firebase/AppsFlyer/Facebook) can release resources.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/Modules/Analytics/Sources/Analytics.swift` around lines 11 - 14, The configure(trackers:additionalParameters:) method currently replaces self.trackers without cleaning up the previous trackers; before assigning the new trackers call a teardown/reset on the old ones (e.g., iterate self.trackers and invoke a tearDown()/reset() method or make that method optional on the AnalyticsTracker protocol if it doesn't exist), then assign self.trackers = trackers and call setUp(additionalParameters:) on the new ones; this ensures third‑party SDKs are properly released when configure(trackers:) is called multiple times.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@template/Modules/Analytics/Sources/Analytics.swift`:
- Around line 5-19: The trackers array is accessed without synchronization;
protect all mutations and reads by adding a private serial DispatchQueue (e.g.,
let trackersQueue = DispatchQueue(label: "Analytics.trackers")) and use it to
serialize access in configure(trackers:), addTracker(_:,), and all reader
methods (trackEvent, trackScreen, setUserProperty, setUserId, tracker(for:)) —
perform writes with trackersQueue.async or .sync as appropriate and perform
reads by capturing the needed snapshot via trackersQueue.sync { return
trackers.filter { ... } } so callers operate on a stable copy; alternatively, if
you choose not to add synchronization, add a clear "Thread Safety" comment
documenting that configure() and addTracker() must only be called from the main
thread during app initialization.
In `@template/Modules/Analytics/Sources/AnalyticsEvent.swift`:
- Around line 1-5: Change the untyped parameters on AnalyticsEvent to a
type-safe wrapper: add an enum AnalyticsParameter (cases like string, int,
double, bool, date) that conforms to Sendable, update the protocol signature
from var parameters: [String: Any]? to var parameters: [String:
AnalyticsParameter]?, and update all implementations (e.g.,
ConsoleAnalyticsTracker) to convert AnalyticsParameter to the tracker/SDK-native
representation in one place; ensure AnalyticsParameter and AnalyticsEvent
conform to Sendable where appropriate and update any callers/tests to construct
AnalyticsParameter values instead of raw Any.
In `@template/Modules/Analytics/Sources/AnalyticsTrackerType.swift`:
- Around line 1-4: The enum AnalyticsTrackerType currently only defines case
.console but tests create MockAnalyticsTracker(type: .firebase), causing build
failures; either update all test usages (e.g., MockAnalyticsTracker(type:
.firebase)) to use .console, or add the missing cases to the enum by extending
AnalyticsTrackerType to include .firebase, .appsFlyer, and .facebook (keep the
rawValue strings as "firebase", "appsFlyer", "facebook" to match test/PR
expectations) so MockAnalyticsTracker(type: .firebase) compiles; pick one
approach and apply it consistently across the tests and enum declarations (refer
to AnalyticsTrackerType and MockAnalyticsTracker(type: .firebase) when making
the change).
In `@template/Modules/Analytics/Sources/ConsoleAnalyticsTracker.swift`:
- Around line 1-50: The ConsoleAnalyticsTracker currently uses print in methods
(setUp, trackEvent, trackScreen, setUserProperty, setUserId) which can leak PII
if used in release; replace print usage by either (a) wrapping the entire
implementation body in `#if` DEBUG/#endif so ConsoleAnalyticsTracker becomes a
no-op in release, or (b) switch to os.Logger: import os, create a private Logger
instance on the ConsoleAnalyticsTracker type, and log messages with
logger.info/logger.debug using interpolations marked with .private for dynamic
values (e.g., event names, parameters, userId) to ensure redaction; apply this
change across setUp(additionalParameters:), trackEvent(name:parameters:),
trackScreen(name:screenClass:), setUserProperty(key:value:) and setUserId(_:)
and remove raw print calls.
In `@template/Modules/Analytics/Sources/MockAnalyticsTracker.swift`:
- Around line 1-64: The MockAnalyticsTracker class is shipping in the production
Analytics module; either move the MockAnalyticsTracker type (public final class
MockAnalyticsTracker) out of template/Modules/Analytics/Sources into a test-only
target (e.g., AnalyticsTests or a new AnalyticsTestSupport target) so it is not
compiled into release builds, or wrap the entire file with a compile-time guard
(`#if` DEBUG ... `#endif`) to exclude it from release; after moving/wrapping ensure
the symbol MockAnalyticsTracker is no longer exported from the Analytics module
and update any test imports to reference the new test target or the conditional
compilation path.
In `@template/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swift`:
- Around line 25-47: The tests fail because
Analytics.configure(trackers:additionalParameters:) and
Analytics.addTracker(_:additionalParameters:) only store trackers and params but
never call the trackers' setUp(additionalParameters:) API; update the Analytics
class so configure assigns self.trackers and self.additionalParameters then
calls setUp(additionalParameters:) on each tracker (e.g., iterate trackers and
call setUp), and make addTracker append the tracker and immediately call
tracker.setUp(additionalParameters:) using the provided additionalParameters.
- Around line 76-87: The test trackEventWithoutParameters references and asserts
on trackers that are not declared or registered: declare mockFirebaseTracker
(e.g., let mockFirebaseTracker = MockAnalyticsTracker(type: .firebase)),
register both mockAppsFlyerTracker and mockFirebaseTracker with the SUT when
calling sut.configure(trackers: [...]), and update the assertions to expect
those declared mocks (use mockFirebaseTracker and mockAppsFlyerTracker). Also
add .appsFlyer and .firebase cases to the AnalyticsTrackerType enum so
MockAnalyticsTracker(type: .appsFlyer) and .firebase compile; ensure
MockAnalyticsTracker's initializer supports the new enum values.
- Around line 118-128: The test declares mockAppsFlyerTracker but never
registers it and uses a tracker type (.appsFlyer) that doesn't exist, so the
test is vacuous and may not compile; fix by adding an .appsFlyer case to
AnalyticsTrackerType (or the enum used), register both trackers with the SUT via
Analytics.configure(trackers: [mockConsoleTracker, mockAppsFlyerTracker]), call
sut.trackScreen(name: "ProfileScreen", screenClass: nil, on: [.appsFlyer]) and
update the assertions to assert
mockAppsFlyerTracker.hasTrackedScreen("ProfileScreen") is true and
mockConsoleTracker.hasTrackedScreen("ProfileScreen") is false so the test
actually validates filtering behavior for Analytics.trackScreen.
- Around line 9-21: The tests fail because Analytics.shared is typed as
AnalyticsProtocol so identity (===) and the `is` check are wrong; update
AnalyticsSpec.swift: in analyticsSingleton(), cast the shared instances to
AnyObject (e.g. let instance1 = Analytics.shared as AnyObject and same for
instance2) before using === to test singleton identity; in
analyticsProtocolConformance(), avoid the tautology by checking against the
concrete type (e.g. expect(Analytics.shared is Analytics)) or remove the
redundant test — alternatively, if you prefer making the protocol class-only,
change AnalyticsProtocol to `public protocol AnalyticsProtocol: AnyObject` in
AnalyticsProtocol.swift so the original code compiles.
In
`@template/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swift`:
- Line 11: Tests instantiate MockAnalyticsTracker(type: .firebase) but
AnalyticsTrackerType only defines .console, causing build failures; update the
enum AnalyticsTrackerType (in AnalyticsTrackerType.swift) to include a .firebase
case (and any associated logic or switch exhaustiveness updates) so
MockAnalyticsTracker(type: .firebase) compiles, or alternatively change all test
usages of MockAnalyticsTracker to use .console if you prefer not to add a
firebase case—locate references to MockAnalyticsTracker and the enum name
AnalyticsTrackerType to implement the chosen fix.
---
Nitpick comments:
In
`@template/`{PROJECT_NAME}Tests/Sources/Specs/Presentation/Modules/Landing/LandingViewModelTests.swift:
- Around line 193-226: Extract the AnalyticsProtocolMock class into the shared
test helpers module used by other ViewModel tests: create a new test helper file
containing AnalyticsProtocolMock (preserving trackedStructEvents, trackEvent(_:)
and trackEvent(_:on:)), give it appropriate access level (internal/public) so
tests can use it, add it to the test target and import the helpers where needed,
and remove the local definition from LandingViewModelTests to avoid duplication.
In `@template/Modules/Analytics/Sources/Analytics.swift`:
- Around line 23-30: The current overload trackEvent(name:parameters:) forwards
AnalyticsTrackerType.allCases into trackEvent(name:parameters:on:) and then
filters trackers with trackerTypes.contains($0.type), causing an unnecessary
allocation and loop when all trackers are intended; update the implementations
to skip filtering when trackerTypes == AnalyticsTrackerType.allCases by directly
calling trackers.forEach { $0.trackEvent(...) } in that case, or change the
filtered overload signature to accept trackerTypes: [AnalyticsTrackerType]? (nil
== all) and branch to trackers.forEach when nil; apply the same pattern to
trackScreen, setUserProperty, and setUserId, and if you keep a contains-based
path convert trackerTypes to a Set for O(1) lookups to reduce cost.
- Around line 11-14: The configure(trackers:additionalParameters:) method
currently replaces self.trackers without cleaning up the previous trackers;
before assigning the new trackers call a teardown/reset on the old ones (e.g.,
iterate self.trackers and invoke a tearDown()/reset() method or make that method
optional on the AnalyticsTracker protocol if it doesn't exist), then assign
self.trackers = trackers and call setUp(additionalParameters:) on the new ones;
this ensures third‑party SDKs are properly released when configure(trackers:) is
called multiple times.
In `@template/Modules/Analytics/Sources/AnalyticsProtocol.swift`:
- Around line 1-30: Make AnalyticsProtocol class-bound by adding the class-only
constraint (conform to AnyObject) so implementations like the final class
Analytics remain reference types; update the protocol declaration
(AnalyticsProtocol) to inherit from AnyObject and adjust any test/mocks (e.g.,
AnalyticsSpec.swift singleton expectations or mock types) to conform to the new
class-bound protocol if needed so identity comparisons (===) and reference
semantics work as intended.
In `@template/Modules/Analytics/Sources/AnalyticsTrackerType.swift`:
- Around line 1-4: The public enum AnalyticsTrackerType must explicitly declare
Sendable conformance because it crosses actor boundaries; update the declaration
for AnalyticsTrackerType to add Sendable (e.g., public enum
AnalyticsTrackerType: String, CaseIterable, Sendable) so the type is explicitly
Sendable under strict concurrency checking, and ensure any related usages
respect Sendable requirements.
In `@template/Modules/Analytics/Sources/ConsoleAnalyticsTracker.swift`:
- Line 17: Replace the unstable use of additionalParameters?.description with a
deterministic, trackEvent-consistent formatting: if additionalParameters is
non-nil, sort its keys, map each (key, value) to "key: value" using
String(describing: value), join pairs with ", " and assign that to paramsString;
otherwise use "none". Update the code around the paramsString assignment
(referencing additionalParameters and paramsString) so logs match the formatting
used by trackEvent.
In `@template/Modules/Analytics/Sources/Events/UserLoginEvent.swift`:
- Around line 6-12: Change the loginMethod property on UserLoginEvent from
String to a type-safe enum (e.g., enum LoginMethod: String { case demo, email,
social, ... }) and update the public init(loginMethod:isSuccessful:) to accept
LoginMethod instead of String; keep isSuccessful default as before. Update any
places that construct analytics parameters (e.g., parameters building code or
AnalyticsParameter usage) to pass loginMethod.rawValue when a String is required
by the analytics API so call sites use UserLoginEvent(loginMethod: .demo) rather
than string literals. Ensure the new LoginMethod enum is public and documented
near UserLoginEvent so consumers can use the typed values.
In `@template/Modules/Analytics/Tests/Sources/Events/AnalyticsEventsSpec.swift`:
- Around line 1-23: The test file is using Swift Testing annotations (`@Suite`,
`@Test`) but has a .Spec.swift suffix; rename AnalyticsEventsSpec.swift to a
.Tests.swift name (e.g., UserLoginEventTests.swift or
AnalyticsEventsTests.swift) and likewise rename companion files
(AnalyticsSpec.swift -> AnalyticsTests.swift, MockAnalyticsTrackerSpec.swift ->
MockAnalyticsTrackerTests.swift) so the framework and file suffix match; update
any test target references or import entries that reference the old filenames
and keep the struct UserLoginEventTests and its test methods
createEventWithCorrectParameters and handleFailedLogin unchanged.
In
`@template/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swift`:
- Around line 1-5: The file MockAnalyticsTrackerSpec.swift is using Swift
Testing annotations (`@Suite`, `@Test`, `#expect`) but is named with the .Spec.swift
convention; rename the file to MockAnalyticsTrackerTests.swift and keep the
Swift Testing constructs (retain `@Suite`("MockAnalyticsTracker Tests") and any
`@Test` methods) so the filename matches the framework; ensure imports/testing
annotations remain unchanged and run the test target to verify discovery; if you
prefer to standardize on Quick/Nimble instead, rewrite the suite as a QuickSpec
(matching AnalyticsSpec.swift) rather than renaming.
In `@template/Tuist/Interfaces/SwiftUI/Sources/Application/App.swift`:
- Around line 7-16: The Release branch currently calls
Analytics.shared.configure(trackers: [], additionalParameters: nil) which
explicitly sets zero trackers and can be confusing; either remove that call so
Release implicitly has no trackers or add a clear TODO comment pointing to the
follow-up work to wire production trackers (Firebase/AppsFlyer/Facebook) —
update the init() near Analytics.shared.configure (and ConsoleAnalyticsTracker
reference) to either delete the `#else` configure call or replace it with a
single-line // TODO: wire production trackers (link-to-issue/PR) so reviewers
know this is intentional.
In
`@template/Tuist/Interfaces/SwiftUI/Sources/Application/Dependencies/Container`+Application.swift:
- Around line 8-10: The analytics Factory currently wraps Analytics.shared and
then applies .singleton which is redundant; update the factory provider for
analytics (the var analytics: Factory<AnalyticsProtocol> block) to return self {
Analytics.shared } without the .singleton scope (or leave as-is if you prefer
consistency), so remove the trailing .singleton from the factory definition
referencing Analytics.shared.
In
`@template/Tuist/Interfaces/SwiftUI/Sources/Presentation/Modules/Landing/LandingViewModel.swift`:
- Around line 49-64: In continueWithDemoSession(), avoid duplicating
analytics.trackEvent by computing a success flag inside the do/catch: set state
= .signedIn and success = true in the do block after await
sessionRepository.save(tokenSet: DemoTokenSet()), set state = .signedOut and
success = false in the catch, then create one UserLoginEvent(loginMethod:
"demo", isSuccessful: success) and call analytics.trackEvent(...) once after the
do/catch; this keeps tracking centralized and prevents forgetting to emit the
event in future paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7b37830-036e-4c07-abd5-170212d82351
📒 Files selected for processing (18)
template/Modules/Analytics/Sources/Analytics.swifttemplate/Modules/Analytics/Sources/AnalyticsEvent.swifttemplate/Modules/Analytics/Sources/AnalyticsProtocol.swifttemplate/Modules/Analytics/Sources/AnalyticsTracker.swifttemplate/Modules/Analytics/Sources/AnalyticsTrackerType.swifttemplate/Modules/Analytics/Sources/ConsoleAnalyticsTracker.swifttemplate/Modules/Analytics/Sources/Events/UserLoginEvent.swifttemplate/Modules/Analytics/Sources/MockAnalyticsTracker.swifttemplate/Modules/Analytics/Tests/Resources/.gitkeeptemplate/Modules/Analytics/Tests/Sources/Events/AnalyticsEventsSpec.swifttemplate/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swifttemplate/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swifttemplate/Tuist/Interfaces/SwiftUI/Sources/Application/App.swifttemplate/Tuist/Interfaces/SwiftUI/Sources/Application/Dependencies/Container+Application.swifttemplate/Tuist/Interfaces/SwiftUI/Sources/Presentation/Modules/Landing/LandingViewModel.swifttemplate/Tuist/ProjectDescriptionHelpers/Module.swifttemplate/Tuist/ProjectDescriptionHelpers/Target+Initializing.swifttemplate/{PROJECT_NAME}Tests/Sources/Specs/Presentation/Modules/Landing/LandingViewModelTests.swift
| private var trackers: [AnalyticsTracker] = [] | ||
|
|
||
| private init() {} | ||
|
|
||
| // MARK: - Setup | ||
|
|
||
| public func configure(trackers: [AnalyticsTracker], additionalParameters: [String: Any]? = nil) { | ||
| self.trackers = trackers | ||
| trackers.forEach { $0.setUp(additionalParameters: additionalParameters) } | ||
| } | ||
|
|
||
| public func addTracker(_ tracker: AnalyticsTracker, additionalParameters: [String: Any]? = nil) { | ||
| trackers.append(tracker) | ||
| tracker.setUp(additionalParameters: additionalParameters) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the complete Analytics.swift file
cat -n template/Modules/Analytics/Sources/Analytics.swiftRepository: nimblehq/ios-templates
Length of output: 3548
🏁 Script executed:
# Search for calls to Analytics.shared or Analytics() in the codebase to understand usage patterns
rg "Analytics\.shared|Analytics\(\)" --max-count=50Repository: nimblehq/ios-templates
Length of output: 2279
🏁 Script executed:
# Check if there are any synchronization primitives (DispatchQueue, Lock, etc.) in the Analytics module
rg "DispatchQueue|os_unfair_lock|NSLock|OperationQueue" template/Modules/Analytics/Repository: nimblehq/ios-templates
Length of output: 315
🏁 Script executed:
# Look at the AnalyticsProtocol to understand the interface contract
find template/Modules/Analytics -name "*.swift" -type f | head -20Repository: nimblehq/ios-templates
Length of output: 761
🏁 Script executed:
# Let's check the test file to understand how this is being tested
cat -n template/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swift | head -100Repository: nimblehq/ios-templates
Length of output: 4348
🏁 Script executed:
# Check if App.swift initialization happens on main thread and how analytics is used
cat -n template/Tuist/Interfaces/SwiftUI/Sources/Application/App.swift | head -60Repository: nimblehq/ios-templates
Length of output: 684
🏁 Script executed:
# Look for any dispatcher/queue patterns in the Analytics module or test setup
rg -A 5 -B 5 "configure|addTracker" template/Modules/Analytics/Tests/Repository: nimblehq/ios-templates
Length of output: 14836
Thread safety: trackers is mutated and read without synchronization, and this becomes critical if configure()/addTracker() are called from multiple threads.
Analytics.shared is a process-wide singleton invoked from many threads (main thread for UI, background threads from network/Combine callbacks). The trackers array is mutated by configure()/addTracker() (lines 12, 17) and read by trackEvent / trackScreen / setUserProperty / setUserId / tracker(for:) (lines 28, 47, 58, 67, 74). Swift Array is not thread-safe: concurrent append during .filter/.first read causes a data race and can crash.
In this template's current usage, configure() is called once in App.init() on the main thread, after which only reads occur—so the practical risk is low. However, if addTracker() or configure() are called dynamically from a background thread (e.g., switching analytics providers), the race becomes critical.
Option A (recommended): Guard with a serial dispatch queue:
public final class Analytics: AnalyticsProtocol {
public static let shared: AnalyticsProtocol = Analytics()
private var trackers: [AnalyticsTracker] = []
+ private let queue = DispatchQueue(label: "co.nimblehq.analytics", qos: .utility)
private init() {}
public func configure(trackers: [AnalyticsTracker], additionalParameters: [String: Any]? = nil) {
- self.trackers = trackers
+ queue.sync { self.trackers = trackers }
trackers.forEach { $0.setUp(additionalParameters: additionalParameters) }
}
public func addTracker(_ tracker: AnalyticsTracker, additionalParameters: [String: Any]? = nil) {
- trackers.append(tracker)
+ queue.sync { trackers.append(tracker) }
tracker.setUp(additionalParameters: additionalParameters)
}Then wrap each read: let targetTrackers = queue.sync { trackers.filter { ... } } (lines 28, 47, 58, 67, 74).
Option B: Document main-thread-only usage: Add a // MARK: - Thread Safety comment clarifying that configure() and addTracker() must be called only from the main thread during app initialization to prevent future regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Sources/Analytics.swift` around lines 5 - 19, The
trackers array is accessed without synchronization; protect all mutations and
reads by adding a private serial DispatchQueue (e.g., let trackersQueue =
DispatchQueue(label: "Analytics.trackers")) and use it to serialize access in
configure(trackers:), addTracker(_:,), and all reader methods (trackEvent,
trackScreen, setUserProperty, setUserId, tracker(for:)) — perform writes with
trackersQueue.async or .sync as appropriate and perform reads by capturing the
needed snapshot via trackersQueue.sync { return trackers.filter { ... } } so
callers operate on a stable copy; alternatively, if you choose not to add
synchronization, add a clear "Thread Safety" comment documenting that
configure() and addTracker() must only be called from the main thread during app
initialization.
| public protocol AnalyticsEvent { | ||
|
|
||
| var name: String { get } | ||
| var parameters: [String: Any]? { get } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no AnalyticsParameter type exists in the module.
rg -nP --type=swift -C2 '\bAnalyticsParameter\b'Repository: nimblehq/ios-templates
Length of output: 281
🏁 Script executed:
#!/bin/bash
# Search for AnalyticsParameter in Swift files only, ignoring gitignore parsing
cd template && rg -nP --type=swift '\bAnalyticsParameter\b' 2>/dev/null || echo "No matches found"Repository: nimblehq/ios-templates
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Check the full AnalyticsEvent.swift file
cd template && find . -name "AnalyticsEvent.swift" -type f -exec cat -n {} \;Repository: nimblehq/ios-templates
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Look for any parameter-related enums or protocols in the Analytics module
cd template && find ./Modules/Analytics -name "*.swift" -type f | head -20Repository: nimblehq/ios-templates
Length of output: 684
🏁 Script executed:
#!/bin/bash
# Check if there are any other analytics-related files that might define types
cd template && ls -la Modules/Analytics/Sources/ 2>/dev/null || echo "Directory not found"Repository: nimblehq/ios-templates
Length of output: 745
🏁 Script executed:
#!/bin/bash
# Check AnalyticsProtocol.swift to understand the broader API
cd template && cat -n Modules/Analytics/Sources/AnalyticsProtocol.swiftRepository: nimblehq/ios-templates
Length of output: 2114
🏁 Script executed:
#!/bin/bash
# Check ConsoleAnalyticsTracker to see how parameters are actually handled
cd template && cat -n Modules/Analytics/Sources/ConsoleAnalyticsTracker.swiftRepository: nimblehq/ios-templates
Length of output: 2063
🏁 Script executed:
#!/bin/bash
# Check Analytics.swift for usage context
cd template && cat -n Modules/Analytics/Sources/Analytics.swiftRepository: nimblehq/ios-templates
Length of output: 3548
🏁 Script executed:
#!/bin/bash
# Look for any references to issue `#701` or PR objectives in the codebase
cd template && grep -r "701\|AnalyticsParameter" . --include="*.md" --include="*.swift" 2>/dev/null | head -20Repository: nimblehq/ios-templates
Length of output: 48
AnalyticsEvent uses untyped [String: Any]? parameters without type-safe wrapper.
The protocol currently exposes parameters as [String: Any]?, which:
- Loses type-safety at call sites and in tracker implementations (e.g.,
ConsoleAnalyticsTrackerhandles parameters via.descriptionwith no compile-time guarantees). - Prevents
AnalyticsEventand related types from conforming toSendable, blocking compatibility with Swift's strict concurrency model when events are dispatched from non-@MainActorcontexts.
Consider introducing an AnalyticsParameter enum (e.g., cases for string, int, double, bool, date) and updating AnalyticsEvent.parameters to use [String: AnalyticsParameter]?. This would allow trackers to safely convert to their SDK-native representation in one place while maintaining Sendable conformance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Sources/AnalyticsEvent.swift` around lines 1 - 5,
Change the untyped parameters on AnalyticsEvent to a type-safe wrapper: add an
enum AnalyticsParameter (cases like string, int, double, bool, date) that
conforms to Sendable, update the protocol signature from var parameters:
[String: Any]? to var parameters: [String: AnalyticsParameter]?, and update all
implementations (e.g., ConsoleAnalyticsTracker) to convert AnalyticsParameter to
the tracker/SDK-native representation in one place; ensure AnalyticsParameter
and AnalyticsEvent conform to Sendable where appropriate and update any
callers/tests to construct AnalyticsParameter values instead of raw Any.
| public enum AnalyticsTrackerType: String, CaseIterable { | ||
|
|
||
| case console | ||
| } |
There was a problem hiding this comment.
Enum only defines .console but tests reference .firebase — module won’t compile.
template/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swift uses MockAnalyticsTracker(type: .firebase) in every test. With only .console defined here, the Analytics test target will fail to build.
Decide on one of two fixes and apply it consistently:
- If the intent of this foundation PR is console-only, update the tests to use
.console. - Otherwise, extend the enum to cover the tracker types referenced in tests (and implied by the PR description: Firebase/AppsFlyer/Facebook).
🛠 Suggested extension if you want to keep the `.firebase` test references
public enum AnalyticsTrackerType: String, CaseIterable {
-
- case console
+
+ case console
+ case firebase
+ case appsFlyer
+ case facebook
}📝 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.
| public enum AnalyticsTrackerType: String, CaseIterable { | |
| case console | |
| } | |
| public enum AnalyticsTrackerType: String, CaseIterable { | |
| case console | |
| case firebase | |
| case appsFlyer | |
| case facebook | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Sources/AnalyticsTrackerType.swift` around lines 1
- 4, The enum AnalyticsTrackerType currently only defines case .console but
tests create MockAnalyticsTracker(type: .firebase), causing build failures;
either update all test usages (e.g., MockAnalyticsTracker(type: .firebase)) to
use .console, or add the missing cases to the enum by extending
AnalyticsTrackerType to include .firebase, .appsFlyer, and .facebook (keep the
rawValue strings as "firebase", "appsFlyer", "facebook" to match test/PR
expectations) so MockAnalyticsTracker(type: .firebase) compiles; pick one
approach and apply it consistently across the tests and enum declarations (refer
to AnalyticsTrackerType and MockAnalyticsTracker(type: .firebase) when making
the change).
| /// Mock tracker for testing purposes | ||
| public final class MockAnalyticsTracker: AnalyticsTracker { | ||
|
|
||
| public let type: AnalyticsTrackerType | ||
|
|
||
| // MARK: - Tracked Data | ||
|
|
||
| public private(set) var isSetUp = false | ||
| public private(set) var setupParameters: [String: Any]? | ||
| public private(set) var trackedEvents: [(name: String, parameters: [String: Any]?)] = [] | ||
| public private(set) var trackedScreens: [(name: String, screenClass: String?)] = [] | ||
| public private(set) var userProperties: [String: String] = [:] | ||
| public private(set) var userId: String? | ||
|
|
||
| public init(type: AnalyticsTrackerType) { | ||
| self.type = type | ||
| } | ||
|
|
||
| // MARK: - AnalyticsTracker Implementation | ||
|
|
||
| public func setUp(additionalParameters: [String: Any]?) { | ||
| isSetUp = true | ||
| setupParameters = additionalParameters | ||
| } | ||
|
|
||
| public func trackEvent(name: String, parameters: [String: Any]?) { | ||
| trackedEvents.append((name: name, parameters: parameters)) | ||
| } | ||
|
|
||
| public func trackScreen(name: String, screenClass: String?) { | ||
| trackedScreens.append((name: name, screenClass: screenClass)) | ||
| } | ||
|
|
||
| public func setUserProperty(key: String, value: String) { | ||
| userProperties[key] = value | ||
| } | ||
|
|
||
| public func setUserId(_ userId: String?) { | ||
| self.userId = userId | ||
| } | ||
|
|
||
| // MARK: - Test Helpers | ||
|
|
||
| public func reset() { | ||
| isSetUp = false | ||
| setupParameters = nil | ||
| trackedEvents.removeAll() | ||
| trackedScreens.removeAll() | ||
| userProperties.removeAll() | ||
| userId = nil | ||
| } | ||
|
|
||
| public func hasTrackedEvent(name: String) -> Bool { | ||
| trackedEvents.contains { $0.name == name } | ||
| } | ||
|
|
||
| public func hasTrackedScreen(name: String) -> Bool { | ||
| trackedScreens.contains { $0.name == name } | ||
| } | ||
|
|
||
| public func eventCount(for name: String) -> Int { | ||
| trackedEvents.filter { $0.name == name }.count | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
MockAnalyticsTracker lives in production Sources and ships with the module.
This file is under template/Modules/Analytics/Sources/, which means it becomes part of the Analytics module that App / Domain link against in release builds. A mock intended for tests shouldn't be a publicly exported API of the production framework — besides the binary size cost, it invites accidental use from real code.
Recommended options:
- Move this type into a test-only target (e.g. the Analytics tests target, or a dedicated
AnalyticsTestSupporttarget). - At minimum, wrap the entire file in
#if DEBUGso release builds don't ship it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Sources/MockAnalyticsTracker.swift` around lines 1
- 64, The MockAnalyticsTracker class is shipping in the production Analytics
module; either move the MockAnalyticsTracker type (public final class
MockAnalyticsTracker) out of template/Modules/Analytics/Sources into a test-only
target (e.g., AnalyticsTests or a new AnalyticsTestSupport target) so it is not
compiled into release builds, or wrap the entire file with a compile-time guard
(`#if` DEBUG ... `#endif`) to exclude it from release; after moving/wrapping ensure
the symbol MockAnalyticsTracker is no longer exported from the Analytics module
and update any test imports to reference the new test target or the conditional
compilation path.
| @Test("Analytics should be a singleton") | ||
| func analyticsSingleton() { | ||
| let instance1 = Analytics.shared | ||
| let instance2 = Analytics.shared | ||
|
|
||
| #expect(instance1 === instance2) | ||
| } | ||
|
|
||
| @Test("Analytics should conform to AnalyticsProtocol") | ||
| func analyticsProtocolConformance() { | ||
| let analytics = Analytics.shared | ||
| #expect(analytics is AnalyticsProtocol) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Swift === operator with non-class-bound protocol
💡 Result:
The Swift === (identity) operator cannot be used directly with non-class-bound protocols. It is defined only for AnyObject? operands, and AnyObject is the protocol that all classes implicitly conform to. Protocols without a class constraint (inheriting from AnyObject or deprecated class) can be adopted by value types like structs or enums, which lack reference identity. Attempting p1 === p2 where p1: Protocol and p2: Protocol results in a compiler error: "Binary operator '===' cannot be applied to two 'Protocol' operands". To enable === on protocol-typed values, declare the protocol as class-only: protocol MyProtocol: AnyObject { ... } Only classes can then conform, ensuring reference semantics. Example: class MyClass: MyProtocol { ... } let p1: MyProtocol = MyClass let p2: MyClass print(p1 === p2) // false (different instances) let p3 = p1 print(p1 === p3) // true (same instance) This works because the compiler knows all MyProtocol instances are class references compatible with AnyObject. Note: Use AnyObject (not deprecated class) for modern Swift. For value equality, conform to Equatable instead.
Citations:
- 1: https://stackoverflow.com/questions/34569180/swift-binary-operator-cannot-be-applied-to-two-protocols
- 2: https://stackoverflow.com/questions/32038750/can-we-test-if-objects-conforming-to-the-same-protocol-are-identical-in-swift-wi
- 3: https://stackoverflow.com/questions/32490973/object-identity-and-protocols
- 4: https://developer.apple.com/documentation/swift/anyobject
- 5: https://stackoverflow.com/questions/30176814/whats-the-difference-between-a-protocol-extended-from-anyobject-and-a-class-onl
- 6: https://www.hackingwithswift.com/example-code/language/how-to-restrict-a-protocol-to-classes
🏁 Script executed:
# Find and examine AnalyticsSpec.swift
find . -type f -name "AnalyticsSpec.swift" | head -5Repository: nimblehq/ios-templates
Length of output: 135
🏁 Script executed:
# Find AnalyticsProtocol definition
find . -type f -name "*.swift" | xargs grep -l "protocol AnalyticsProtocol" | head -5Repository: nimblehq/ios-templates
Length of output: 127
🏁 Script executed:
cat -n ./template/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swift | head -40Repository: nimblehq/ios-templates
Length of output: 1641
🏁 Script executed:
cat -n ./template/Modules/Analytics/Sources/AnalyticsProtocol.swiftRepository: nimblehq/ios-templates
Length of output: 2114
🏁 Script executed:
find . -type f -name "*.swift" -path "*/Analytics/Sources/*" | grep -i analytics | grep -v Protocol | grep -v Tracker | head -10Repository: nimblehq/ios-templates
Length of output: 242
🏁 Script executed:
cat -n ./template/Modules/Analytics/Sources/Analytics.swift | head -60Repository: nimblehq/ios-templates
Length of output: 2919
Line 14 won't compile: === requires class-bound protocol; line 20 is a tautological check.
Two issues in the singleton section:
- Line 14:
instance1 === instance2uses the identity operator on twoAnalyticsProtocolvalues, butAnalyticsProtocolis not class-bound (declared as a plainpublic protocol, not constrained toAnyObject). SinceAnalytics.sharedis explicitly typed asAnalyticsProtocolon line 3 ofAnalytics.swift, the inferred type of both variables is the protocol, not the concrete class. The===operator only applies to class types, so this fails to compile with error: "Binary operator '===' cannot be applied to two 'AnalyticsProtocol' operands". - Line 20:
analytics is AnalyticsProtocolis alwaystruebecauseanalyticsis already statically typed asAnalyticsProtocol. The compiler will warn that this condition is always true.
Suggested fixes
Option A: Declare the protocol as class-only in AnalyticsProtocol.swift:
public protocol AnalyticsProtocol: AnyObject {Option B: Cast to AnyObject in the test:
`@Test`("Analytics should be a singleton")
func analyticsSingleton() {
- let instance1 = Analytics.shared
- let instance2 = Analytics.shared
+ let instance1 = Analytics.shared as AnyObject
+ let instance2 = Analytics.shared as AnyObject
`#expect`(instance1 === instance2)
}
-
- `@Test`("Analytics should conform to AnalyticsProtocol")
- func analyticsProtocolConformance() {
- let analytics = Analytics.shared
- `#expect`(analytics is AnalyticsProtocol)
- }📝 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.
| @Test("Analytics should be a singleton") | |
| func analyticsSingleton() { | |
| let instance1 = Analytics.shared | |
| let instance2 = Analytics.shared | |
| #expect(instance1 === instance2) | |
| } | |
| @Test("Analytics should conform to AnalyticsProtocol") | |
| func analyticsProtocolConformance() { | |
| let analytics = Analytics.shared | |
| #expect(analytics is AnalyticsProtocol) | |
| } | |
| `@Test`("Analytics should be a singleton") | |
| func analyticsSingleton() { | |
| let instance1 = Analytics.shared as AnyObject | |
| let instance2 = Analytics.shared as AnyObject | |
| `#expect`(instance1 === instance2) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swift` around
lines 9 - 21, The tests fail because Analytics.shared is typed as
AnalyticsProtocol so identity (===) and the `is` check are wrong; update
AnalyticsSpec.swift: in analyticsSingleton(), cast the shared instances to
AnyObject (e.g. let instance1 = Analytics.shared as AnyObject and same for
instance2) before using === to test singleton identity; in
analyticsProtocolConformance(), avoid the tautology by checking against the
concrete type (e.g. expect(Analytics.shared is Analytics)) or remove the
redundant test — alternatively, if you prefer making the protocol class-only,
change AnalyticsProtocol to `public protocol AnalyticsProtocol: AnyObject` in
AnalyticsProtocol.swift so the original code compiles.
| @Test("Track events without parameters") | ||
| func trackEventWithoutParameters() { | ||
| let sut = Analytics() | ||
| let mockConsoleTracker = MockAnalyticsTracker(type: .console) | ||
| let mockAppsFlyerTracker = MockAnalyticsTracker(type: .appsFlyer) | ||
|
|
||
| sut.configure(trackers: [mockConsoleTracker]) | ||
| sut.trackEvent(name: "app_open") | ||
|
|
||
| #expect(mockFirebaseTracker.hasTrackedEvent(name: "app_open")) | ||
| #expect(mockAppsFlyerTracker.hasTrackedEvent(name: "app_open")) | ||
| } |
There was a problem hiding this comment.
Compile error: mockFirebaseTracker is not declared.
Line 85 references mockFirebaseTracker, which is never defined in this test (only mockConsoleTracker and mockAppsFlyerTracker are). In addition, mockAppsFlyerTracker is declared but never registered with the SUT (sut.configure(trackers: [mockConsoleTracker])), so the AppsFlyer assertion on line 86 would also fail even after fixing the symbol error.
🔧 Suggested fix
`@Test`("Track events without parameters")
func trackEventWithoutParameters() {
let sut = Analytics()
let mockConsoleTracker = MockAnalyticsTracker(type: .console)
- let mockAppsFlyerTracker = MockAnalyticsTracker(type: .appsFlyer)
-
- sut.configure(trackers: [mockConsoleTracker])
+
+ sut.configure(trackers: [mockConsoleTracker])
sut.trackEvent(name: "app_open")
-
- `#expect`(mockFirebaseTracker.hasTrackedEvent(name: "app_open"))
- `#expect`(mockAppsFlyerTracker.hasTrackedEvent(name: "app_open"))
+
+ `#expect`(mockConsoleTracker.hasTrackedEvent(name: "app_open"))
+ `#expect`(mockConsoleTracker.trackedEvents.first?.parameters == nil)
}Also note that .appsFlyer isn't defined in AnalyticsTrackerType at the moment (only .console), so any AppsFlyer/Firebase tracker types will need to be added to the enum before they can be used in tests.
📝 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.
| @Test("Track events without parameters") | |
| func trackEventWithoutParameters() { | |
| let sut = Analytics() | |
| let mockConsoleTracker = MockAnalyticsTracker(type: .console) | |
| let mockAppsFlyerTracker = MockAnalyticsTracker(type: .appsFlyer) | |
| sut.configure(trackers: [mockConsoleTracker]) | |
| sut.trackEvent(name: "app_open") | |
| #expect(mockFirebaseTracker.hasTrackedEvent(name: "app_open")) | |
| #expect(mockAppsFlyerTracker.hasTrackedEvent(name: "app_open")) | |
| } | |
| `@Test`("Track events without parameters") | |
| func trackEventWithoutParameters() { | |
| let sut = Analytics() | |
| let mockConsoleTracker = MockAnalyticsTracker(type: .console) | |
| sut.configure(trackers: [mockConsoleTracker]) | |
| sut.trackEvent(name: "app_open") | |
| `#expect`(mockConsoleTracker.hasTrackedEvent(name: "app_open")) | |
| `#expect`(mockConsoleTracker.trackedEvents.first?.parameters == nil) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swift` around
lines 76 - 87, The test trackEventWithoutParameters references and asserts on
trackers that are not declared or registered: declare mockFirebaseTracker (e.g.,
let mockFirebaseTracker = MockAnalyticsTracker(type: .firebase)), register both
mockAppsFlyerTracker and mockFirebaseTracker with the SUT when calling
sut.configure(trackers: [...]), and update the assertions to expect those
declared mocks (use mockFirebaseTracker and mockAppsFlyerTracker). Also add
.appsFlyer and .firebase cases to the AnalyticsTrackerType enum so
MockAnalyticsTracker(type: .appsFlyer) and .firebase compile; ensure
MockAnalyticsTracker's initializer supports the new enum values.
| @Test("Track screens on specific trackers") | ||
| func trackScreenOnSpecificTrackers() { | ||
| let sut = Analytics() | ||
| let mockConsoleTracker = MockAnalyticsTracker(type: .console) | ||
| let mockAppsFlyerTracker = MockAnalyticsTracker(type: .appsFlyer) | ||
|
|
||
| sut.configure(trackers: [mockConsoleTracker]) | ||
| sut.trackScreen(name: "ProfileScreen", screenClass: nil, on: [.appsFlyer]) | ||
|
|
||
| #expect(!mockConsoleTracker.hasTrackedScreen(name: "ProfileScreen")) | ||
| } |
There was a problem hiding this comment.
Test doesn't verify what its name claims.
mockAppsFlyerTracker is declared but never registered with the SUT (only mockConsoleTracker is), so the on: [.appsFlyer] filter matches zero configured trackers. The assertion !mockConsoleTracker.hasTrackedScreen(...) is therefore trivially true regardless of filtering behavior and doesn't actually prove that "on specific trackers" works.
Also note .appsFlyer isn't a declared case in AnalyticsTrackerType (currently only .console), so this test won't compile until that enum is extended.
🔧 Suggested fix
`@Test`("Track screens on specific trackers")
func trackScreenOnSpecificTrackers() {
let sut = Analytics()
let mockConsoleTracker = MockAnalyticsTracker(type: .console)
let mockAppsFlyerTracker = MockAnalyticsTracker(type: .appsFlyer)
-
- sut.configure(trackers: [mockConsoleTracker])
+
+ sut.configure(trackers: [mockConsoleTracker, mockAppsFlyerTracker])
sut.trackScreen(name: "ProfileScreen", screenClass: nil, on: [.appsFlyer])
-
- `#expect`(!mockConsoleTracker.hasTrackedScreen(name: "ProfileScreen"))
+
+ `#expect`(mockAppsFlyerTracker.hasTrackedScreen(name: "ProfileScreen"))
+ `#expect`(!mockConsoleTracker.hasTrackedScreen(name: "ProfileScreen"))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/Modules/Analytics/Tests/Sources/Specs/AnalyticsSpec.swift` around
lines 118 - 128, The test declares mockAppsFlyerTracker but never registers it
and uses a tracker type (.appsFlyer) that doesn't exist, so the test is vacuous
and may not compile; fix by adding an .appsFlyer case to AnalyticsTrackerType
(or the enum used), register both trackers with the SUT via
Analytics.configure(trackers: [mockConsoleTracker, mockAppsFlyerTracker]), call
sut.trackScreen(name: "ProfileScreen", screenClass: nil, on: [.appsFlyer]) and
update the assertions to assert
mockAppsFlyerTracker.hasTrackedScreen("ProfileScreen") is true and
mockConsoleTracker.hasTrackedScreen("ProfileScreen") is false so the test
actually validates filtering behavior for Analytics.trackScreen.
|
|
||
| @Test("MockAnalyticsTracker initializes with correct type") | ||
| func initializeWithCorrectType() { | ||
| let sut = MockAnalyticsTracker(type: .firebase) |
There was a problem hiding this comment.
.firebase is not a defined AnalyticsTrackerType case.
Every test in this file (lines 11, 21, 32, 44, 58, 68, 83, 96, 108, 119, 128, 140) passes type: .firebase, but template/Modules/Analytics/Sources/AnalyticsTrackerType.swift currently defines only .console. The Analytics test target won’t compile until the enum is extended or these usages are changed to .console. Root-cause fix is discussed on AnalyticsTrackerType.swift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@template/Modules/Analytics/Tests/Sources/Specs/MockAnalyticsTrackerSpec.swift`
at line 11, Tests instantiate MockAnalyticsTracker(type: .firebase) but
AnalyticsTrackerType only defines .console, causing build failures; update the
enum AnalyticsTrackerType (in AnalyticsTrackerType.swift) to include a .firebase
case (and any associated logic or switch exhaustiveness updates) so
MockAnalyticsTracker(type: .firebase) compiles, or alternatively change all test
usages of MockAnalyticsTracker to use .console if you prefer not to add a
firebase case—locate references to MockAnalyticsTracker and the enum name
AnalyticsTrackerType to implement the chosen fix.
1 participant
What happened 👀
Analyticsmodule:AnalyticsProtocol: contract for tracking events, screens, user properties, and setup.Analytics: singleton implementation; routes events to registered trackers byAnalyticsTrackerType.AnalyticsTracker: per-provider adapter (e.g. Firebase, AppsFlyer, Facebook).AnalyticsEvent: protocol for structured events; example:UserLoginEvent.ConsoleAnalyticsTracker: mock debug implementation that logs to the console.LandingViewModel: tracks login only viaUserLoginEvent.init()callsAnalytics.shared.configure(...)Insight 📝
N/A
Proof Of Work 📹
Kapture.2026-04-24.at.14.31.23.mp4
Summary by CodeRabbit
New Features
Tests