Merge pull request #592 from durban/issue590

HashMap: fix removal with collision

Author: armanbilge

core/src/main/scala/cats/collections/HashMap.scala

@@ -617,7 +617,7 @@ object HashMap extends HashMapInstances with compat.HashMapCompatCompanion {
         this
       else if (contents.toVector.lengthCompare(2) == 0) {
         // There will no longer be any collisions once the key is removed
-        val keepIndex = ~keyIndex
+        val keepIndex = 1 - keyIndex
         // This is a singleton node so the depth doesn't matter;
         // we only need to index into it to inline the value in our parent node
         val mask = Node.maskFrom(collisionHash, depth = 0)

tests/src/test/scala/cats/collections/HashMapSuite.scala

@@ -23,6 +23,7 @@ package cats.collections
 
 import cats.collections.arbitrary.hashmap._
 import cats.collections.tests.compat._
+import cats.kernel.Hash
 import cats.kernel.laws.discipline.CommutativeMonoidTests
 import cats.kernel.laws.discipline.HashTests
 import cats.laws.discipline.SerializableTests
@@ -164,6 +165,18 @@ class HashMapSuite extends DisciplineSuite with ScalaVersionSpecificSyntax {
     }
   }
 
+  test("removed with collision") {
+    val myHash: Hash[Int] = new Hash[Int] {
+      final override def eqv(x: Int, y: Int): Boolean =
+        (x % 8) === (y % 8)
+      final override def hash(x: Int): Int =
+        x % 4
+    }
+    val m1 = HashMap(1 -> 1, 8 -> 8, 4 -> 4)(myHash)
+    val m2 = m1.removed(16)
+    assertEquals(m2, HashMap(1 -> 1, 4 -> 4)(myHash))
+  }
+
   test("concat") {
     forAll { (left: HashMap[Int, String], right: HashMap[Int, String]) =>
       val leftKvs = left.iterator.toList