From f152fa537dfb1448af0f65f0193ad2d3a6a7c56e Mon Sep 17 00:00:00 2001 From: Abduqodiri Qurbonzoda Date: Tue, 11 Jul 2023 09:58:38 +0000 Subject: [PATCH] Collection.toString() should not throw if it contains itself Merge-request: KT-MR-10591 Merged-by: Abduqodiri Qurbonzoda --- .../kotlin/kotlin/collections/ArrayUtil.kt | 15 ----- .../collections/builders/ListBuilder.kt | 11 +++- .../kotlin/collections/builders/MapBuilder.kt | 4 +- .../src/kotlin/collections/ArrayList.kt | 2 +- .../src/kotlin/collections/Arrays.kt | 9 ++- .../src/kotlin/collections/HashMap.kt | 4 +- .../stdlib/test/collections/CollectionTest.kt | 55 ++++++++++++++++++- 7 files changed, 72 insertions(+), 28 deletions(-) diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/collections/ArrayUtil.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/collections/ArrayUtil.kt index 71bca9a23f2..169bd2ed0b9 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/collections/ArrayUtil.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/collections/ArrayUtil.kt @@ -137,18 +137,3 @@ internal external fun arrayCopy(array: DoubleArray, fromIndex: Int, destination: @GCUnsafeCall("Kotlin_BooleanArray_copyImpl") internal external fun arrayCopy(array: BooleanArray, fromIndex: Int, destination: BooleanArray, toIndex: Int, count: Int) - -internal fun Collection.collectionToString(): String { - val sb = StringBuilder(2 + size * 3) - sb.append("[") - var i = 0 - val it = iterator() - while (it.hasNext()) { - if (i > 0) sb.append(", ") - val next = it.next() - if (next == this) sb.append("(this Collection)") else sb.append(next) - i++ - } - sb.append("]") - return sb.toString() -} diff --git a/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt b/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt index 4b17489f92f..bea37cd16cd 100644 --- a/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt +++ b/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt @@ -174,7 +174,7 @@ internal class ListBuilder private constructor( } override fun toString(): String { - return array.subarrayContentToString(offset, length) + return array.subarrayContentToString(offset, length, this) } // ---------------------------- private ---------------------------- @@ -359,13 +359,18 @@ internal fun arrayOfUninitializedElements(size: Int): Array { return arrayOfNulls(size) as Array } -private fun Array.subarrayContentToString(offset: Int, length: Int): String { +private fun Array.subarrayContentToString(offset: Int, length: Int, thisCollection: Collection): String { val sb = StringBuilder(2 + length * 3) sb.append("[") var i = 0 while (i < length) { if (i > 0) sb.append(", ") - sb.append(this[offset + i]) + val nextElement = this[offset + i] + if (nextElement === thisCollection) { + sb.append("(this Collection)") + } else { + sb.append(nextElement) + } i++ } sb.append("]") diff --git a/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt b/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt index 921a6219a5b..422b5fcef4b 100644 --- a/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt +++ b/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt @@ -571,10 +571,10 @@ internal class MapBuilder private constructor( if (index >= map.length) throw NoSuchElementException() lastIndex = index++ val key = map.keysArray[lastIndex] - if (key == map) sb.append("(this Map)") else sb.append(key) + if (key === map) sb.append("(this Map)") else sb.append(key) sb.append('=') val value = map.valuesArray!![lastIndex] - if (value == map) sb.append("(this Map)") else sb.append(value) + if (value === map) sb.append("(this Map)") else sb.append(value) initNext() } } diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt index 82b6debfe6e..5d322fd10af 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt @@ -182,7 +182,7 @@ actual class ArrayList private constructor( } override fun toString(): String { - return backingArray.subarrayContentToString(offset, length) + return backingArray.subarrayContentToString(offset, length, this) } @Suppress("UNCHECKED_CAST") diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/Arrays.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/Arrays.kt index a9081ec199f..93fce876762 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/Arrays.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/Arrays.kt @@ -23,13 +23,18 @@ internal fun checkCopyOfRangeArguments(fromIndex: Int, toIndex: Int, size: Int) /** * Returns a string representation of the contents of the subarray of the specified array as if it is [List]. */ -internal inline fun Array.subarrayContentToString(offset: Int, length: Int): String { +internal inline fun Array.subarrayContentToString(offset: Int, length: Int, thisCollection: Collection): String { val sb = StringBuilder(2 + length * 3) sb.append("[") var i = 0 while (i < length) { if (i > 0) sb.append(", ") - sb.append(this[offset + i]) + val nextElement = this[offset + i] + if (nextElement === thisCollection) { + sb.append("(this Collection)") + } else { + sb.append(nextElement) + } i++ } sb.append("]") diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt index 0862edd6855..9876ee30c97 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt @@ -634,10 +634,10 @@ actual class HashMap private constructor( if (index >= map.length) throw NoSuchElementException() lastIndex = index++ val key = map.keysArray[lastIndex] - if (key == map) sb.append("(this Map)") else sb.append(key) + if (key === map) sb.append("(this Map)") else sb.append(key) sb.append('=') val value = map.valuesArray!![lastIndex] - if (value == map) sb.append("(this Map)") else sb.append(value) + if (value === map) sb.append("(this Map)") else sb.append(value) initNext() } } diff --git a/libraries/stdlib/test/collections/CollectionTest.kt b/libraries/stdlib/test/collections/CollectionTest.kt index 10c7166f8a3..43f0ef43243 100644 --- a/libraries/stdlib/test/collections/CollectionTest.kt +++ b/libraries/stdlib/test/collections/CollectionTest.kt @@ -5,9 +5,7 @@ package test.collections -import test.assertIsNegativeZero -import test.assertIsPositiveZero -import test.assertStaticAndRuntimeTypeIs +import test.* import kotlin.test.* import test.collections.behaviors.* import test.comparisons.STRING_CASE_INSENSITIVE_ORDER @@ -1224,6 +1222,57 @@ class CollectionTest { assertEquals("[1, a, null, ${Long.MAX_VALUE.toString()}]", listOf(1, "a", null, Long.MAX_VALUE).toString()) } + @Test fun toStringContainingThis() = testExceptOn(TestPlatform.Js) { + // resulting string is platform-dependent, but shouldn't throw + arrayOf("a", "b", "c").apply { this[1] = this }.toString() + + assertEquals( + "[a, (this Collection), c]", + arrayListOf("a", "b", "c").apply { this[1] = this }.toString() + ) + assertEquals( + "[a, (this Collection), c]", + buildList { + addAll(listOf("a", "b", "c")) + this[1] = this + }.toString() + ) + + assertEquals( + "[a, (this Collection), c]", + linkedSetOf().apply { + add("a") + add(this) + add("c") + }.toString() + ) + assertEquals( + "[a, (this Collection), c]", + buildSet { + add("a") + add(this) + add("c") + }.toString() + ) + + assertEquals( + "{a=1, (this Map)=(this Map), c=3}", + linkedMapOf().apply { + put("a", "1") + put(this, this) + put("c", "3") + }.toString() + ) + assertEquals( + "{a=1, (this Map)=(this Map), c=3}", + buildMap { + put("a", "1") + put(this, this) + put("c", "3") + }.toString() + ) + } + @Test fun randomAccess() { assertStaticAndRuntimeTypeIs(arrayListOf(1)) assertTrue(listOf(1, 2) is RandomAccess, "Default read-only list implementation is RandomAccess")