Skip to content

Commit 5a70f34

Browse files
committed
Fix noop in ordered map when replacing value with an equal one, but not the same
1 parent 82d71e5 commit 5a70f34

File tree

2 files changed

+20
-7
lines changed
  • kotlinx-collections-immutable

2 files changed

+20
-7
lines changed

kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/persistentOrderedMap/PersistentOrderedMap.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ internal class PersistentOrderedMap<K, V>(
8686

8787
val links = hashMap[key]
8888
if (links != null) {
89-
if (links.value == value) {
89+
if (links.value === value) {
9090
return this
9191
}
9292
val newMap = hashMap.put(key, links.withValue(value))

kotlinx-collections-immutable/tests/src/test/kotlin/kotlinx.collections.immutable/contractTests/immutableMap/ImmutableMapTest.kt

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ abstract class ImmutableMapTest {
6060
val empty2 = immutableMapOf<String, Int>()
6161
assertEquals<ImmutableMap<*, *>>(empty1, empty2)
6262
assertEquals(mapOf<Int, String>(), empty1)
63-
assertTrue(empty1 === empty2)
63+
assertSame<ImmutableMap<*, *>>(empty1, empty2)
6464

6565
compareMaps(emptyMap(), empty1)
6666
}
@@ -85,7 +85,7 @@ abstract class ImmutableMapTest {
8585
val map = HashMap(original) // copy
8686
var immMap = map.toPersistentMap()
8787
val immMap2 = immMap.toImmutableMap()
88-
assertTrue(immMap2 === immMap)
88+
assertSame(immMap2, immMap)
8989

9090
compareMapsUnordered(original, immMap)
9191
compareMapsUnordered(map, immMap)
@@ -115,6 +115,19 @@ abstract class ImmutableMapTest {
115115
assertEquals(mapOf("x" to null, "y" to 1, "x!" to null, "y!" to 1), map)
116116
}
117117

118+
@Test fun putEqualButNotSameValue() {
119+
data class Value<T>(val value: T)
120+
val map = immutableMapOf("x" to Value(1))
121+
122+
val newValue = Value(1)
123+
val newMap = map.put("x", newValue)
124+
assertNotSame(map, newMap)
125+
assertEquals(map, newMap)
126+
127+
val sameMap = newMap.put("x", newValue)
128+
assertSame(newMap, sameMap)
129+
}
130+
118131
@Test fun removeElements() {
119132
val map = immutableMapOf("x" to 1, null to "x").toPersistentMap()
120133

@@ -149,10 +162,10 @@ abstract class ImmutableMapTest {
149162
"abcxaxyz12".associateTo(builder) { it to it.toInt() }
150163
val map = builder.build()
151164
assertEquals<Map<*, *>>(map, builder)
152-
assertTrue(map === builder.build(), "Building the same list without modifications")
165+
assertSame(map, builder.build(), "Building the same list without modifications")
153166

154167
val map2 = builder.toImmutableMap()
155-
assertTrue(map2 === map, "toImmutable calls build()")
168+
assertSame(map2, map, "toImmutable calls build()")
156169

157170
with(map) {
158171
testMutation { put('K', null) }
@@ -191,8 +204,8 @@ abstract class ImmutableMapTest {
191204
val result = this.persistent()
192205
val buildResult = this.mutate(mutating)
193206
// Ensure non-mutating operations return the same instance
194-
assertTrue(this === result)
195-
assertTrue(this === buildResult)
207+
assertSame(this, result)
208+
assertSame(this, buildResult)
196209
}
197210

198211

0 commit comments

Comments
 (0)