[Repo Assist] fix: PersistentHashMap.containsKey returns true for None/unit/null values

src/FSharpx.Collections/PersistentHashMap.fs

@@ -13,6 +13,7 @@ type internal INode =
     abstract member assoc: Thread ref * int * int * obj * obj * Box -> INode
     abstract member find: int * int * obj -> obj
     abstract member tryFind: int * int * obj -> obj option
+    abstract member containsKey: int * int * obj -> bool
     abstract member without: int * int * obj -> INode
     abstract member without: Thread ref * int * int * obj * Box -> INode
     abstract member nodeSeq: unit -> (obj * obj) seq
@@ -220,6 +221,10 @@ and private HashCollisionNode(thread, hashCollisionKey, count', array': obj[]) =
             else
                 None
 
+        member this.containsKey(shift, hash, key) =
+            let idx = this.findIndex(key)
+            idx >= 0 && key = this.array.[idx]
+
         member this.without(shift, hashKey, key) =
             let idx = this.findIndex(key)
 
@@ -345,6 +350,13 @@ and private ArrayNode(thread, count', array': INode[]) =
             else
                 node.tryFind(shift + 5, hash, key)
 
+        member this.containsKey(shift, hash, key) =
+            let idx = mask(hash, shift)
+            let node = this.array.[idx]
+
+            node <> Unchecked.defaultof<INode>
+            && node.containsKey(shift + 5, hash, key)
+
         member this.without(shift, hashKey, key) =
             let idx = mask(hashKey, shift)
             let node = this.array.[idx]
@@ -470,6 +482,21 @@ and private BitmapIndexedNode(thread, bitmap', array': obj[]) =
                 else
                     None
 
+        member this.containsKey(shift, hash, key) =
+            let bit = bitpos(hash, shift)
+
+            if this.bitmap &&& bit = 0 then
+                false
+            else
+                let idx' = index(this.bitmap, bit) * 2
+                let keyOrNull = this.array.[idx']
+                let valOrNode = this.array.[idx' + 1]
+
+                if keyOrNull = null then
+                    (valOrNode :?> INode).containsKey(shift + 5, hash, key)
+                else
+                    key = keyOrNull
+
         member this.assoc(shift, hashKey, key, value, addedLeaf) =
             let bit = bitpos(hashKey, shift)
             let idx' = index(this.bitmap, bit) * 2
@@ -698,7 +725,7 @@ type internal TransientHashMap<[<EqualityConditionalOn>] 'T, 'S when 'T: equalit
     member this.ContainsKey(key: 'T) =
         if key = Unchecked.defaultof<'T> then this.hasNull
         else if this.root = Unchecked.defaultof<INode> then false
-        else this.root.find(0, hash(key), key) <> null
+        else this.root.containsKey(0, hash(key), key)
 
     member this.Add(key: 'T, value: 'S) =
         if key = Unchecked.defaultof<'T> then
@@ -793,7 +820,7 @@ and PersistentHashMap<[<EqualityConditionalOn>] 'T, 'S when 'T: equality and 'S:
     member this.ContainsKey(key: 'T) =
         if key = Unchecked.defaultof<'T> then this.hasNull
         else if this.root = Unchecked.defaultof<INode> then false
-        else this.root.find(0, hash(key), key) <> null
+        else this.root.containsKey(0, hash(key), key)
 
     static member ofSeq(items: ('T * 'S) seq) =
         let mutable ret = TransientHashMap<'T, 'S>.Empty()

tests/FSharpx.Collections.Tests/PersistentHashMapTest.fs

@@ -40,12 +40,32 @@ module PersistentHashMapTests =
               }
 
               //https://github.com/fsprojects/FSharpx.Collections/issues/85
-              ptest "can add None value to empty map" {
+              test "containsKey returns true for None value" {
                   let x = PersistentHashMap<string, string option>.Empty()
 
                   Expect.isTrue "PersistentHashMap.containsKey" (x.Add("Hello", None) |> PersistentHashMap.containsKey "Hello")
               }
 
+              test "containsKey returns true for unit value" {
+                  let map = PersistentHashMap.empty |> PersistentHashMap.add "key" ()
+
+                  Expect.isTrue "containsKey with unit value" (PersistentHashMap.containsKey "key" map)
+                  Expect.isFalse "containsKey absent key" (PersistentHashMap.containsKey "other" map)
+              }
+
+              test "containsKey returns true for None value after multiple adds" {
+                  let map =
+                      PersistentHashMap.empty
+                      |> PersistentHashMap.add "a" (None: string option)
+                      |> PersistentHashMap.add "b" (Some "hello")
+                      |> PersistentHashMap.add "c" None
+
+                  Expect.isTrue "key a with None" (PersistentHashMap.containsKey "a" map)
+                  Expect.isTrue "key b with Some" (PersistentHashMap.containsKey "b" map)
+                  Expect.isTrue "key c with None" (PersistentHashMap.containsKey "c" map)
+                  Expect.isFalse "absent key" (PersistentHashMap.containsKey "d" map)
+              }
+
               test "can PersistentHashMap.add PersistentHashMap.empty string as key to PersistentHashMap.empty map" {
                   Expect.isFalse "PersistentHashMap.empty"
                   <| PersistentHashMap.containsKey "" PersistentHashMap.empty

tests/FSharpx.Collections.Tests/TransientHashMapTest.fs

@@ -67,7 +67,7 @@ module TransientHashMapTests =
               }
 
               //https://github.com/fsprojects/FSharpx.Collections/issues/85
-              ptest "can add None value to empty map" {
+              test "can add None value to empty map" {
                   let x = TransientHashMap<string, string option>.Empty()
 
                   Expect.isTrue