'abbreviate' contract violations (#1490)

* add unit tests showing 'abbreviate' contract violations

* add unit tests showing mishandling of null abbrevMarker

* fix handling of null abbrevMarker

- treat null marker as empty string
- ensure offset and maxWidth are applied as usual (simple 'substring' won't cut it)

Author: ThrawnCA

src/main/java/org/apache/commons/lang3/StringUtils.java

@@ -352,13 +352,13 @@ public static String abbreviate(final String str, final String abbrevMarker, fin
      * @throws IllegalArgumentException if the width is too small.
      * @since 3.6
      */
-    public static String abbreviate(final String str, final String abbrevMarker, int offset, final int maxWidth) {
-        if (isNotEmpty(str) && EMPTY.equals(abbrevMarker) && maxWidth > 0) {
-            return substring(str, 0, maxWidth);
-        }
-        if (isAnyEmpty(str, abbrevMarker)) {
+    public static String abbreviate(final String str, String abbrevMarker, int offset, final int maxWidth) {
+        if (isEmpty(str)) {
             return str;
         }
+        if (abbrevMarker == null) {
+            abbrevMarker = EMPTY;
+        }
         final int abbrevMarkerLength = abbrevMarker.length();
         final int minAbbrevWidth = abbrevMarkerLength + 1;
         final int minAbbrevWidthOffset = abbrevMarkerLength + abbrevMarkerLength + 1;

src/test/java/org/apache/commons/lang3/StringUtilsAbbreviateTest.java

@@ -101,13 +101,16 @@ void testAbbreviate_StringIntInt() {
         assertAbbreviateWithOffset("...ijklmno", 15, 10);
         assertAbbreviateWithOffset("...ijklmno", 16, 10);
         assertAbbreviateWithOffset("...ijklmno", Integer.MAX_VALUE, 10);
+
+        // abbreviating a shorter string allows maxWidth < 7
+        assertEquals("...efg", StringUtils.abbreviate("abcdefg", 5, 6));
     }
 
     @Test
     void testAbbreviate_StringStringInt() {
         assertNull(StringUtils.abbreviate(null, null, 10));
         assertNull(StringUtils.abbreviate(null, "...", 10));
-        assertEquals("paranaguacu", StringUtils.abbreviate("paranaguacu", null, 10));
+        assertEquals("paranaguac", StringUtils.abbreviate("paranaguacu", null, 10));
         assertEquals("", StringUtils.abbreviate("", "...", 2));
         assertEquals("wai**", StringUtils.abbreviate("waiheke", "**", 5));
         assertEquals("And af,,,,", StringUtils.abbreviate("And after a long time, he finally met his son.", ",,,,", 10));
@@ -130,6 +133,12 @@ void testAbbreviate_StringStringIntInt() {
         assertNull(StringUtils.abbreviate(null, "...", 10, 12));
         assertEquals("", StringUtils.abbreviate("", null, 0, 10));
         assertEquals("", StringUtils.abbreviate("", "...", 2, 10));
+        assertEquals("abcdefg", StringUtils.abbreviate("abcdefg", null, 2, 10));
+        assertEquals("abcdefg", StringUtils.abbreviate("abcdefg", "", 2, 10));
+        assertEquals("abc", StringUtils.abbreviate("abcdefg", null, 0, 3));
+        assertEquals("cde", StringUtils.abbreviate("abcdefg", null, 2, 3));
+        assertEquals("abc", StringUtils.abbreviate("abcdefg", "", 0, 3));
+        assertEquals("cde", StringUtils.abbreviate("abcdefg", "", 2, 3));
         assertIllegalArgumentException(() -> StringUtils.abbreviate("abcdefghij", "::", 0, 2),
                 "StringUtils.abbreviate expecting IllegalArgumentException");
         assertIllegalArgumentException(() -> StringUtils.abbreviate("abcdefghij", "!!!", 5, 6),
@@ -157,6 +166,10 @@ void testAbbreviate_StringStringIntInt() {
         assertAbbreviateWithAbbrevMarkerAndOffset("999ijklmno", "999", 15, 10);
         assertAbbreviateWithAbbrevMarkerAndOffset("_ghijklmno", "_", 16, 10);
         assertAbbreviateWithAbbrevMarkerAndOffset("+ghijklmno", "+", Integer.MAX_VALUE, 10);
+
+        // abbreviating a shorter string allows maxWidth < abbrevMarker.length * 2 + 1
+        assertEquals("..de", StringUtils.abbreviate("abcde", "..", 4, 4));
+        assertEquals("....fg", StringUtils.abbreviate("abcdefg", "....", 5, 6));
     }
 
     // Fixed LANG-1463