Skip to content

refactor: clean up force-unwraps and enable lint ratchet#29

Merged
Ivan-LB merged 5 commits into
devfrom
refactor/cleanup-force-unwraps
Apr 26, 2026
Merged

refactor: clean up force-unwraps and enable lint ratchet#29
Ivan-LB merged 5 commits into
devfrom
refactor/cleanup-force-unwraps

Conversation

@Ivan-LB
Copy link
Copy Markdown
Owner

@Ivan-LB Ivan-LB commented Apr 26, 2026

Summary

Closes Task 3 of docs/plans/maintenance-cleanup.md. Five commits:

  1. e640ab9 — services layer (CameraManager, OpenAIClient, SignInWithAppleHelper)
  2. a53346c — fix broken pre-commit hook (discovered during Task 3 — see below)
  3. 7935d15 — data models (IncomeModel, ExpenseModel)
  4. b13e472 — viewmodels + one view (ProfileViewModel, ReportsViewModel, MainNavigationView)
  5. aa1ae55 — flip .swiftlint.yml: force_unwrapping severity warningerror

After this PR, any new value! from this commit forward fails CI and the local pre-commit hook.

Why

swiftlint lint was surfacing 10 force_unwrapping warnings on every CI run. They were warnings (not errors) so CI passed, but every new force-unwrap added to the pile silently. The "ratchet" pattern says: clean the existing violations, then bump severity to error so any future violation is caught at the door.

After this PR:

  • swiftlint lint --strict reports 0 violations across all 75 linted files.
  • Local pre-commit hook now actually runs SwiftLint on staged Swift files (was structurally broken — see commit 2).
  • CI's existing swiftlint lint --reporter github-actions-logging step (no --strict) keeps annotating, but the bumped severity means any new force-unwrap shows as a CI error and blocks merge.

What each fix looks like

Services (commit 1)

  • CameraManager.swift:93previewLayer! after just-initialized → use a local let layer = … and assign to the stored property at the end. Removes both the ! and the noisy previewLayer?. chain.
  • OpenAIClient.swift:26URL(string: literal)!guard let url = … else { return nil }. The function already returns String? and nil is the established failure path.
  • SignInWithAppleHelper.swift:236self.view.window! in presentationAnchor(for:)self.view.window ?? ASPresentationAnchor(). The protocol can be invoked before a window is attached; a fallback anchor is the documented escape hatch.

Data models (commit 3) and viewmodels (commit 4)

Six sites all had the same shape: Calendar.current.date(byAdding: .day, value: 1, to: endDate)!. All replaced with guard let nextDay = … else { return [] } (functions return arrays of IncomeModel / ExpenseModel, so empty array is the safe degenerate result that matches existing error paths). No shared helper extracted — duplication was 4 short lines per file, not worth a new file + pbxproj edit.

View (commit 4)

  • MainNavigationView.swift:659 — Force-unwrap of goalName: String? inside a Text(...) ternary → goalName.map { "Add $\(Int(selectedPreset)) to \($0)" } ?? "Select a goal". No new hardcoded strings introduced (both literals were already there). The nil branch is unreachable in practice (button is .disabled(selectedGoal == nil)), but the safe form documents the contract anyway.

Out-of-scope but folded in: pre-commit hook fix (commit 2)

The .githooks/pre-commit script called swiftlint lint --use-script-input-files <<< "$file", but --use-script-input-files reads from Xcode build-phase env vars (SCRIPT_INPUT_FILE_*), not stdin. Every CLI git commit was failing with SCRIPT_INPUT_FILE_COUNT variable not set since PR #22 — local lint enforcement had never actually run. Fixed by passing the file as a positional argument:

-    swiftlint lint --quiet --strict --use-script-input-files <<< "$file" || {
+    swiftlint lint --quiet --strict "$file" || {

Verified by staging a Swift file with a known force_unwrapping warning — the hook caught it. Folded into this PR (rather than a separate one) because the entire point of Task 3 is "make lint enforcement actually work end-to-end" and the hook was the broken local half of that.

Test plan

  • swiftlint lint --strict — 0 violations across 75 files
  • xcodebuild build -scheme Savely -destination 'platform=iOS Simulator,name=iPhone 17 Pro' — passes
  • xcodebuild test -scheme Savely -destination 'platform=iOS Simulator,name=iPhone 17 Pro' -skip-testing:SavelyUITests — passes
  • Pre-commit hook verified working (caught known force_unwrapping on staged file)
  • No new hardcoded user-facing strings
  • No secrets staged

Risks

  • The 6 sites that returned [] on calendar-failure now degrade silently instead of crashing. If you ever want to surface this as a UI error, those would be the spots to add an error state — but a calendar arithmetic failure on +1 day is essentially impossible in practice, so silent degradation matches the prior implicit behavior more honestly than a crash did.
  • MainNavigationView.swift:659 change uses Optional.map(...) ?? default instead of an if let/else view-builder split. Functionally identical but slightly less SwiftUI-idiomatic — flag if you'd rather see the explicit form.
  • After this merges, any new force-unwrap from any future PR will fail SwiftLint locally (pre-commit hook) and on CI. Intentional. If you ever need a true invariant unwrap, use guard let x = value else { fatalError("invariant violated: …") } — explicit failure mode, satisfies the linter.

🤖 Generated with Claude Code

Ivan-LB added 5 commits April 25, 2026 14:21
…forms

Cleans up 3 force_unwrapping violations in:
- Managers/CameraManager.swift:93
- Utilities/OpenAIClient.swift:26
- Utilities/SignInWithAppleHelper.swift:236

Each site uses the smallest fix that matches its semantics — guard let
with early return, if let, ?? default, or guard let + fatalError when an
invariant guarantees non-nil. No swiftlint:disable directives added.

Part 1 of 4 in docs/plans/maintenance-cleanup.md Task 3 cleanup.
The --use-script-input-files flag reads from Xcode build-phase env
vars (SCRIPT_INPUT_FILE_*), not stdin. The heredoc <<< was a no-op
and every CLI git commit failed with "SCRIPT_INPUT_FILE_COUNT variable
not set". Pass each staged file as a positional argument instead.

Discovered while running the Task 3 force-unwrap cleanup.
Cleans up 2 force_unwrapping violations in:
- Models/IncomeModel.swift:32
- Models/ExpenseModel.swift:32

Applied the fix inline in each file rather than introducing a shared
helper, since deduplicating a single line across two files would have
required adding a third file and a project.pbxproj edit.

Part 2 of 4 in docs/plans/maintenance-cleanup.md Task 3 cleanup.
Cleans up 5 force_unwrapping violations in:
- ViewModels/ProfileTab/ProfileViewModel.swift:125, 142
- ViewModels/Dashboard/ReportsViewModel.swift:77, 92
- Views/MainNavigationView.swift:659

Each site uses the smallest fix that matches its semantics — guard let
with early return, if let, ?? default. No swiftlint:disable directives.

Part 3 of 4 in docs/plans/maintenance-cleanup.md Task 3 cleanup.
The 10 force_unwrapping sites that previously triggered warnings have
all been refactored in this PR to use safe forms (guard let, if let,
?? default, or fatalError for invariant-guaranteed values). Bumping
severity to error means any new force-unwrap from this commit forward
fails CI and the local pre-commit hook.

Part 4 of 4 in docs/plans/maintenance-cleanup.md Task 3 cleanup.
@Ivan-LB Ivan-LB merged commit bf93d1e into dev Apr 26, 2026
1 check passed
@Ivan-LB Ivan-LB deleted the refactor/cleanup-force-unwraps branch April 26, 2026 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant