From 94ec5a11122b2e60fbffcc35373c978c839bf8ae Mon Sep 17 00:00:00 2001 From: Loic Guibert Date: Mon, 5 Oct 2015 17:00:50 +0400 Subject: [PATCH 1/3] LANG-1171 Add null safe compare methods in StringUtils : - StringUtils.compare(String str1, String str2); - StringUtils.compare(String str1, String str2, boolean nullIsLess); - StringUtils.compareIgnoreCase(String str1, String str2); - StringUtils.compareIgnoreCase(String str1, String str2, boolean nullIsLess); --- .../org/apache/commons/lang3/StringUtils.java | 176 +++++++++++++++++- .../lang3/StringUtilsEqualsIndexOfTest.java | 69 +++++++ 2 files changed, 244 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/lang3/StringUtils.java b/src/main/java/org/apache/commons/lang3/StringUtils.java index 3f0314f6a..cb3912038 100644 --- a/src/main/java/org/apache/commons/lang3/StringUtils.java +++ b/src/main/java/org/apache/commons/lang3/StringUtils.java @@ -35,7 +35,7 @@ * - checks if a String contains text *
  • Trim/Strip * - removes leading and trailing whitespace
    • - *
  • Equals + *
  • Equals/Compare * - compares two strings null-safe
    • *
  • startsWith * - check if a String starts with a prefix null-safe
    • @@ -830,6 +830,180 @@ public static boolean equalsIgnoreCase(final CharSequence str1, final CharSequen } } + // Compare + //----------------------------------------------------------------------- + /** + *

    Compare two Strings lexicographically, as per {@link String#compareTo(String)}, returning :

    + * + * + *

    This is a {@code null} safe version of :

    + * 
    str1.compareTo(str2)
    + * + *

    {@code null} value is considered less than non-{@code null} value. + * Two {@code null} references are considered equal.

    + * + * 
    +     * StringUtils.compare(null, null)   = 0
+     * StringUtils.compare(null , "a")   < 0
+     * StringUtils.compare("a", null)    > 0
+     * StringUtils.compare("abc", "abc") = 0
+     * StringUtils.compare("a", "b")     < 0
+     * StringUtils.compare("b", "a")     > 0
+     * StringUtils.compare("a", "B")     > 0
+     * StringUtils.compare("ab", "abc")  < 0
+     *
    + * + * @see #compare(String, String, boolean) + * @see String#compareTo(String) + * @param str1 the String to compare from + * @param str2 the String to compare to + * @return < 0, 0, > 0, if {@code str1} is respectively less, equal ou greater than {@code str2} + */ + public static int compare(final String str1, final String str2) { + return compare(str1, str2, true); + } + + /** + *

    Compare two Strings lexicographically, as per {@link String#compareTo(String)}, returning :

    + * + * + *

    This is a {@code null} safe version of :

    + * 
    str1.compareTo(str2)
    + * + *

    {@code null} inputs are handled according to the {@code nullIsLess} parameter. + * Two {@code null} references are considered equal.

    + * + * 
    +     * StringUtils.compare(null, null, *)     = 0
+     * StringUtils.compare(null , "a", true)  < 0
+     * StringUtils.compare(null , "a", false) > 0
+     * StringUtils.compare("a", null, true)   > 0
+     * StringUtils.compare("a", null, false)  < 0
+     * StringUtils.compare("abc", "abc", *)   = 0
+     * StringUtils.compare("a", "b", *)       < 0
+     * StringUtils.compare("b", "a", *)       > 0
+     * StringUtils.compare("a", "B", *)       > 0
+     * StringUtils.compare("ab", "abc", *)    < 0
+     *
    + * + * @see String#compareTo(String) + * @param str1 the String to compare from + * @param str2 the String to compare to + * @param nullIsLess whether consider {@code null} value less than non-{@code null} value + * @return < 0, 0, > 0, if {@code str1} is respectively less, equal ou greater than {@code str2} + */ + public static int compare(final String str1, final String str2, final boolean nullIsLess) { + if (str1 == str2) { + return 0; + } + if (str1 == null) { + return nullIsLess ? -1 : 1; + } + if (str2 == null) { + return nullIsLess ? 1 : - 1; + } + return str1.compareTo(str2); + } + + /** + *

    Compare two Strings lexicographically, ignoring case differences, + * as per {@link String#compareToIgnoreCase(String)}, returning :

    + * + * + *

    This is a {@code null} safe version of :

    + * 
    str1.compareToIgnoreCase(str2)
    + * + *

    {@code null} value is considered less than non-{@code null} value. + * Two {@code null} references are considered equal. + * Comparison is case insensitive.

    + * + * 
    +     * StringUtils.compareIgnoreCase(null, null)   = 0
+     * StringUtils.compareIgnoreCase(null , "a")   < 0
+     * StringUtils.compareIgnoreCase("a", null)    > 0
+     * StringUtils.compareIgnoreCase("abc", "abc") = 0
+     * StringUtils.compareIgnoreCase("abc", "ABC") = 0
+     * StringUtils.compareIgnoreCase("a", "b")     < 0
+     * StringUtils.compareIgnoreCase("b", "a")     > 0
+     * StringUtils.compareIgnoreCase("a", "B")     < 0
+     * StringUtils.compareIgnoreCase("A", "b")     < 0
+     * StringUtils.compareIgnoreCase("ab", "ABC")  < 0
+     *
    + * + * @see #compareIgnoreCase(String, String, boolean) + * @see String#compareToIgnoreCase(String) + * @param str1 the String to compare from + * @param str2 the String to compare to + * @return < 0, 0, > 0, if {@code str1} is respectively less, equal ou greater than {@code str2}, + * ignoring case differences. + */ + public static int compareIgnoreCase(final String str1, final String str2) { + return compareIgnoreCase(str1, str2, true); + } + + /** + *

    Compare two Strings lexicographically, ignoring case differences, + * as per {@link String#compareToIgnoreCase(String)}, returning :

    + * + * + *

    This is a {@code null} safe version of :

    + * 
    str1.compareToIgnoreCase(str2)
    + * + *

    {@code null} inputs are handled according to the {@code nullIsLess} parameter. + * Two {@code null} references are considered equal. + * Comparison is case insensitive.

    + * + * 
    +     * StringUtils.compareIgnoreCase(null, null, *)     = 0
+     * StringUtils.compareIgnoreCase(null , "a", true)  < 0
+     * StringUtils.compareIgnoreCase(null , "a", false) > 0
+     * StringUtils.compareIgnoreCase("a", null, true)   > 0
+     * StringUtils.compareIgnoreCase("a", null, false)  < 0
+     * StringUtils.compareIgnoreCase("abc", "abc", *)   = 0
+     * StringUtils.compareIgnoreCase("abc", "ABC", *)   = 0
+     * StringUtils.compareIgnoreCase("a", "b", *)       < 0
+     * StringUtils.compareIgnoreCase("b", "a", *)       > 0
+     * StringUtils.compareIgnoreCase("a", "B", *)       < 0
+     * StringUtils.compareIgnoreCase("A", "b", *)       < 0
+     * StringUtils.compareIgnoreCase("ab", "abc", *)    < 0
+     *
    + * + * @see String#compareToIgnoreCase(String) + * @param str1 the String to compare from + * @param str2 the String to compare to + * @param nullIsLess whether consider {@code null} value less than non-{@code null} value + * @return < 0, 0, > 0, if {@code str1} is respectively less, equal ou greater than {@code str2}, + * ignoring case differences. + */ + public static int compareIgnoreCase(final String str1, final String str2, final boolean nullIsLess) { + if (str1 == str2) { + return 0; + } + if (str1 == null) { + return nullIsLess ? -1 : 1; + } + if (str2 == null) { + return nullIsLess ? 1 : - 1; + } + return str1.compareToIgnoreCase(str2); + } + // IndexOf //----------------------------------------------------------------------- /** diff --git a/src/test/java/org/apache/commons/lang3/StringUtilsEqualsIndexOfTest.java b/src/test/java/org/apache/commons/lang3/StringUtilsEqualsIndexOfTest.java index d2b6418bf..b45a5f625 100644 --- a/src/test/java/org/apache/commons/lang3/StringUtilsEqualsIndexOfTest.java +++ b/src/test/java/org/apache/commons/lang3/StringUtilsEqualsIndexOfTest.java @@ -576,6 +576,75 @@ public void testEqualsIgnoreCase() { assertFalse(StringUtils.equalsIgnoreCase("abcd","abcd ")); } + //----------------------------------------------------------------------- + @Test + public void testCompare_StringString() { + assertTrue(StringUtils.compare(null, null) == 0); + assertTrue(StringUtils.compare(null, "a") < 0); + assertTrue(StringUtils.compare("a", null) > 0); + assertTrue(StringUtils.compare("abc", "abc") == 0); + assertTrue(StringUtils.compare("a", "b") < 0); + assertTrue(StringUtils.compare("b", "a") > 0); + assertTrue(StringUtils.compare("a", "B") > 0); + assertTrue(StringUtils.compare("abc", "abd") < 0); + assertTrue(StringUtils.compare("ab", "abc") < 0); + assertTrue(StringUtils.compare("ab", "ab ") < 0); + assertTrue(StringUtils.compare("abc", "ab ") > 0); + } + + @Test + public void testCompare_StringStringBoolean() { + assertTrue(StringUtils.compare(null, null, false) == 0); + assertTrue(StringUtils.compare(null, "a", true) < 0); + assertTrue(StringUtils.compare(null, "a", false) > 0); + assertTrue(StringUtils.compare("a", null, true) > 0); + assertTrue(StringUtils.compare("a", null, false) < 0); + assertTrue(StringUtils.compare("abc", "abc", false) == 0); + assertTrue(StringUtils.compare("a", "b", false) < 0); + assertTrue(StringUtils.compare("b", "a", false) > 0); + assertTrue(StringUtils.compare("a", "B", false) > 0); + assertTrue(StringUtils.compare("abc", "abd", false) < 0); + assertTrue(StringUtils.compare("ab", "abc", false) < 0); + assertTrue(StringUtils.compare("ab", "ab ", false) < 0); + assertTrue(StringUtils.compare("abc", "ab ", false) > 0); + } + + @Test + public void testCompareIgnoreCase_StringString() { + assertTrue(StringUtils.compareIgnoreCase(null, null) == 0); + assertTrue(StringUtils.compareIgnoreCase(null, "a") < 0); + assertTrue(StringUtils.compareIgnoreCase("a", null) > 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "abc") == 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "ABC") == 0); + assertTrue(StringUtils.compareIgnoreCase("a", "b") < 0); + assertTrue(StringUtils.compareIgnoreCase("b", "a") > 0); + assertTrue(StringUtils.compareIgnoreCase("a", "B") < 0); + assertTrue(StringUtils.compareIgnoreCase("A", "b") < 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "ABD") < 0); + assertTrue(StringUtils.compareIgnoreCase("ab", "ABC") < 0); + assertTrue(StringUtils.compareIgnoreCase("ab", "AB ") < 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "AB ") > 0); + } + + @Test + public void testCompareIgnoreCase_StringStringBoolean() { + assertTrue(StringUtils.compareIgnoreCase(null, null, false) == 0); + assertTrue(StringUtils.compareIgnoreCase(null, "a", true) < 0); + assertTrue(StringUtils.compareIgnoreCase(null, "a", false) > 0); + assertTrue(StringUtils.compareIgnoreCase("a", null, true) > 0); + assertTrue(StringUtils.compareIgnoreCase("a", null, false) < 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "abc", false) == 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "ABC", false) == 0); + assertTrue(StringUtils.compareIgnoreCase("a", "b", false) < 0); + assertTrue(StringUtils.compareIgnoreCase("b", "a", false) > 0); + assertTrue(StringUtils.compareIgnoreCase("a", "B", false) < 0); + assertTrue(StringUtils.compareIgnoreCase("A", "b", false) < 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "ABD", false) < 0); + assertTrue(StringUtils.compareIgnoreCase("ab", "ABC", false) < 0); + assertTrue(StringUtils.compareIgnoreCase("ab", "AB ", false) < 0); + assertTrue(StringUtils.compareIgnoreCase("abc", "AB ", false) > 0); + } + //----------------------------------------------------------------------- @Test public void testIndexOf_char() { From 131917a0d3303ca2c38fd1d6765b9bed2c23ff89 Mon Sep 17 00:00:00 2001 From: Loic Guibert Date: Tue, 20 Oct 2015 18:50:13 +0400 Subject: [PATCH 2/3] LANG-1171 Exclude methods from StringUtilsTest.testStringUtilsCharSequenceContract() : - StringUtils.compare(String str1, String str2); - StringUtils.compare(String str1, String str2, boolean nullIsLess); - StringUtils.compareIgnoreCase(String str1, String str2); - StringUtils.compareIgnoreCase(String str1, String str2, boolean nullIsLess); --- .../apache/commons/lang3/StringUtilsTest.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/apache/commons/lang3/StringUtilsTest.java b/src/test/java/org/apache/commons/lang3/StringUtilsTest.java index 5cc665a15..a57a7f5ca 100644 --- a/src/test/java/org/apache/commons/lang3/StringUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/StringUtilsTest.java @@ -2373,22 +2373,39 @@ public void testLANG666() { @Test public void testStringUtilsCharSequenceContract() { final Class c = StringUtils.class; + // Methods that are expressly excluded from testStringUtilsCharSequenceContract() + final String[] excludeMethods = { + "public static int org.apache.commons.lang3.StringUtils.compare(java.lang.String,java.lang.String)", + "public static int org.apache.commons.lang3.StringUtils.compare(java.lang.String,java.lang.String,boolean)", + "public static int org.apache.commons.lang3.StringUtils.compareIgnoreCase(java.lang.String,java.lang.String)", + "public static int org.apache.commons.lang3.StringUtils.compareIgnoreCase(java.lang.String,java.lang.String,boolean)" + }; final Method[] methods = c.getMethods(); + for (final Method m : methods) { + String methodStr = m.toString(); if (m.getReturnType() == String.class || m.getReturnType() == String[].class) { // Assume this is mutable and ensure the first parameter is not CharSequence. // It may be String or it may be something else (String[], Object, Object[]) so // don't actively test for that. final Class[] params = m.getParameterTypes(); if (params.length > 0 && (params[0] == CharSequence.class || params[0] == CharSequence[].class)) { - fail("The method " + m + " appears to be mutable in spirit and therefore must not accept a CharSequence"); + if (ArrayUtils.contains(excludeMethods, methodStr)) { + System.out.println("The mutable method \"" + methodStr + "\" is expressly excluded from testStringUtilsCharSequenceContract()"); + } else { + fail("The method \"" + methodStr + "\" appears to be mutable in spirit and therefore must not accept a CharSequence"); + } } } else { // Assume this is immutable in spirit and ensure the first parameter is not String. // As above, it may be something other than CharSequence. final Class[] params = m.getParameterTypes(); if (params.length > 0 && (params[0] == String.class || params[0] == String[].class)) { - fail("The method " + m + " appears to be immutable in spirit and therefore must not accept a String"); + if (ArrayUtils.contains(excludeMethods, methodStr)) { + System.out.println("The immutable method \"" + methodStr + "\" is expressly excluded from testStringUtilsCharSequenceContract()"); + } else { + fail("The method \"" + methodStr + "\" appears to be immutable in spirit and therefore must not accept a String"); + } } } } From 51512905c31ff19b91fc67b2cb3ddb5b1fa0fd51 Mon Sep 17 00:00:00 2001 From: Loic Guibert Date: Wed, 21 Oct 2015 15:15:06 +0400 Subject: [PATCH 3/3] LANG-1171 Remove log of excluded methods in StringUtilsTest.testStringUtilsCharSequenceContract() --- .../java/org/apache/commons/lang3/StringUtilsTest.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/apache/commons/lang3/StringUtilsTest.java b/src/test/java/org/apache/commons/lang3/StringUtilsTest.java index a57a7f5ca..c49ee6a07 100644 --- a/src/test/java/org/apache/commons/lang3/StringUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/StringUtilsTest.java @@ -2390,9 +2390,7 @@ public void testStringUtilsCharSequenceContract() { // don't actively test for that. final Class[] params = m.getParameterTypes(); if (params.length > 0 && (params[0] == CharSequence.class || params[0] == CharSequence[].class)) { - if (ArrayUtils.contains(excludeMethods, methodStr)) { - System.out.println("The mutable method \"" + methodStr + "\" is expressly excluded from testStringUtilsCharSequenceContract()"); - } else { + if (!ArrayUtils.contains(excludeMethods, methodStr)) { fail("The method \"" + methodStr + "\" appears to be mutable in spirit and therefore must not accept a CharSequence"); } } @@ -2401,9 +2399,7 @@ public void testStringUtilsCharSequenceContract() { // As above, it may be something other than CharSequence. final Class[] params = m.getParameterTypes(); if (params.length > 0 && (params[0] == String.class || params[0] == String[].class)) { - if (ArrayUtils.contains(excludeMethods, methodStr)) { - System.out.println("The immutable method \"" + methodStr + "\" is expressly excluded from testStringUtilsCharSequenceContract()"); - } else { + if (!ArrayUtils.contains(excludeMethods, methodStr)) { fail("The method \"" + methodStr + "\" appears to be immutable in spirit and therefore must not accept a String"); } }