fix(effect): TMap.remove/removeAll clears entire bucket on hash collision

.changeset/fix-tmap-remove-colliding-keys.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Fix TMap.remove and removeAll erroneously clearing entire bucket on hash collision.

packages/effect/src/internal/stm/tMap.ts

@@ -305,7 +305,7 @@ export const remove = dual<
     const buckets = tRef.unsafeGet(self.tBuckets, journal)
     const index = indexOf(key, buckets.chunk.length)
     const bucket = tRef.unsafeGet(buckets.chunk[index], journal)
-    const [toRemove, toRetain] = Chunk.partition(bucket, (entry) => Equal.equals(entry[1], key))
+    const [toRetain, toRemove] = Chunk.partition(bucket, (entry) => Equal.equals(entry[0], key))
     if (Chunk.isNonEmpty(toRemove)) {
       const currentSize = tRef.unsafeGet(self.tSize, journal)
       tRef.unsafeSet(buckets.chunk[index], toRetain, journal)
@@ -325,7 +325,7 @@ export const removeAll = dual<
       const buckets = tRef.unsafeGet(self.tBuckets, journal)
       const index = indexOf(next.value, buckets.chunk.length)
       const bucket = tRef.unsafeGet(buckets.chunk[index], journal)
-      const [toRemove, toRetain] = Chunk.partition(bucket, (entry) => Equal.equals(next.value)(entry[0]))
+      const [toRetain, toRemove] = Chunk.partition(bucket, (entry) => Equal.equals(next.value)(entry[0]))
       if (Chunk.isNonEmpty(toRemove)) {
         const currentSize = tRef.unsafeGet(self.tSize, journal)
         tRef.unsafeSet(buckets.chunk[index], toRetain, journal)

packages/effect/test/TMap.test.ts

@@ -13,6 +13,16 @@ class HashContainer implements Equal.Equal {
   }
 }
 
+class CollidingKey implements Equal.Equal {
+  constructor(readonly id: string) {}
+  [Hash.symbol](): number {
+    return 1
+  }
+  [Equal.symbol](that: unknown): boolean {
+    return that instanceof CollidingKey && this.id === that.id
+  }
+}
+
 const mapEntriesArb: fc.Arbitrary<Array<readonly [string, number]>> = fc.uniqueArray(fc.char())
   .chain((keys) =>
     fc.uniqueArray(fc.integer())
@@ -190,6 +200,50 @@ describe("TMap", () => {
       assertNone(result)
     }))
 
+  it.effect("remove - preserves colliding keys", () =>
+    Effect.gen(function*() {
+      const a = new CollidingKey("a")
+      const b = new CollidingKey("b")
+      const c = new CollidingKey("c")
+      const transaction = STM.gen(function*() {
+        const map = yield* (TMap.make([a, 1], [b, 2], [c, 3]))
+        yield* (pipe(map, TMap.remove(b)))
+        return yield* (STM.all({
+          a: pipe(map, TMap.get(a)),
+          b: pipe(map, TMap.get(b)),
+          c: pipe(map, TMap.get(c)),
+          size: TMap.size(map)
+        }))
+      })
+      const result = yield* (STM.commit(transaction))
+      assertSome(result.a, 1)
+      assertNone(result.b)
+      assertSome(result.c, 3)
+      strictEqual(result.size, 2)
+    }))
+
+  it.effect("removeAll - preserves colliding keys", () =>
+    Effect.gen(function*() {
+      const a = new CollidingKey("a")
+      const b = new CollidingKey("b")
+      const c = new CollidingKey("c")
+      const transaction = STM.gen(function*() {
+        const map = yield* (TMap.make([a, 1], [b, 2], [c, 3]))
+        yield* (pipe(map, TMap.removeAll([b])))
+        return yield* (STM.all({
+          a: pipe(map, TMap.get(a)),
+          b: pipe(map, TMap.get(b)),
+          c: pipe(map, TMap.get(c)),
+          size: TMap.size(map)
+        }))
+      })
+      const result = yield* (STM.commit(transaction))
+      assertSome(result.a, 1)
+      assertNone(result.b)
+      assertSome(result.c, 3)
+      strictEqual(result.size, 2)
+    }))
+
   it.effect("removeIf", () =>
     Effect.gen(function*() {
       const transaction = STM.gen(function*() {