GROOVY-7585: ObjectRange stricter about input compatibility - Strings must be 
of same lengths (lexicographic ordering does not permit moving from one length 
to another) - do not allow Combinations of Strings and numbers as ranges (after 
normalization)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/320500cd
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/320500cd
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/320500cd

Branch: refs/heads/master
Commit: 320500cd90a2de90a444fcc59b84b343679dbdd9
Parents: 849af36
Author: Thibault Kruse <thibault.kr...@gmx.de>
Authored: Thu Oct 15 09:38:42 2015 +0200
Committer: paulk <pa...@asert.com.au>
Committed: Tue Jun 28 17:05:26 2016 +1000

----------------------------------------------------------------------
 src/main/groovy/lang/ObjectRange.java     |  42 +++++--
 src/test/groovy/lang/ObjectRangeTest.java | 164 ++++++++++++++++++++++++-
 2 files changed, 193 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/320500cd/src/main/groovy/lang/ObjectRange.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/ObjectRange.java 
b/src/main/groovy/lang/ObjectRange.java
index d328631..f196c93 100644
--- a/src/main/groovy/lang/ObjectRange.java
+++ b/src/main/groovy/lang/ObjectRange.java
@@ -129,8 +129,25 @@ public class ObjectRange extends AbstractList implements 
Range {
             this.from = smaller;
             this.to = larger;
         } else {
-            this.from = normaliseStringType(smaller);
-            this.to = normaliseStringType(larger);
+            // Convenience hack: try convert single-char strings to ints
+            Comparable tempfrom = normaliseStringType(smaller);
+            Comparable tempto = normaliseStringType(larger);
+            if (tempfrom.getClass() != tempto.getClass()) {
+                // if convenience hack did not make classes match,
+                // and thus "from" cannot be advanced over "to", throw 
exception.
+                // Note if from as an unusual Object, it could have a next() 
method
+                // that yields a Number or String to close the range
+                if ((tempfrom instanceof Number && !(tempto instanceof Number))
+                        || (tempfrom instanceof String && !(tempto instanceof 
String))) {
+                    throw new IllegalArgumentException("Incompatible Argument 
classes for ObjectRange " + smaller.getClass() + ", " + larger.getClass());
+                }
+                // Since normalizing did not help, use original values at 
users risk
+                this.from = smaller;
+                this.to = larger;
+            } else {
+                this.from = tempfrom;
+                this.to = tempto;
+            }
         }
         checkBoundaryCompatibility();
     }
