Correctly implement specialized MutableEntrySet.contains KT-41278

This is a workaround for the problem KT-43321.

Introduce an intermediate abstract set specialized for Map.Entry elements
and implement 'contains(Map.Entry)' method there.

Then inherit that intermediate set in entrysets of JS HashMap,
JS LinkedHashMap, JVM MapBuilder, that are specialized for
MutableMap.MutableEntry elements, so that no override of 'contains' is
required anymore.

This allows to avoid incorrect special 'contains' bridge being generated
that otherwise rejects all arguments except ones of MutableEntry type.

libraries/stdlib/js/src/kotlin/collections/AbstractMutableMap.kt

@@ -46,6 +46,12 @@ public actual abstract class AbstractMutableMap<K, V> protected actual construct
 
     }
 
+    // intermediate abstract class to workaround KT-43321
+    internal abstract class AbstractEntrySet<E : Map.Entry<K, V>, K, V> : AbstractMutableSet<E>() {
+        final override fun contains(element: E): Boolean = containsEntry(element)
+        abstract fun containsEntry(element: Map.Entry<K, V>): Boolean
+    }
+
     actual override fun clear() {
         entries.clear()
     }

libraries/stdlib/js/src/kotlin/collections/HashMap.kt

@@ -1,7 +1,8 @@
 /*
- * Copyright 2010-2018 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
  * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
  */
+
 /*
  * Based on GWT AbstractHashMap
  * Copyright 2008 Google Inc.
@@ -20,14 +21,14 @@ import kotlin.collections.MutableMap.MutableEntry
 // have to make sure mutating methods check `checkIsMutable`.
 public actual open class HashMap<K, V> : AbstractMutableMap<K, V>, MutableMap<K, V> {
 
-    private inner class EntrySet : AbstractMutableSet<MutableEntry<K, V>>() {
+    private inner class EntrySet : AbstractEntrySet<MutableEntry<K, V>, K, V>() {
 
         override fun add(element: MutableEntry<K, V>): Boolean = throw UnsupportedOperationException("Add is not supported on entries")
         override fun clear() {
             this@HashMap.clear()
         }
 
-        override operator fun contains(element: MutableEntry<K, V>): Boolean = containsEntry(element)
+        override fun containsEntry(element: Map.Entry<K, V>): Boolean = this@HashMap.containsEntry(element)
 
         override operator fun iterator(): MutableIterator<MutableEntry<K, V>> = internalMap.iterator()
 
@@ -121,4 +122,4 @@ public actual open class HashMap<K, V> : AbstractMutableMap<K, V>, MutableMap<K,
  */
 public fun <V> stringMapOf(vararg pairs: Pair<String, V>): HashMap<String, V> {
     return HashMap<String, V>(InternalStringMap(EqualityComparator.HashCode)).apply { putAll(pairs) }
-}
+}

libraries/stdlib/js/src/kotlin/collections/LinkedHashMap.kt

@@ -1,7 +1,8 @@
 /*
- * Copyright 2010-2018 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
  * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
  */
+
 /*
  * Based on GWT LinkedHashMap
  * Copyright 2008 Google Inc.
@@ -40,7 +41,7 @@ public actual open class LinkedHashMap<K, V> : HashMap<K, V>, MutableMap<K, V> {
         }
     }
 
-    private inner class EntrySet : AbstractMutableSet<MutableEntry<K, V>>() {
+    private inner class EntrySet : AbstractEntrySet<MutableEntry<K, V>, K, V>() {
 
         private inner class EntryIterator : MutableIterator<MutableEntry<K, V>> {
             // The last entry that was returned from this iterator.
@@ -85,7 +86,7 @@ public actual open class LinkedHashMap<K, V> : HashMap<K, V>, MutableMap<K, V> {
             this@LinkedHashMap.clear()
         }
 
-        override operator fun contains(element: MutableEntry<K, V>): Boolean = containsEntry(element)
+        override fun containsEntry(element: Map.Entry<K, V>): Boolean = this@LinkedHashMap.containsEntry(element)
 
         override operator fun iterator(): MutableIterator<MutableEntry<K, V>> = EntryIterator()
 
@@ -275,4 +276,4 @@ public actual open class LinkedHashMap<K, V> : HashMap<K, V>, MutableMap<K, V> {
  */
 public fun <V> linkedStringMapOf(vararg pairs: Pair<String, V>): LinkedHashMap<String, V> {
     return LinkedHashMap<String, V>(stringMapOf<Any>()).apply { putAll(pairs) }
-}
+}

libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt

@@ -595,13 +595,19 @@ internal class MapBuilderValues<V> internal constructor(
     }
 }
 
+// intermediate abstract class to workaround KT-43321
+internal abstract class AbstractMapBuilderEntrySet<E : Map.Entry<K, V>, K, V> : AbstractMutableSet<E>() {
+    final override fun contains(element: E): Boolean = containsEntry(element)
+    abstract fun containsEntry(element: Map.Entry<K, V>): Boolean
+}
+
 internal class MapBuilderEntries<K, V> internal constructor(
     val backing: MapBuilder<K, V>
-) : MutableSet<MutableMap.MutableEntry<K, V>>, AbstractMutableSet<MutableMap.MutableEntry<K, V>>() {
+) : AbstractMapBuilderEntrySet<MutableMap.MutableEntry<K, V>, K, V>() {
 
     override val size: Int get() = backing.size
     override fun isEmpty(): Boolean = backing.isEmpty()
-    override fun contains(element: MutableMap.MutableEntry<K, V>): Boolean = backing.containsEntry(element)
+    override fun containsEntry(element: Map.Entry<K, V>): Boolean = backing.containsEntry(element)
     override fun clear() = backing.clear()
     override fun add(element: MutableMap.MutableEntry<K, V>): Boolean = throw UnsupportedOperationException()
     override fun addAll(elements: Collection<MutableMap.MutableEntry<K, V>>): Boolean = throw UnsupportedOperationException()

libraries/stdlib/test/collections/MapTest.kt

@@ -400,6 +400,43 @@ class MapTest {
         assertEquals(3, filteredByValue["b"])
     }
 
+    @Test
+    fun entriesCovariantContains() {
+        // Based on https://youtrack.jetbrains.com/issue/KT-42428.
+        fun doTest(implName: String, map: Map<String, Int>, key: String, value: Int) {
+            class SimpleEntry<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
+            }
+
+            val mapDescription = "$implName: ${map::class}"
+
+            assertTrue(map.keys.contains(key), mapDescription)
+            assertEquals(value, map[key], mapDescription)
+            // 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] (including [SimpleEntry]).
+            assertTrue(map.entries.contains(SimpleEntry(key, value)), mapDescription)
+            assertTrue(map.entries.toSet().contains(SimpleEntry(key, value)), mapDescription)
+        }
+
+        val mapLetterToIndex = ('a'..'z').mapIndexed { i, c -> "$c" to i }.toMap()
+        doTest("default read-only", mapLetterToIndex, "h", 7)
+        doTest("default mutable", mapLetterToIndex.toMutableMap(), "b", 1)
+        doTest("HashMap", mapLetterToIndex.toMap(HashMap()), "c", 2)
+        doTest("LinkedHashMap", mapLetterToIndex.toMap(LinkedHashMap()), "d", 3)
+
+        val builtMap = buildMap {
+            putAll(mapLetterToIndex)
+            doTest("MapBuilder", this, "z", 25)
+        }
+        doTest("built Map", builtMap, "y", 24)
+    }
+
+
     fun testPlusAssign(doPlusAssign: (MutableMap<String, Int>) -> Unit) {
         val map = hashMapOf("a" to 1, "b" to 2)
         doPlusAssign(map)