Fix iterator.next() behavior in some places

Author: Abduqodiri Qurbonzoda

kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableSet/PersistentHashSetIterator.kt

@@ -70,7 +70,8 @@ internal open class PersistentHashSetIterator<E>(node: TrieNode<E>) : Iterator<E
     }
 
     override fun next(): E {
-        assert(hasNext())
+        if (!hasNext)
+            throw NoSuchElementException()
 
         val result = path[pathLastIndex].nextElement()
         ensureNextElementIsReady()

kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/persistentOrderedSet/PersistentOrderedSetIterator.kt

@@ -25,7 +25,9 @@ internal open class PersistentOrderedSetIterator<E>(private var nextElement: E?,
     }
 
     override fun next(): E {
-        assert(hasNext())
+        if (!hasNext())
+            throw NoSuchElementException()
+
         val result = nextElement as E
         index++
         nextElement = map[result]!!.next

kotlinx-collections-immutable/tests/src/test/kotlin/kotlinx.collections.immutable/contractTests/immutableSet/ImmutableSetTest.kt

@@ -17,76 +17,64 @@
 package kotlinx.collections.immutable.contractTests.immutableSet
 
 import kotlinx.collections.immutable.*
+import kotlinx.collections.immutable.contractTests.compare
+import kotlinx.collections.immutable.contractTests.setBehavior
 import org.junit.Test
 import kotlin.test.*
 
-class ImmutableSetTest : ImmutableSetTestBase() {
-    override fun <T> immutableSetOf(vararg elements: T) = persistentSetOf(*elements)
-}
-
 class ImmutableHashSetTest : ImmutableSetTestBase() {
     override fun <T> immutableSetOf(vararg elements: T) = persistentHashSetOf(*elements)
-
-    override fun empty() {
-        val empty1 = immutableSetOf<Int>()
-        val empty2 = immutableSetOf<String>()
-        assertEquals<ImmutableSet<Any>>(empty1, empty2)
-        assertEquals<Set<Any>>(setOf(), empty1)
-//        assertTrue(empty1 === empty2)   // fails to implement this property
-    }
-
-    override fun noOperation() {
-        // immutableSetOf<Int>().testNoOperation({ clear() }, { clear() })  // fails to implement this property
-
-        val list = "abcxyz12".toList().toPersistentSet()
-        with(list) {
-            testNoOperation({ add('a') }, { add('a') })
-            testNoOperation({ addAll(listOf('a', 'b')) }, { addAll(listOf('a', 'b')) })
-            testNoOperation({ remove('d') }, { remove('d') })
-            testNoOperation({ removeAll(listOf('d', 'e')) }, { removeAll(listOf('d', 'e')) })
-            testNoOperation({ removeAll { it.isUpperCase() } }, { removeAll { it.isUpperCase() } })
-        }
-    }
+}
+class ImmutableOrderedSetTest : ImmutableSetTestBase() {
+    override fun <T> immutableSetOf(vararg elements: T) = persistentSetOf(*elements)
+    override fun <T> compareSets(expected: Set<T>, actual: Set<T>) = compare(expected, actual) { setBehavior(ordered = true) }
 }
 
 abstract class ImmutableSetTestBase {
 
     abstract fun <T> immutableSetOf(vararg elements: T): PersistentSet<T>
+    fun <T> immutableSetOf(elements: Collection<T>) = immutableSetOf<T>() + elements
+
+    open fun <T> compareSets(expected: Set<T>, actual: Set<T>) = compareSetsUnordered(expected, actual)
+    fun <T> compareSetsUnordered(expected: Set<T>, actual: Set<T>) = compare(expected, actual) { setBehavior(ordered = false) }
 
     @Test open fun empty() {
         val empty1 = immutableSetOf<Int>()
         val empty2 = immutableSetOf<String>()
         assertEquals<ImmutableSet<Any>>(empty1, empty2)
         assertEquals<Set<Any>>(setOf(), empty1)
         assertTrue(empty1 === empty2)
+
+        compareSets(emptySet(), empty1)
     }
 
     @Test fun ofElements() {
         val set0 = setOf("a", "d", 1, null)
         val set1 = immutableSetOf("a", "d", 1, null)
         val set2 = immutableSetOf("a", "d", 1, null)
 
-        assertEquals(set0, set1)
-        assertEquals(set1, set2)
+        compareSets(set0, set1)
+        compareSets(set1, set2)
     }
 
     @Test fun toImmutable() {
         val original = setOf("a", "bar", "cat", null)
+        val immOriginal = immutableSetOf(original)
+        compareSets(original, immOriginal)
 
-        val set = original.toMutableSet() // copy
-        var immSet = set.toPersistentSet()
+        val hashSet = HashSet(original) // copy
+        var immSet = immutableSetOf(hashSet)
         val immSet2 = immSet.toImmutableSet()
         assertTrue(immSet2 === immSet)
 
-        assertEquals<Set<*>>(set, immSet) // problem
-        assertEquals(set.toString(), immSet.toString())
-        assertEquals(set.hashCode(), immSet.hashCode())
+        compareSetsUnordered(original, immSet)
+        compareSetsUnordered(hashSet, immSet)
 
-        set.remove("a")
-        assertNotEquals<Set<*>>(set, immSet)
+        hashSet.remove("a")
+        assertNotEquals<Set<*>>(hashSet, immSet)
 
         immSet = immSet.remove("a")
-        assertEquals<Set<*>>(set, immSet) // problem
+        compareSetsUnordered(hashSet, immSet)
     }
 
     @Test fun addElements() {
@@ -96,14 +84,14 @@ abstract class ImmutableSetTestBase {
         set = set + "y"
         set += "z"
         set += arrayOf("1", "2").asIterable()
-        assertEquals("xyz12".map { it.toString() }.toSet(), set)
+        compareSets("xyz12".map { it.toString() }.toSet(), set)
     }
 
 
     @Test fun removeElements() {
-        val set = "abcxyz12".toList().toPersistentSet()
+        val set = immutableSetOf("abcxyz12".toList())
         fun expectSet(content: String, set: ImmutableSet<Char>) {
-            assertEquals(content.toSet(), set)
+            compareSets(content.toSet(), set)
         }
 
         expectSet("abcyz12", set.remove('x'))
@@ -112,19 +100,21 @@ abstract class ImmutableSetTestBase {
         expectSet("abcy12", set - setOf('x', 'z'))
         expectSet("abcxyz", set.removeAll { it.isDigit() })
 
-        assertEquals(emptySet<Char>(), set - set)
-        assertEquals(emptySet<Char>(), set.clear())
+        compareSets(emptySet(), set - set)
+        compareSets(emptySet(), set.clear())
     }
 
     @Test fun builder() {
         val builder = immutableSetOf<Char>().builder()
         "abcxaxyz12".toCollection(builder)
         val set = builder.build()
-        assertEquals<Set<*>>(set, builder)
+        compareSets(set, builder)
         assertTrue(set === builder.build(), "Building the same set without modifications")
 
         val set2 = builder.toImmutableSet()
         assertTrue(set2 === set, "toImmutable calls build()")
+        val set3 = builder.toPersistentSet()
+        assertTrue(set3 === set, "toPersistent calls build()")
 
         with(set) {
             testMutation { add('K') }
@@ -139,22 +129,20 @@ abstract class ImmutableSetTestBase {
     }
 
     fun <T> PersistentSet<T>.testMutation(operation: MutableSet<T>.() -> Unit) {
-        val mutable = this.toMutableSet()
+        val mutable = HashSet(this) as MutableSet<T>
         val builder = this.builder()
 
         operation(mutable)
         operation(builder)
 
-        assertEquals(mutable, builder)
-        assertEquals<Set<*>>(mutable, builder.build())
+        compareSetsUnordered(mutable, builder)
+        compareSetsUnordered(mutable, builder.build())
     }
 
-
-
     @Test open fun noOperation() {
         immutableSetOf<Int>().testNoOperation({ clear() }, { clear() })
 
-        val set = "abcxyz12".asIterable().toPersistentSet()
+        val set = immutableSetOf("abcxyz12".toList())
         with(set) {
             testNoOperation({ add('a') }, { add('a') })
             testNoOperation({ addAll(listOf('a', 'b')) }, { addAll(listOf('a', 'b')) })