From 2ccc58e34472cbc0c4b9163bbd7f6b22dfda0089 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sun, 15 Nov 2020 23:37:54 +0100 Subject: [PATCH] [LANG-1615] - ArrayUtils contains && indexOf fails to handle Float.NaN (#651) * LANG-1615 - ArrayUtils contains && indexOf fails to handle Float.NaN * Change var name * Fix checkstyle spaces black * Fix checkstyle spaces black --- .../org/apache/commons/lang3/ArrayUtils.java | 8 +++++--- .../org/apache/commons/lang3/ArrayUtilsTest.java | 16 ++++++++++++++++ .../commons/lang3/reflect/FieldUtilsTest.java | 2 +- .../commons/lang3/reflect/TypeUtilsTest.java | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/ArrayUtils.java b/src/main/java/org/apache/commons/lang3/ArrayUtils.java index cee5f3d61..650e7dfbe 100644 --- a/src/main/java/org/apache/commons/lang3/ArrayUtils.java +++ b/src/main/java/org/apache/commons/lang3/ArrayUtils.java @@ -2518,8 +2518,10 @@ public class ArrayUtils { if (startIndex < 0) { startIndex = 0; } + final boolean searchNaN = Float.isNaN(valueToFind); for (int i = startIndex; i < array.length; i++) { - if (valueToFind == array[i]) { + final float element = array[i]; + if (valueToFind == element || (searchNaN && Float.isNaN(element))) { return i; } } @@ -7877,7 +7879,7 @@ public static int indexOf(final int[] array, final int valueToFind, int startInd * * @param the array type. * @param array the array to sort. - * @param comparator the comparator to determine the order of the array. + * @param comparator the comparator to determine the order of the array. * A {@code null} value uses the elements' {@link Comparable natural ordering}. * @return the given array. * @see Arrays#sort(Object[]) @@ -7887,7 +7889,7 @@ public static int indexOf(final int[] array, final int valueToFind, int startInd Arrays.sort(array, comparator); return array; } - + /** *

Produces a new {@code boolean} array containing the elements * between the start and end indices. diff --git a/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java b/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java index 99ea44d7b..38a5f9ffa 100644 --- a/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java @@ -309,6 +309,14 @@ public class ArrayUtilsTest { assertFalse(ArrayUtils.contains(array, (float) 99)); } + @Test + public void testContainsFloatNaN() { + float[] array = new float[] { Float.NEGATIVE_INFINITY, Float.NaN, Float.POSITIVE_INFINITY }; + assertTrue(ArrayUtils.contains(array, Float.POSITIVE_INFINITY)); + assertTrue(ArrayUtils.contains(array, Float.NEGATIVE_INFINITY)); + assertTrue(ArrayUtils.contains(array, Float.NaN)); + } + @Test public void testContainsInt() { int[] array = null; @@ -1124,6 +1132,14 @@ public class ArrayUtilsTest { assertEquals(-1, ArrayUtils.indexOf(array, (float) 99)); } + @Test + public void testIndexOfFloatNaN() { + float[] array = new float[] { Float.NEGATIVE_INFINITY, Float.NaN, Float.POSITIVE_INFINITY, Float.NaN }; + assertEquals(0, ArrayUtils.indexOf(array, Float.NEGATIVE_INFINITY)); + assertEquals(1, ArrayUtils.indexOf(array, Float.NaN)); + assertEquals(2, ArrayUtils.indexOf(array, Float.POSITIVE_INFINITY)); + } + @SuppressWarnings("cast") @Test public void testIndexOfFloatWithStartIndex() { diff --git a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java index 15fe8c32c..0bcacea67 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java @@ -210,7 +210,7 @@ public class FieldUtilsTest { @Test public void testGetFieldsWithAnnotation() throws NoSuchFieldException { assertArrayEquals(new Field[0], FieldUtils.getFieldsWithAnnotation(Object.class, Annotated.class)); - final Field[] annotatedFields = sort(new Field[] { + final Field[] annotatedFields = sort(new Field[] { FieldUtilsTest.class.getDeclaredField("publicChild"), FieldUtilsTest.class.getDeclaredField("privatelyShadowedChild") }); assertArrayEquals(annotatedFields, diff --git a/src/test/java/org/apache/commons/lang3/reflect/TypeUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/TypeUtilsTest.java index a3711bc53..8cdce0b89 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/TypeUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/reflect/TypeUtilsTest.java @@ -790,7 +790,7 @@ public class TypeUtilsTest { assertFalse(paramType.getClass().isAssignableFrom(WildcardType.class)); WildcardType testType = TypeUtils.WILDCARD_ALL; - // TODO This test returns true unlike the test above. + // TODO This test returns true unlike the test above. // Is this a bug in this test or in the main code? assertFalse(TypeUtils.isAssignable(paramType, testType), () -> String.format("TypeUtils.isAssignable(%s, %s)", paramType, testType));