@@ -140,19 +157,20 @@ public class ObjectRange extends AbstractList implements 
Range {
      * Called at construction time, subclasses may override cautiously (using 
only members to and from).
      */
     protected void checkBoundaryCompatibility() {
-        if (from instanceof String || to instanceof String) {
+        if (from instanceof String && to instanceof String) {
             // this test depends deeply on the String.next implementation
             // 009.next is 00:, not 010
             String start = from.toString();
             String end = to.toString();
-            if (start.length() > end.length()) {
-                throw new IllegalArgumentException("Incompatible Strings for 
Range: starting String is longer than ending string");
+            if (start.length() != end.length()) {
+                throw new IllegalArgumentException("Incompatible Strings for 
Range: different length");
             }
-            int length = Math.min(start.length(), end.length());
+            int length = start.length();
             int i;
             for (i = 0; i < length; i++) {
                 if (start.charAt(i) != end.charAt(i)) break;
             }
+            // strings must be equal except for the last character
             if (i < length - 1) {
                 throw new IllegalArgumentException("Incompatible Strings for 
Range: String#next() will not reach the expected value");
             }
@@ -287,8 +305,8 @@ public class ObjectRange extends AbstractList implements 
Range {
                 char fromNum = (Character) from;
                 char toNum = (Character) to;
                 size = toNum - fromNum + 1;
-            } else if (from instanceof BigDecimal || to instanceof BigDecimal 
||
-                    from instanceof BigInteger || to instanceof BigInteger) {
+            } else if (((from instanceof BigDecimal || from instanceof 
BigInteger ) && to instanceof Number) ||
+                    ((to instanceof BigDecimal || to instanceof BigInteger) && 
from instanceof Number)) {
                 // let's fast calculate the size
                 BigDecimal fromNum = new BigDecimal(from.toString());
                 BigDecimal toNum = new BigDecimal(to.toString());
@@ -302,7 +320,8 @@ public class ObjectRange extends AbstractList implements 
Range {
                 while (compareTo(to, value) >= 0) {
                     value = (Comparable) increment(value);
                     size++;
-                    if (compareTo(first, value) >= 0) break; // handle back to 
beginning due to modulo incrementing
+                    // handle back to beginning due to modulo incrementing
+                    if (compareTo(first, value) >= 0) break;
                 }
             }
         }
@@ -415,6 +434,11 @@ public class ObjectRange extends AbstractList implements 
Range {
         return InvokerHelper.invokeMethod(value, "previous", null);
     }
 
+    /**
+     * if operand is a Character or a String with one character, return that 
characters int value.
+     * @param operand
+     * @return
+     */
     private static Comparable normaliseStringType(final Comparable operand) {
         if (operand instanceof Character) {
             return (int) (Character) operand;

http://git-wip-us.apache.org/repos/asf/groovy/blob/320500cd/src/test/groovy/lang/ObjectRangeTest.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/lang/ObjectRangeTest.java 
b/src/test/groovy/lang/ObjectRangeTest.java
index 6fdba89..a98dd97 100644
--- a/src/test/groovy/lang/ObjectRangeTest.java
+++ b/src/test/groovy/lang/ObjectRangeTest.java
@@ -21,6 +21,8 @@ package groovy.lang;
 import junit.framework.TestCase;
 
 import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -34,17 +36,49 @@ public class ObjectRangeTest extends TestCase {
         assertEquals("Size of " + r, 11, r.size());
         r = createRange(0, 1);
         assertEquals("Size of " + r, 2, r.size());
+        r = createRange(1, 0);
+        assertEquals("Size of " + r, 2, r.size());
         r = createRange(0, 0);
         assertEquals("Size of " + r, 1, r.size());
 
         r = createRange(new BigDecimal("2.1"), new BigDecimal("10.0"));
         assertEquals("Size of " + r, 8, r.size());
+        r = createRange(new BigDecimal("10"), new BigDecimal("2.1"));
+        assertEquals("Size of " + r, 8, r.size());
+
+        r = createRange("a", "d");
+        assertEquals("Size of " + r, 4, r.size());
+        r = createRange("d", "a");
+        assertEquals("Size of " + r, 4, r.size());
+
+        r = createRange("aa1", "aa4");
+        assertEquals("Size of " + r, 4, r.size());
+        r = createRange("aa4", "aa1");
+        assertEquals("Size of " + r, 4, r.size());
+        r = createRange('7',  ';');
+        assertEquals(5, r.size());
+
+        // '7', '8', '9', ':', ';'
+        Range mixed = createRange('7',  ';');
+        assertEquals(5, mixed.size());
+        mixed = createRange('7',  59.5);
+        assertEquals(5, mixed.size());
+        mixed = createRange('7',  59);
+        assertEquals(5, mixed.size());
+        mixed = createRange('7',  new BigInteger("59"));
+        assertEquals(5, mixed.size());
+        mixed = createRange('7',  new BigDecimal("59.5"));
+        assertEquals(5, mixed.size());
     }
 
     public void testProperties() {
         Range r = createRange(0, 10);
         assertEquals("from", 0, r.getFrom());
         assertEquals("to", 10, r.getTo());
+
+        r = createRange(10, 0);
+        assertEquals("from", 0, r.getFrom());
+        assertEquals("to", 10, r.getTo());
     }
 
     public void testGet() {
@@ -59,6 +93,18 @@ public class ObjectRangeTest extends TestCase {
             BigDecimal value = (BigDecimal) r.get(i);
             assertEquals("Item at index: " + i, new BigDecimal("3.2").add(new 
BigDecimal("" + i)), value);
         }
+
+        r = new ObjectRange(10, 20, false);
+        for (int i = 0; i < 10; i++) {
+            Integer value = (Integer) r.get(i);
+            assertEquals("Item at index: " + i, i + 10, value.intValue());
+        }
+
+        r = new ObjectRange(10, 20, true);
+        for (int i = 0; i < 10; i++) {
+            Integer value = (Integer) r.get(i);
+            assertEquals("Item at index: " + i, 20 - i, value.intValue());
+        }
     }
 
     public void testNullForFromOrToIsIllegal() {
@@ -73,7 +119,26 @@ public class ObjectRangeTest extends TestCase {
     }
 
     public void testGetOutOfRange() {
-        Range r = createRange(10, 20);
+        Range r = createRange(1, 1);
+        assertEquals("Item at index: 0", 1, r.get(0));
+
+        try {
+            r.get(-1);
+            fail("Should have thrown IndexOutOfBoundsException");
+        }
+        catch (IndexOutOfBoundsException e) {
+            // worked
+        }
+
+        try {
+            r.get(1);
+            fail("Should have thrown IndexOutOfBoundsException");
+        }
+        catch (IndexOutOfBoundsException e) {
+            // worked
+        }
+
+        r = createRange(10, 20);
 
         try {
             r.get(-1);
@@ -109,6 +174,36 @@ public class ObjectRangeTest extends TestCase {
 
     }
 
+    public void testMixedCreation() {
+        try {
+            createRange("aa", "a");
+            fail();
+        } catch (IllegalArgumentException e) {
+            // pass
+        }
+        try {
+            createRange("11", 11);
+            fail();
+        } catch (IllegalArgumentException e) {
+            // pass
+        }
+        try {
+            createRange(11, "11");
+            fail();
+        } catch (IllegalArgumentException e) {
+            // pass
+        }
+
+        Range mixed = createRange('7',  59.5);
+        assertEquals(5, mixed.size());
+        assertEquals(Arrays.asList('7', '8', '9', ':', ';'), mixed.step(1));
+
+        mixed = createRange('7', BigInteger.valueOf(59));
+        assertEquals(5, mixed.size());
+        assertEquals(Arrays.asList('7', '8', '9', ':', ';'), mixed.step(1));
+    }
+
+
     public void testContains() {
         Range r = createRange(10, 20);
 
@@ -149,6 +244,9 @@ public class ObjectRangeTest extends TestCase {
 
     public void testSubList() {
         Range r = createRange(10, 20);
+        assertEquals("from", 10, r.getFrom());
+        assertEquals("to", 20, r.getTo());
+        assertEquals("size", 11, r.size());
 
         List s = r.subList(2, 4);
 
@@ -158,6 +256,28 @@ public class ObjectRangeTest extends TestCase {
         assertEquals("to", 13, sr.getTo());
         assertEquals("size", 2, sr.size());
 
+        s = r.subList(0, 11);
+
+        sr = (Range) s;
+
+        assertEquals("from", 10, sr.getFrom());
+        assertEquals("to", 20, sr.getTo());
+        assertEquals("size", 11, sr.size());
+
+        try {
+            r.subList(-2, 4);
+            fail();
+        } catch (IndexOutOfBoundsException e) {
+            // pass
+        }
+
+        try {
+            r.subList(5, 12);
+            fail();
+        } catch (IndexOutOfBoundsException e) {
+            // pass
+        }
+
         r = createRange(new BigDecimal("0.5"), new BigDecimal("8.5"));
         assertEquals("size", 9, r.size());
         s = r.subList(2, 5);
@@ -183,14 +303,22 @@ public class ObjectRangeTest extends TestCase {
         assertFalse("a != c", a.equals(c));
     }
 
-    public void testIterator() {
+    public void testIteratorAndStep1() {
         Range r = createRange(5, 11);
 
-        int i = 5;
+        int i = 4;
         for (Iterator it = r.iterator(); it.hasNext();) {
+            i++;
             assertEquals("equals to " + i, new Integer(i), (Integer) 
(it.next()));
+        }
+        assertEquals(11, i);
+
+        i = 4;
+        for (Iterator it = r.step(1).iterator(); it.hasNext();) {
             i++;
+            assertEquals("equals to " + i, new Integer(i), (Integer) 
(it.next()));
         }
+        assertEquals(11, i);
 
         r = createRange(new BigDecimal("5.0"), new BigDecimal("11.0"));
         BigDecimal one = new BigDecimal("1.0");
@@ -200,16 +328,44 @@ public class ObjectRangeTest extends TestCase {
             assertEquals("equals to " + val, val, (BigDecimal) (it.next()));
             val = val.add(one);
         }
+        assertEquals(11, i);
+
+        val = new BigDecimal("5.0");
+        for (Iterator it = r.step(1).iterator(); it.hasNext();) {
+            assertEquals("equals to " + val, val, (BigDecimal) (it.next()));
+            val = val.add(one);
+        }
+        assertEquals(11, i);
+
+        r = createRange(new Character('a'), new Character('z'));
+        char valChar = 'a';
+        for (Iterator it = r.iterator(); it.hasNext();) {
+            assertEquals("equals to " + valChar, valChar, ((Character) 
it.next()).charValue());
+            if (it.hasNext()) {
+                valChar = (char) (((int) valChar) + 1);
+            }
+        }
+        assertEquals('z', valChar);
+
+        valChar = 'a';
+        for (Iterator it = r.step(1).iterator(); it.hasNext();) {
+            assertEquals("equals to " + valChar, valChar, ((Character) 
it.next()).charValue());
+            if (it.hasNext()) {
+                valChar = (char) (((int) valChar) + 1);
+            }
+        }
+        assertEquals('z', valChar);
     }
 
     protected Range createRange(int from, int to) {
         return new ObjectRange(new Integer(from), new Integer(to));
     }
 
-    protected Range createRange(BigDecimal from, BigDecimal to) {
+    protected Range createRange(Comparable from, Comparable to) {
         return new ObjectRange(from, to);
     }
 
+
     protected void assertEquals(String msg, int expected, Object value) {
         assertEquals(msg, new Integer(expected), value);
     }

Reply via email to