From a1074918cf5fa00c885c8cc537381f1bb8866d90 Mon Sep 17 00:00:00 2001 From: Ivan Hetman Date: Mon, 3 Apr 2023 11:07:28 +0300 Subject: [PATCH] classlib: fix issues related to ArrayList, Arrays and Comparator --- .../classlib/java/util/TAbstractList.java | 6 +- .../teavm/classlib/java/util/TArrayList.java | 7 ++ .../org/teavm/classlib/java/util/TArrays.java | 28 +++--- .../classlib/java/util/TCollections.java | 25 ++--- .../teavm/classlib/java/util/TComparator.java | 23 ++++- .../classlib/java/util/ArrayListTest.java | 14 +++ .../teavm/classlib/java/util/ArraysTest.java | 2 + .../classlib/java/util/ComparatorTest.java | 92 +++++++++++++++++++ 8 files changed, 157 insertions(+), 40 deletions(-) create mode 100644 tests/src/test/java/org/teavm/classlib/java/util/ComparatorTest.java diff --git a/classlib/src/main/java/org/teavm/classlib/java/util/TAbstractList.java b/classlib/src/main/java/org/teavm/classlib/java/util/TAbstractList.java index db70ab302..fefad1d18 100644 --- a/classlib/src/main/java/org/teavm/classlib/java/util/TAbstractList.java +++ b/classlib/src/main/java/org/teavm/classlib/java/util/TAbstractList.java @@ -99,8 +99,7 @@ public abstract class TAbstractList extends TAbstractCollection implements public int indexOf(Object o) { int sz = size(); for (int i = 0; i < sz; ++i) { - Object e = get(i); - if (o == null ? e == null : o.equals(e)) { + if (TObjects.equals(o, get(i))) { return i; } } @@ -111,8 +110,7 @@ public abstract class TAbstractList extends TAbstractCollection implements public int lastIndexOf(Object o) { int sz = size(); for (int i = sz - 1; i >= 0; --i) { - Object e = get(i); - if (o == null ? e == null : o.equals(e)) { + if (TObjects.equals(o, get(i))) { return i; } } diff --git a/classlib/src/main/java/org/teavm/classlib/java/util/TArrayList.java b/classlib/src/main/java/org/teavm/classlib/java/util/TArrayList.java index 3f83e821b..d6efa22ea 100644 --- a/classlib/src/main/java/org/teavm/classlib/java/util/TArrayList.java +++ b/classlib/src/main/java/org/teavm/classlib/java/util/TArrayList.java @@ -129,6 +129,7 @@ public class TArrayList extends TAbstractList implements TCloneable, TSeri public void clear() { Arrays.fill(array, 0, size, null); size = 0; + ++modCount; } @Override @@ -210,4 +211,10 @@ public class TArrayList extends TAbstractList implements TCloneable, TSeri buffer.append(array[length] == this ? "(this Collection)" : array[length]); return buffer.append(']').toString(); } + + @Override + public void sort(TComparator comp) { + TArrays.sort(array, 0, size, comp); + modCount++; + } } diff --git a/classlib/src/main/java/org/teavm/classlib/java/util/TArrays.java b/classlib/src/main/java/org/teavm/classlib/java/util/TArrays.java index 99e23bf56..da7295ef8 100644 --- a/classlib/src/main/java/org/teavm/classlib/java/util/TArrays.java +++ b/classlib/src/main/java/org/teavm/classlib/java/util/TArrays.java @@ -912,27 +912,17 @@ public class TArrays extends TObject { } public static void sort(Object[] a) { - sort(a, new NaturalOrder()); + sort(a, TComparator.NaturalOrder.instance()); } public static void sort(Object[] a, int fromIndex, int toIndex) { - sort(a, fromIndex, toIndex, new NaturalOrder()); - } - - private static class NaturalOrder implements TComparator { - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Override public int compare(Object o1, Object o2) { - if (o1 != null) { - return ((TComparable) o1).compareTo(o2); - } else if (o2 != null) { - return ((TComparable) o2).compareTo(o1); - } else { - return 0; - } - } + sort(a, fromIndex, toIndex, TComparator.NaturalOrder.instance()); } public static void sort(T[] a, int fromIndex, int toIndex, TComparator c) { + if (c == null) { + c = TComparator.NaturalOrder.instance(); + } @SuppressWarnings("unchecked") T[] subarray = (T[]) new Object[toIndex - fromIndex]; for (int i = fromIndex; i < toIndex; ++i) { @@ -949,6 +939,9 @@ public class TArrays extends TObject { if (a.length == 0) { return; } + if (c == null) { + c = TComparator.NaturalOrder.instance(); + } Object[] first = a; Object[] second = new Object[a.length]; int chunkSize = 1; @@ -1225,7 +1218,7 @@ public class TArrays extends TObject { } public static int binarySearch(Object[] a, int fromIndex, int toIndex, Object key) { - return binarySearch(a, fromIndex, toIndex, key, new NaturalOrder()); + return binarySearch(a, fromIndex, toIndex, key, TComparator.NaturalOrder.instance()); } public static int binarySearch(T[] a, T key, TComparator c) { @@ -1233,6 +1226,9 @@ public class TArrays extends TObject { } public static int binarySearch(T[] a, int fromIndex, int toIndex, T key, TComparator c) { + if (c == null) { + c = TComparator.NaturalOrder.instance(); + } if (fromIndex > toIndex) { throw new TIllegalArgumentException(); } diff --git a/classlib/src/main/java/org/teavm/classlib/java/util/TCollections.java b/classlib/src/main/java/org/teavm/classlib/java/util/TCollections.java index df46f8074..637eb92a5 100644 --- a/classlib/src/main/java/org/teavm/classlib/java/util/TCollections.java +++ b/classlib/src/main/java/org/teavm/classlib/java/util/TCollections.java @@ -216,7 +216,7 @@ public class TCollections extends TObject { public static void sort(TList list, TComparator c) { if (c == null) { - c = naturalOrder; + c = TComparator.NaturalOrder.instance(); } @SuppressWarnings("unchecked") T[] array = (T[]) new Object[list.size()]; @@ -228,7 +228,7 @@ public class TCollections extends TObject { } public static > void sort(TList list) { - sort(list, naturalOrder); + sort(list, TComparator.NaturalOrder.instance()); } @SuppressWarnings("unchecked") @@ -244,20 +244,15 @@ public class TCollections extends TObject { } public static int binarySearch(TList> list, T key) { - return binarySearch(list, key, naturalOrder); + return binarySearch(list, key, TComparator.NaturalOrder.instance()); } - @SuppressWarnings("unchecked") - private static TComparator naturalOrder = (o1, o2) -> o1 != null - ? ((TComparable) o1).compareTo(o2) - : -((TComparable) o2).compareTo(o1); - public static int binarySearch(TList list, T key, TComparator c) { if (!(list instanceof TRandomAccess)) { list = new TArrayList<>(list); } if (c == null) { - c = naturalOrder; + c = TComparator.NaturalOrder.instance(); } int l = 0; int u = list.size() - 1; @@ -339,12 +334,12 @@ public class TCollections extends TObject { } public static > T min(TCollection coll) { - return min(coll, naturalOrder); + return min(coll, TComparator.NaturalOrder.instance()); } public static T min(TCollection coll, TComparator comp) { if (comp == null) { - comp = naturalOrder; + comp = TComparator.NaturalOrder.instance(); } TIterator iter = coll.iterator(); T min = iter.next(); @@ -358,12 +353,12 @@ public class TCollections extends TObject { } public static > T max(TCollection coll) { - return max(coll, naturalOrder); + return max(coll, TComparator.NaturalOrder.instance()); } public static T max(TCollection coll, TComparator comp) { if (comp == null) { - comp = naturalOrder; + comp = TComparator.NaturalOrder.instance(); } TIterator iter = coll.iterator(); T max = iter.next(); @@ -557,9 +552,7 @@ public class TCollections extends TObject { return (TComparator) reverseOrder; } - private static TComparator reverseOrder = (o1, o2) -> o1 != null - ? -((TComparable) o1).compareTo(o2) - : ((TComparable) o2).compareTo(o1); + private static TComparator reverseOrder = (o1, o2) -> ((TComparable) o2).compareTo(o1); public static TComparator reverseOrder(final TComparator cmp) { return (o1, o2) -> -cmp.compare(o1, o2); diff --git a/classlib/src/main/java/org/teavm/classlib/java/util/TComparator.java b/classlib/src/main/java/org/teavm/classlib/java/util/TComparator.java index f89a14a5f..c337d1cb5 100644 --- a/classlib/src/main/java/org/teavm/classlib/java/util/TComparator.java +++ b/classlib/src/main/java/org/teavm/classlib/java/util/TComparator.java @@ -56,7 +56,7 @@ public interface TComparator { if (r == 0) { U k = keyExtractor.apply(a); U m = keyExtractor.apply(b); - r = k != null ? k.compareTo(m) : -m.compareTo(k); + r = k.compareTo(m); } return r; }; @@ -102,16 +102,31 @@ public interface TComparator { return (a, b) -> { U k = keyExtractor.apply(a); U m = keyExtractor.apply(b); - return k != null ? k.compareTo(m) : -m.compareTo(k); + return k.compareTo(m); }; } + class NaturalOrder implements TComparator { + private static final TComparator INSTANCE = new NaturalOrder(); + + @Override + @SuppressWarnings("unchecked") + public int compare(Object o1, Object o2) { + return ((Comparable) o1).compareTo(o2); + } + + @SuppressWarnings("unchecked") + static TComparator instance() { + return (TComparator) INSTANCE; + } + } + static > TComparator naturalOrder() { - return (o1, o2) -> o1 != null ? o1.compareTo(o2) : o2.compareTo(o1); + return NaturalOrder.instance(); } static > TComparator reverseOrder() { - return (o1, o2) -> o2 != null ? o2.compareTo(o1) : o1.compareTo(o2); + return TCollections.reverseOrder(); } static TComparator nullsFirst(TComparator comparator) { diff --git a/tests/src/test/java/org/teavm/classlib/java/util/ArrayListTest.java b/tests/src/test/java/org/teavm/classlib/java/util/ArrayListTest.java index 6c87be8b6..e17cb65ee 100644 --- a/tests/src/test/java/org/teavm/classlib/java/util/ArrayListTest.java +++ b/tests/src/test/java/org/teavm/classlib/java/util/ArrayListTest.java @@ -18,6 +18,7 @@ package org.teavm.classlib.java.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import java.util.ArrayList; +import java.util.Comparator; import java.util.ConcurrentModificationException; import java.util.List; import org.junit.Test; @@ -184,4 +185,17 @@ public class ArrayListTest { assertEquals("[(this Collection), A]", list.toString()); assertEquals("[]", new ArrayList().toString()); } + + @Test + public void testSort() { + int size = 10; + List list = new ArrayList<>(); + for (int i = 0; i < size; i++) { + list.add(size - 1 - i); + } + list.sort(Comparator.naturalOrder()); + for (int i = 0; i < size; i++) { + assertEquals(i, list.get(i).intValue()); + } + } } diff --git a/tests/src/test/java/org/teavm/classlib/java/util/ArraysTest.java b/tests/src/test/java/org/teavm/classlib/java/util/ArraysTest.java index 864b42b29..dc1fc2348 100644 --- a/tests/src/test/java/org/teavm/classlib/java/util/ArraysTest.java +++ b/tests/src/test/java/org/teavm/classlib/java/util/ArraysTest.java @@ -37,6 +37,7 @@ public class ArraysTest { assertEquals(Integer.valueOf(5), array[3]); assertEquals(Integer.valueOf(6), array[4]); assertEquals(Integer.valueOf(7), array[5]); + Arrays.sort(array, null); // NPE check } @Test @@ -50,6 +51,7 @@ public class ArraysTest { assertEquals(-3, Arrays.binarySearch(array, 5)); assertEquals(-8, Arrays.binarySearch(array, 15)); assertEquals(-9, Arrays.binarySearch(array, 17)); + assertEquals(3, Arrays.binarySearch(array, 8, null)); // NPE check } @Test diff --git a/tests/src/test/java/org/teavm/classlib/java/util/ComparatorTest.java b/tests/src/test/java/org/teavm/classlib/java/util/ComparatorTest.java new file mode 100644 index 000000000..76fd8bc7a --- /dev/null +++ b/tests/src/test/java/org/teavm/classlib/java/util/ComparatorTest.java @@ -0,0 +1,92 @@ +/* + * Copyright 2023 ihromant. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.teavm.classlib.java.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.util.Comparator; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.teavm.junit.TeaVMTestRunner; +import org.teavm.junit.WholeClassCompilation; + +@RunWith(TeaVMTestRunner.class) +@WholeClassCompilation +public class ComparatorTest { + @Test + public void naturalReverseOrder() { + Integer i = 2; + Integer j = 3; + Comparator comp = Comparator.naturalOrder(); + assertTrue(comp.compare(i, j) < 0); + assertEquals(0, comp.compare(i, i)); + assertTrue(comp.compare(j, i) > 0); + Comparator rev = Comparator.reverseOrder(); + assertTrue(rev.compare(i, j) > 0); + assertEquals(0, rev.compare(i, i)); + assertTrue(rev.compare(j, i) < 0); + try { + comp.compare(i, null); + fail("Expected NPE for comparing with null"); + } catch (NullPointerException e) { + // OK + } + try { + comp.compare(null, i); + fail("Expected NPE for comparing with null"); + } catch (NullPointerException e) { + // OK + } + } + + @Test + public void testComparing() { + Point a = new Point(1, 1); + Point b = new Point(1, 2); + Point c = new Point(2, 1); + Point d = new Point(1, null); + Comparator comp = Comparator.comparing(Point::getX).thenComparing(Point::getY); + assertTrue(comp.compare(a, c) < 1); + assertTrue(comp.compare(a, b) < 1); + assertTrue(comp.compare(b, c) < 1); + assertEquals(0, comp.compare(a, a)); + try { + comp.compare(a, d); + fail("Expected NPE for comparing null fields"); + } catch (NullPointerException e) { + // OK + } + } + + private static class Point { + private final Integer x; + private final Integer y; + + private Point(Integer x, Integer y) { + this.x = x; + this.y = y; + } + + private int getX() { + return x; + } + + private int getY() { + return y; + } + } +}