Correctly implement specialized MutableEntrySet.contains KT-41278

Author: qurbonzoda

core/commonMain/src/implementations/immutableMap/PersistentHashMapBuilderContentViews.kt

@@ -5,8 +5,18 @@
 
 package kotlinx.collections.immutable.implementations.immutableMap
 
+// intermediate abstract class to workaround KT-43321
+internal abstract class AbstractMapBuilderEntries<E : Map.Entry<K, V>, K, V> : AbstractMutableSet<E>() {
+    final override fun contains(element: E): Boolean {
+        // TODO: Eliminate this check after KT-30016 gets fixed.
+        if ((element as? Any?) !is Map.Entry<*, *>) return false
+        return containsEntry(element)
+    }
+    abstract fun containsEntry(element: Map.Entry<K, V>): Boolean
+}
+
 internal class PersistentHashMapBuilderEntries<K, V>(private val builder: PersistentHashMapBuilder<K, V>)
-    : MutableSet<MutableMap.MutableEntry<K, V>>, AbstractMutableSet<MutableMap.MutableEntry<K, V>>() {
+    : AbstractMapBuilderEntries<MutableMap.MutableEntry<K, V>, K, V>() {
     override fun add(element: MutableMap.MutableEntry<K, V>): Boolean {
         throw UnsupportedOperationException()
     }
@@ -28,9 +38,7 @@ internal class PersistentHashMapBuilderEntries<K, V>(private val builder: Persis
     override val size: Int
         get() = builder.size
 
-    override fun contains(element: MutableMap.MutableEntry<K, V>): Boolean {
-        // TODO: Eliminate this check after KT-30016 gets fixed.
-        if ((element as Any?) !is Map.Entry<*, *>) return false
+    override fun containsEntry(element: Map.Entry<K, V>): Boolean {
         return builder[element.key]?.let { candidate -> candidate == element.value }
                 ?: (element.value == null && builder.containsKey(element.key))
     }

core/commonMain/src/implementations/persistentOrderedMap/PersistentOrderedMapBuilderContentViews.kt

@@ -5,8 +5,10 @@
 
 package kotlinx.collections.immutable.implementations.persistentOrderedMap
 
+import kotlinx.collections.immutable.implementations.immutableMap.AbstractMapBuilderEntries
+
 internal class PersistentOrderedMapBuilderEntries<K, V>(private val builder: PersistentOrderedMapBuilder<K, V>)
-    : MutableSet<MutableMap.MutableEntry<K, V>>, AbstractMutableSet<MutableMap.MutableEntry<K, V>>() {
+    : AbstractMapBuilderEntries<MutableMap.MutableEntry<K, V>, K, V>() {
     override fun add(element: MutableMap.MutableEntry<K, V>): Boolean {
         throw UnsupportedOperationException()
     }
@@ -28,9 +30,7 @@ internal class PersistentOrderedMapBuilderEntries<K, V>(private val builder: Per
     override val size: Int
         get() = builder.size
 
-    override fun contains(element: MutableMap.MutableEntry<K, V>): Boolean {
-        // TODO: Eliminate this check after KT-30016 gets fixed.
-        if ((element as Any?) !is Map.Entry<*, *>) return false
+    override fun containsEntry(element: Map.Entry<K, V>): Boolean {
         return builder[element.key]?.let { candidate -> candidate == element.value }
                 ?: (element.value == null && builder.containsKey(element.key))
     }

core/commonTest/src/contract/map/KT41278Test.kt

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2016-2021 JetBrains s.r.o.
+ * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
+ */
+
+package tests.contract.map
+
+import kotlinx.collections.immutable.persistentHashMapOf
+import kotlinx.collections.immutable.persistentMapOf
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+
+class KT41278Test {
+    // Based on https://youtrack.jetbrains.com/issue/KT-42428.
+    private fun doTest(map: Map<String, Int>, key: String, value: Int, createEntry: (String, Int) -> Map.Entry<String, Int>) {
+        assertTrue(map.keys.contains(key))
+        assertEquals(value, map[key])
+        // This one requires special efforts to make it work this way.
+        // map.entries can in fact be `MutableSet<MutableMap.MutableEntry>`,
+        // which [contains] method takes [MutableEntry], so the compiler may generate special bridge
+        // returning false for values that aren't [MutableEntry].
+        assertTrue(map.entries.contains(createEntry(key, value)))
+        assertTrue(map.entries.toSet().contains(createEntry(key, value)))
+
+        assertFalse(map.entries.contains(null as Any?))
+        assertFalse(map.entries.contains("not an entry" as Any?))
+    }
+
+    @Test
+    fun persistentOrderedMap() {
+        val mapLetterToIndex = ('a'..'z').mapIndexed { i, c -> "$c" to i }.fold(persistentMapOf<String, Int>()) { map, pair ->
+            map.put(pair.first, pair.second)
+        }
+
+        doTest(mapLetterToIndex, "h", 7, ::TestMapEntry)
+        doTest(mapLetterToIndex, "h", 7, ::TestMutableMapEntry)
+    }
+
+    @Test
+    fun persistentHashMap() {
+        val mapLetterToIndex = ('a'..'z').mapIndexed { i, c -> "$c" to i }.fold(persistentHashMapOf<String, Int>()) { map, pair ->
+            map.put(pair.first, pair.second)
+        }
+
+        doTest(mapLetterToIndex, "h", 7, ::TestMapEntry)
+        doTest(mapLetterToIndex, "h", 7, ::TestMutableMapEntry)
+    }
+
+    @Test
+    fun persistentOrderedMapBuilder() {
+        val mapLetterToIndex = persistentMapOf<String, Int>().builder().apply { putAll(('a'..'z').mapIndexed { i, c -> "$c" to i }) }
+
+        doTest(mapLetterToIndex, "h", 7, ::TestMapEntry)
+        doTest(mapLetterToIndex, "h", 7, ::TestMutableMapEntry)
+    }
+
+    @Test
+    fun persistentHashMapBuilder() {
+        val mapLetterToIndex = persistentHashMapOf<String, Int>().builder().apply { putAll(('a'..'z').mapIndexed { i, c -> "$c" to i }) }
+
+        doTest(mapLetterToIndex, "h", 7, ::TestMapEntry)
+        doTest(mapLetterToIndex, "h", 7, ::TestMutableMapEntry)
+    }
+}
+
+private class TestMapEntry<out K, out V>(override val key: K, override val value: V) : Map.Entry<K, V> {
+    override fun toString(): String = "$key=$value"
+    override fun hashCode(): Int = key.hashCode() xor value.hashCode()
+    override fun equals(other: Any?): Boolean =
+            other is Map.Entry<*, *> && key == other.key && value == other.value
+}
+
+private class TestMutableMapEntry<K, V>(override val key: K, override val value: V) : MutableMap.MutableEntry<K, V> {
+    override fun toString(): String = "$key=$value"
+    override fun hashCode(): Int = key.hashCode() xor value.hashCode()
+    override fun equals(other: Any?): Boolean =
+            other is Map.Entry<*, *> && key == other.key && value == other.value
+
+    override fun setValue(newValue: V): V = TODO("Not yet implemented")
+}