From 7bfd4b5a6c38f14715a6b38a8d759a134537b57b Mon Sep 17 00:00:00 2001
From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com>
Date: Fri, 9 Jan 2026 09:53:38 -0500
Subject: [PATCH] Document best practices for Clock/Time handling (#6340)

---
 .../reference/architectural-patterns.md       | 39 ++++++++
 docs/STYLE_AND_BEST_PRACTICES.md              | 94 +++++++++++++++++++
 2 files changed, 133 insertions(+)

diff --git a/.claude/skills/reviewing-changes/reference/architectural-patterns.md b/.claude/skills/reviewing-changes/reference/architectural-patterns.md
index 6244bae869..08b3e6e69f 100644
--- a/.claude/skills/reviewing-changes/reference/architectural-patterns.md
+++ b/.claude/skills/reviewing-changes/reference/architectural-patterns.md
@@ -11,6 +11,7 @@ Quick reference for Bitwarden Android architectural patterns during code reviews
 - [Hilt Dependency Injection](#hilt-dependency-injection)
   - [ViewModels](#viewmodels)
   - [Repositories and Managers](#repositories-and-managers)
+  - [Clock/Time Handling](#clocktime-handling)
 - [Module Organization](#module-organization)
 - [Error Handling](#error-handling)
   - [Use Result Types, Not Exceptions](#use-result-types-not-exceptions)
@@ -210,6 +211,43 @@ abstract class DataModule {
 
 ---
 
+### Clock/Time Handling
+
+Time-dependent code must use injected `Clock` rather than direct `Instant.now()` or `DateTime.now()` calls. This follows the same DI principle as other dependencies.
+
+**✅ GOOD - Injected Clock**:
+```kotlin
+// ViewModel with Clock injection
+class MyViewModel @Inject constructor(
+    private val clock: Clock,
+) {
+    fun save() {
+        val timestamp = clock.instant()
+    }
+}
+
+// Extension function with Clock parameter
+fun State.getTimestamp(clock: Clock): Instant =
+    existingTime ?: clock.instant()
+```
+
+**❌ BAD - Static/direct calls**:
+```kotlin
+// Hidden dependency, non-testable
+val timestamp = Instant.now()
+val dateTime = DateTime.now()
+```
+
+**Key Rules**:
+- Inject `Clock` via Hilt constructor (like other dependencies)
+- Pass `Clock` as parameter to extension functions
+- `Clock` is provided via `CoreModule` as singleton
+- Enables deterministic testing with `Clock.fixed(...)`
+
+Reference: `docs/STYLE_AND_BEST_PRACTICES.md#best-practices--time-and-clock-handling`
+
+---
+
 ## Module Organization
 
 ```
@@ -299,6 +337,7 @@ Reference: `docs/ARCHITECTURE.md#error-handling`
 - [ ] Business logic in Repository, not ViewModel?
 - [ ] Using Hilt DI (@HiltViewModel, @Inject constructor)?
 - [ ] Injecting interfaces, not implementations?
+- [ ] Time-dependent code uses injected `Clock` (not `Instant.now()`)?
 - [ ] Correct module placement?
 
 ### Error Handling
diff --git a/docs/STYLE_AND_BEST_PRACTICES.md b/docs/STYLE_AND_BEST_PRACTICES.md
index 963a7edfda..3c3626284c 100644
--- a/docs/STYLE_AND_BEST_PRACTICES.md
+++ b/docs/STYLE_AND_BEST_PRACTICES.md
@@ -773,3 +773,97 @@ Special consideration should be taken to avoid unnecessary recompositions. There
 - https://developer.android.com/jetpack/compose/performance/bestpractices
 - https://getstream.io/blog/jetpack-compose-guidelines/
 - https://multithreaded.stitchfix.com/blog/2022/08/05/jetpack-compose-recomposition/
+
+## Best Practices : Time and Clock Handling
+
+To ensure testability and deterministic behavior, all code that needs the current time should use an injected `Clock` rather than calling `Instant.now()` or `DateTime.now()` directly.
+
+### Why
+
+- Direct calls to `Instant.now()` or `DateTime.now()` create non-deterministic behavior
+- Testing requires brittle `mockkStatic` that can interfere across tests
+- Injected `Clock` enables deterministic testing with `Clock.fixed(...)`
+- Follows the dependency injection principle (no hidden dependencies)
+
+### Pattern
+
+**In ViewModels and classes with DI:**
+
+```kotlin
+// Good: Clock injected via Hilt
+class MyViewModel @Inject constructor(
+    private val clock: Clock,
+    // other dependencies...
+) : BaseViewModel<...>(...) {
+
+    private fun handleSaveClick() {
+        val item = Item(
+            createdAt = clock.instant(),
+            // ...
+        )
+    }
+}
+
+// Bad: Direct call creates hidden dependency
+class MyViewModel @Inject constructor(
+    // missing Clock...
+) : BaseViewModel<...>(...) {
+
+    private fun handleSaveClick() {
+        val item = Item(
+            createdAt = Instant.now(), // ❌ Non-testable
+            // ...
+        )
+    }
+}
+```
+
+**In extension functions and utilities:**
+
+```kotlin
+// Good: Accept Clock as parameter
+fun SomeState.getRevisionDate(
+    originalItem: Item?,
+    clock: Clock,
+): Instant = originalItem?.revisionDate ?: clock.instant()
+
+// Bad: Hidden dependency on system clock
+fun SomeState.getRevisionDate(
+    originalItem: Item?,
+): Instant = originalItem?.revisionDate ?: Instant.now() // ❌
+```
+
+### Testing
+
+```kotlin
+// Good: Fixed clock for deterministic tests
+private val FIXED_CLOCK: Clock = Clock.fixed(
+    Instant.parse("2023-10-27T12:00:00Z"),
+    ZoneOffset.UTC,
+)
+
+@Test
+fun `test time-dependent logic`() = runTest {
+    val viewModel = MyViewModel(
+        clock = FIXED_CLOCK,
+        // ...
+    )
+    // Test with predictable time
+}
+
+// Bad: Static mocking is fragile
+mockkStatic(Instant::class)  // ❌ Avoid
+every { Instant.now() } returns fixedInstant
+```
+
+### Clock Provider
+
+The `Clock` is provided via Hilt in `CoreModule`:
+
+```kotlin
+@Provides
+@Singleton
+fun provideClock(): Clock = Clock.systemDefaultZone()
+```
+
+Reference: `core/src/main/kotlin/com/bitwarden/core/di/CoreModule.kt`