Assorted fixes for collections of state

.github/workflows/format.yml

@@ -1,7 +1,7 @@
 name: Format
 
 on:
-  pull_request:
+  push:
     branch: 'master'
 
 jobs:

Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-LoadThenNavigate.swift

@@ -52,15 +52,15 @@ let lazyListNavigationReducer = Reducer<
 
     case .setNavigation(selection: .none):
       if let selection = state.selection {
-        state.rows[selection.id]?.count = selection.count
+        state.rows[id: selection.id]?.count = selection.count
         state.selection = nil
       }
       return .none
 
     case let .setNavigationSelectionDelayCompleted(id):
-      state.rows[id]?.isActivityIndicatorVisible = false
+      state.rows[id: id]?.isActivityIndicatorVisible = false
       state.selection = Identified(
-        CounterState(count: state.rows[id]?.count ?? 0),
+        CounterState(count: state.rows[id: id]?.count ?? 0),
         id: id
       )
       return .none

Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-NavigateAndLoad.swift

@@ -49,14 +49,14 @@ let eagerListNavigationReducer = Reducer<
 
     case .setNavigation(selection: .none):
       if let selection = state.selection, let count = selection.value?.count {
-        state.rows[selection.id]?.count = count
+        state.rows[id: selection.id]?.count = count
         state.selection = nil
       }
       return .cancel(id: CancelId())
 
     case .setNavigationSelectionDelayCompleted:
       guard let id = state.selection?.id else { return .none }
-      state.selection?.value = CounterState(count: state.rows[id]?.count ?? 0)
+      state.selection?.value = CounterState(count: state.rows[id: id]?.count ?? 0)
       return .none
     }
   },

Sources/ComposableArchitecture/Reducer.swift

@@ -194,6 +194,17 @@ public struct Reducer<State, Action, Environment> {
       guard let (index, localAction) = toLocalAction.extract(from: globalAction) else {
         return .none
       }
+      // NB: This does not need to be a fatal error because of the index subscript that follows it.
+      assert(
+        index < globalState[keyPath: toLocalState].endIndex,
+        """
+        Index out of range. This can happen when a reducer that can remove the last element from \
+        an array is then combined with a "forEach" from that array. To avoid this and other \
+        index-related gotchas, consider using an "IdentifiedArray" of state instead. Or, combine \
+        your reducers so that the "forEach" comes before any reducer that can remove elements from \
+        its array.
+        """
+      )
       return self.reducer(
         &globalState[keyPath: toLocalState][index],
         localAction,
@@ -238,7 +249,7 @@ public struct Reducer<State, Action, Environment> {
       guard let (id, localAction) = toLocalAction.extract(from: globalAction) else { return .none }
       return self.optional
         .reducer(
-          &globalState[keyPath: toLocalState][id],
+          &globalState[keyPath: toLocalState][id: id],
           localAction,
           toLocalEnvironment(globalEnvironment)
         )

Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift

@@ -105,7 +105,7 @@ where Data: Collection, ID: Hashable, Content: View {
         ForEach(viewStore.state, id: viewStore.id) { element in
           content(
             store.scope(
-              state: { $0[element[keyPath: viewStore.id]] ?? element },
+              state: { $0[id: element[keyPath: viewStore.id]] ?? element },
               action: { (element[keyPath: viewStore.id], $0) }
             )
           )

Sources/ComposableArchitecture/SwiftUI/IdentifiedArray.swift

@@ -92,12 +92,15 @@ where ID: Hashable {
     }
   }
 
-  public subscript(id: ID) -> Element? {
+  public subscript(id id: ID) -> Element? {
     get {
       self.dictionary[id]
     }
     _modify {
       yield &self.dictionary[id]
+      if self.dictionary[id] == nil {
+        self.ids.removeAll(where: { $0 == id })
+      }
     }
   }
 
@@ -115,6 +118,7 @@ where ID: Hashable {
     }
   }
 
+  @discardableResult
   public mutating func remove(at position: Int) -> Element {
     let id = self.ids.remove(at: position)
     let element = self.dictionary[id]!
@@ -138,7 +142,7 @@ where ID: Hashable {
   }
 
   public mutating func remove(atOffsets offsets: IndexSet) {
-    for offset in offsets {
+    for offset in offsets.reversed() {
       _ = self.remove(at: offset)
     }
   }

Tests/ComposableArchitectureTests/IdentifiedArrayTests.swift

@@ -0,0 +1,113 @@
+import XCTest
+
+@testable import ComposableArchitecture
+
+final class IdentifiedArrayTests: XCTestCase {
+  func testIdSubscript() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [User(id: 1, name: "Blob")]
+
+    XCTAssertEqual(array[id: 1], .some(User(id: 1, name: "Blob")))
+
+    array[id: 1] = nil
+    XCTAssertEqual(array, [])
+  }
+
+  func testInsert() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [User(id: 1, name: "Blob")]
+
+    array.insert(User(id: 2, name: "Blob Jr."), at: 0)
+    XCTAssertEqual(array, [User(id: 2, name: "Blob Jr."), User(id: 1, name: "Blob")])
+  }
+
+  func testInsertContentsOf() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [User(id: 1, name: "Blob")]
+
+    array.insert(contentsOf: [User(id: 3, name: "Blob Sr."), User(id: 2, name: "Blob Jr.")], at: 0)
+    XCTAssertEqual(
+      array,
+      [User(id: 3, name: "Blob Sr."), User(id: 2, name: "Blob Jr."), User(id: 1, name: "Blob")]
+    )
+  }
+
+  func testRemoveAt() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [
+      User(id: 3, name: "Blob Sr."),
+      User(id: 2, name: "Blob Jr."),
+      User(id: 1, name: "Blob"),
+    ]
+
+    array.remove(at: 1)
+    XCTAssertEqual(array, [User(id: 3, name: "Blob Sr."), User(id: 1, name: "Blob")])
+  }
+
+  func testRemoveAllWhere() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [
+      User(id: 3, name: "Blob Sr."),
+      User(id: 2, name: "Blob Jr."),
+      User(id: 1, name: "Blob"),
+    ]
+
+    array.removeAll(where: { $0.name.starts(with: "Blob ") })
+    XCTAssertEqual(array, [User(id: 1, name: "Blob")])
+  }
+
+  func testRemoveAtOffsets() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [
+      User(id: 3, name: "Blob Sr."),
+      User(id: 2, name: "Blob Jr."),
+      User(id: 1, name: "Blob"),
+    ]
+
+    array.remove(atOffsets: [0, 2])
+    XCTAssertEqual(array, [User(id: 2, name: "Blob Jr.")])
+  }
+
+  func testMoveFromOffsets() {
+    struct User: Equatable, Identifiable {
+      let id: Int
+      var name: String
+    }
+
+    var array: IdentifiedArray = [
+      User(id: 3, name: "Blob Sr."),
+      User(id: 2, name: "Blob Jr."),
+      User(id: 1, name: "Blob"),
+    ]
+
+    array.move(fromOffsets: [0], toOffset: 2)
+    XCTAssertEqual(
+      array,
+      [User(id: 2, name: "Blob Jr."), User(id: 3, name: "Blob Sr."), User(id: 1, name: "Blob")]
+    )
+  }
+}