From 9daa498640c55817ed0923824f695567f53adb85 Mon Sep 17 00:00:00 2001 From: Takeshi Shimada Date: Thu, 30 Oct 2025 23:54:21 +0900 Subject: [PATCH 1/2] breaking: Throw error on empty concatenate, remove force cast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make ActionTask.concatenate() throw FlowError.noTasksToExecute when given an empty array, implementing fail-fast principle for better error detection and debugging. Changes: - Add FlowError.noTasksToExecute case with comprehensive documentation - Make ActionTask.concatenate(_:) throwing for array version - Remove force cast from Store.swift executeTask() method - Add precondition with clear invariant documentation - Update all LocalizedError implementations Benefits: - Fail fast: Empty arrays detected immediately - Better debugging: Clear error messages with recovery suggestions - Type safety: Works for all ActionResult types (not just Void) - No force cast: Eliminates runtime type checking - Explicit intent: Developers must handle empty case explicitly Breaking Changes: - ActionTask.concatenate([ActionTask]) now throws - Callers must either: 1. Use try and handle the error 2. Guard against empty arrays before calling 3. Variadic version remains non-throwing (compile-time safe) Migration: ```swift // Before return .concatenate(tasks) // After Option 1: Explicit guard guard !tasks.isEmpty else { return .none // Empty is valid } return try .concatenate(tasks) // After Option 2: Propagate error return try .concatenate(tasks) ``` All tests pass (299 tests). No existing code used empty concatenate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- Sources/Flow/Store/ActionTask.swift | 38 +++++++++++++++--------- Sources/Flow/Store/FlowError.swift | 45 +++++++++++++++++++++++++++++ Sources/Flow/Store/Store.swift | 34 ++++++++++------------ 3 files changed, 86 insertions(+), 31 deletions(-) diff --git a/Sources/Flow/Store/ActionTask.swift b/Sources/Flow/Store/ActionTask.swift index e23bded..4575d81 100644 --- a/Sources/Flow/Store/ActionTask.swift +++ b/Sources/Flow/Store/ActionTask.swift @@ -474,30 +474,42 @@ extension ActionTask { /// - Parameter tasks: Variadic list of tasks to concatenate /// - Returns: A single task that runs all tasks sequentially and returns the last meaningful result public static func concatenate(_ tasks: ActionTask...) -> ActionTask { - concatenate(tasks) + // Variadic guarantees at least one element at call site + // Safe to force try because tasks cannot be empty + try! concatenate(tasks) } /// Concatenates an array of tasks to run sequentially. /// - /// This is the array version of the variadic `concatenate` method. - /// Useful when you have a dynamic number of tasks. + /// Throws `FlowError.noTasksToExecute` if the task array is empty. + /// This strict behavior helps catch logic errors early. /// - /// ## Example + /// ## Example: Dynamic Tasks with Guard /// ```swift - /// // Process items one by one - /// let processTasks = items.map { item in - /// ActionTask.run { state in - /// state.processed.append(try await process(item)) - /// } + /// let tasks = items.map { item in + /// ActionTask.run { state in try await process(item) } + /// } + /// + /// // Explicit handling of empty case + /// guard !tasks.isEmpty else { + /// return .none // Empty is intentional /// } - /// return .concatenate(processTasks) + /// + /// return try .concatenate(tasks) + /// ``` + /// + /// ## Example: Propagating Error + /// ```swift + /// // Let the error propagate if empty is unexpected + /// return try .concatenate(tasks) // May throw /// ``` /// - /// - Parameter tasks: Array of tasks to concatenate + /// - Parameter tasks: Array of tasks to concatenate (must not be empty) /// - Returns: A single task that runs all tasks sequentially - public static func concatenate(_ tasks: [ActionTask]) -> ActionTask { + /// - Throws: `FlowError.noTasksToExecute` if tasks array is empty + public static func concatenate(_ tasks: [ActionTask]) throws -> ActionTask { guard let first = tasks.first else { - fatalError("concatenate requires at least one task") + throw FlowError.noTasksToExecute(context: "concatenate(_:)") } // TCA-style reduce pattern implementing Monoid return tasks.dropFirst().reduce(first) { $0.concatenate(with: $1) } diff --git a/Sources/Flow/Store/FlowError.swift b/Sources/Flow/Store/FlowError.swift index 901006d..9db6778 100644 --- a/Sources/Flow/Store/FlowError.swift +++ b/Sources/Flow/Store/FlowError.swift @@ -133,6 +133,29 @@ public enum FlowError: Error { /// - message: The error message /// - underlying: Optional underlying error case custom(message: String, underlying: Error? = nil) + + /// No tasks provided to concatenate. + /// + /// Thrown when attempting to concatenate an empty task array. + /// This typically indicates a logic error where tasks were expected but none were generated. + /// + /// ## Recovery + /// If empty task lists are valid for your use case, explicitly check before concatenating: + /// ```swift + /// guard !tasks.isEmpty else { + /// return .none // Explicitly handle empty case + /// } + /// return try .concatenate(tasks) + /// ``` + /// + /// ## Debugging + /// Check why the task array is empty: + /// - Is the data source empty? + /// - Is there a filter that's too restrictive? + /// - Is there a mapping error? + /// + /// - Parameter context: Additional context about where the error occurred + case noTasksToExecute(context: String? = nil) } // MARK: - LocalizedError Conformance @@ -164,6 +187,12 @@ extension FlowError: LocalizedError { return "\(message): \(underlying.localizedDescription)" } return message + + case .noTasksToExecute(let context): + if let context = context { + return "No tasks to execute in \(context). Empty task arrays are not allowed. If this is intentional, explicitly return .none instead." + } + return "No tasks to execute. Empty task arrays are not allowed. If empty is valid, explicitly check and return .none." } } @@ -187,6 +216,16 @@ extension FlowError: LocalizedError { case .custom: return nil + + case .noTasksToExecute: + return """ + Check if empty is expected: + + guard !tasks.isEmpty else { + return .none // Explicit: empty is OK + } + return try .concatenate(tasks) + """ } } @@ -210,6 +249,12 @@ extension FlowError: LocalizedError { case .custom(_, let underlying): return underlying?.localizedDescription + + case .noTasksToExecute(let context): + if let context = context { + return "Empty task array in \(context)" + } + return "Empty task array provided to concatenate" } } } diff --git a/Sources/Flow/Store/Store.swift b/Sources/Flow/Store/Store.swift index 31bdc62..ecb52d4 100644 --- a/Sources/Flow/Store/Store.swift +++ b/Sources/Flow/Store/Store.swift @@ -330,32 +330,30 @@ public final class Store { case .concatenated: // Flatten concatenate tree for sequential iteration (O(n) → O(1) depth) let tasks = task.flattenConcatenated() - var lastResult: F.ActionResult? - for task in tasks { + // INVARIANT: flattenConcatenated() always returns ≥1 element + // + // Proof: + // 1. concatenate([]) now throws FlowError.noTasksToExecute + // 2. .concatenated can only be constructed via concatenate() + // 3. Therefore, .concatenated always contains ≥1 task + // 4. flattenConcatenated() preserves this property + // + // If this precondition fails, there's a bug in ActionTask construction + precondition(!tasks.isEmpty, "Implementation error: concatenated task list is empty. This should be impossible due to concatenate() throwing on empty arrays.") + + var lastResult: F.ActionResult = try await self.executeTask(tasks[0]) + + for task in tasks.dropFirst() { // Check for cancellation between sequential tasks guard !Task.isCancelled else { throw StoreError.cancelled } - let result = try await self.executeTask(task) - - // Update the last result - lastResult = result + lastResult = try await self.executeTask(task) } - // Return the last result, or throw if none found (all were .just) - guard let result = lastResult else { - // All tasks were .just with Void - return Void - if F.ActionResult.self is Void.Type { - // Safe to return () as F.ActionResult when ActionResult == Void - // swiftlint:disable:next force_cast - return (() as! F.ActionResult) - } - throw StoreError.cancelled - } - - return result + return lastResult } } } From 67efea47bd5edf18431b111699afb317e4ac1bcc Mon Sep 17 00:00:00 2001 From: Takeshi Shimada Date: Fri, 31 Oct 2025 00:02:09 +0900 Subject: [PATCH 2/2] fix: SwiftLint violations and update DocC for concatenate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix SwiftLint errors and add documentation for dynamic array usage. Changes: - Add swiftlint:disable for justified force_try in variadic concatenate - Fix line length violations in Store.swift and FlowError.swift - Add DocC example for dynamic task concatenation with empty handling SwiftLint fixes: - ActionTask.swift:479 - Disable force_try (justified: variadic guarantees non-empty) - Store.swift:343 - Split long precondition message across multiple lines - FlowError.swift:193,195 - Use multiline strings for long error messages DocC updates: - Add migration guide for dynamic concatenate in CoreElements.md - Show both static (variadic) and dynamic (array) usage patterns - Demonstrate explicit empty array handling with guard All tests pass (299 tests). SwiftLint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- Sources/Flow/Flow.docc/CoreElements.md | 20 ++++++++++++++++++++ Sources/Flow/Store/ActionTask.swift | 1 + Sources/Flow/Store/FlowError.swift | 10 ++++++++-- Sources/Flow/Store/Store.swift | 6 +++++- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/Sources/Flow/Flow.docc/CoreElements.md b/Sources/Flow/Flow.docc/CoreElements.md index 4066076..3e571e2 100644 --- a/Sources/Flow/Flow.docc/CoreElements.md +++ b/Sources/Flow/Flow.docc/CoreElements.md @@ -209,6 +209,8 @@ return .cancel(id: "save", returning: .cancelled) **Task executing multiple tasks sequentially** +Static tasks (compile-time): + ```swift // Execute multiple steps in sequence return .concatenate( @@ -226,6 +228,24 @@ return .concatenate( ) ``` +Dynamic tasks (runtime) - guard against empty: + +```swift +// Process items dynamically +let tasks = items.map { item in + ActionTask.run { state in + try await process(item) + } +} + +// Empty arrays require explicit handling +guard !tasks.isEmpty else { + return .none // Or handle empty case appropriately +} + +return try .concatenate(tasks) +``` + Each task executes after the previous one completes. The final task's result is returned to the view. ## Next Steps diff --git a/Sources/Flow/Store/ActionTask.swift b/Sources/Flow/Store/ActionTask.swift index 4575d81..801f80f 100644 --- a/Sources/Flow/Store/ActionTask.swift +++ b/Sources/Flow/Store/ActionTask.swift @@ -476,6 +476,7 @@ extension ActionTask { public static func concatenate(_ tasks: ActionTask...) -> ActionTask { // Variadic guarantees at least one element at call site // Safe to force try because tasks cannot be empty + // swiftlint:disable:next force_try try! concatenate(tasks) } diff --git a/Sources/Flow/Store/FlowError.swift b/Sources/Flow/Store/FlowError.swift index 9db6778..ff1d833 100644 --- a/Sources/Flow/Store/FlowError.swift +++ b/Sources/Flow/Store/FlowError.swift @@ -190,9 +190,15 @@ extension FlowError: LocalizedError { case .noTasksToExecute(let context): if let context = context { - return "No tasks to execute in \(context). Empty task arrays are not allowed. If this is intentional, explicitly return .none instead." + return """ + No tasks to execute in \(context). Empty task arrays are not allowed. \ + If this is intentional, explicitly return .none instead. + """ } - return "No tasks to execute. Empty task arrays are not allowed. If empty is valid, explicitly check and return .none." + return """ + No tasks to execute. Empty task arrays are not allowed. \ + If empty is valid, explicitly check and return .none. + """ } } diff --git a/Sources/Flow/Store/Store.swift b/Sources/Flow/Store/Store.swift index ecb52d4..0ae0f29 100644 --- a/Sources/Flow/Store/Store.swift +++ b/Sources/Flow/Store/Store.swift @@ -340,7 +340,11 @@ public final class Store { // 4. flattenConcatenated() preserves this property // // If this precondition fails, there's a bug in ActionTask construction - precondition(!tasks.isEmpty, "Implementation error: concatenated task list is empty. This should be impossible due to concatenate() throwing on empty arrays.") + precondition( + !tasks.isEmpty, + "Implementation error: concatenated task list is empty. " + + "This should be impossible due to concatenate() throwing on empty arrays." + ) var lastResult: F.ActionResult = try await self.executeTask(tasks[0])