You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2016/06/28 07:05:44 UTC

[1/4] groovy git commit: Refactor: rename RangeTest to ObjectRangeTest, because it tests ObjectRange only

Repository: groovy
Updated Branches:
  refs/heads/master f646e6e12 -> 19a9bf5d0


Refactor: rename RangeTest to ObjectRangeTest, because it tests ObjectRange only


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

Branch: refs/heads/master
Commit: 849af362de116561f78d1aa6b4215960b7657eff
Parents: f646e6e
Author: Thibault Kruse <th...@gmx.de>
Authored: Tue Sep 8 14:31:17 2015 +0200
Committer: paulk <pa...@asert.com.au>
Committed: Tue Jun 28 17:05:24 2016 +1000

----------------------------------------------------------------------
 src/test/groovy/lang/ObjectRangeTest.java | 217 +++++++++++++++++++++++++
 src/test/groovy/lang/RangeTest.java       | 217 -------------------------
 2 files changed, 217 insertions(+), 217 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/849af362/src/test/groovy/lang/ObjectRangeTest.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/lang/ObjectRangeTest.java b/src/test/groovy/lang/ObjectRangeTest.java
new file mode 100644
index 0000000..6fdba89
--- /dev/null
+++ b/src/test/groovy/lang/ObjectRangeTest.java
@@ -0,0 +1,217 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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 groovy.lang;
+
+import junit.framework.TestCase;
+
+import java.math.BigDecimal;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * @author James Strachan
+ */
+public class ObjectRangeTest extends TestCase {
+
+    public void testSize() {
+        Range r = createRange(0, 10);
+        assertEquals("Size of " + r, 11, r.size());
+        r = createRange(0, 1);
+        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());
+    }
+
+    public void testProperties() {
+        Range r = createRange(0, 10);
+        assertEquals("from", 0, r.getFrom());
+        assertEquals("to", 10, r.getTo());
+    }
+
+    public void testGet() {
+        Range r = createRange(10, 20);
+        for (int i = 0; i < 10; i++) {
+            Integer value = (Integer) r.get(i);
+            assertEquals("Item at index: " + i, i + 10, value.intValue());
+        }
+
+        r = createRange(new BigDecimal("3.2"), new BigDecimal("9.9"));
+        for (int i = 0; i < r.size(); i++) {
+            BigDecimal value = (BigDecimal) r.get(i);
+            assertEquals("Item at index: " + i, new BigDecimal("3.2").add(new BigDecimal("" + i)), value);
+        }
+    }
+
+    public void testNullForFromOrToIsIllegal() {
+        Comparable dontcare = new Integer(0);
+        try {
+            new ObjectRange((Comparable)null, dontcare);
+            fail("Should have thrown IllegalArgumentException");
+        }
+        catch (IllegalArgumentException e) {
+            // worked
+        }
+    }
+
+    public void testGetOutOfRange() {
+        Range r = createRange(10, 20);
+
+        try {
+            r.get(-1);
+            fail("Should have thrown IndexOutOfBoundsException");
+        }
+        catch (IndexOutOfBoundsException e) {
+            // worked
+        }
+        try {
+            r.get(11);
+            fail("Should have thrown IndexOutOfBoundsException");
+        }
+        catch (IndexOutOfBoundsException e) {
+            // worked
+        }
+
+        r = createRange(new BigDecimal("-4.3"), new BigDecimal("1.4"));
+
+        try {
+            r.get(-1);
+            fail("Should have thrown IndexOutOfBoundsException");
+        }
+        catch (IndexOutOfBoundsException e) {
+            // worked
+        }
+        try {
+            r.get(7);
+            fail("Should have thrown IndexOutOfBoundsException");
+        }
+        catch (IndexOutOfBoundsException e) {
+            // worked
+        }
+
+    }
+
+    public void testContains() {
+        Range r = createRange(10, 20);
+
+        assertTrue("contains 11", r.contains(new Integer(11)));
+        assertTrue("contains 10", r.contains(new Integer(10)));
+        assertTrue("contains 19", r.contains(new Integer(19)));
+        assertFalse("contains 9", r.contains(new Integer(9)));
+        assertFalse("contains 21", r.contains(new Integer(21)));
+        assertFalse("contains 100", r.contains(new Integer(100)));
+        assertFalse("contains -1", r.contains(new Integer(-1)));
+
+        r = createRange(new BigDecimal("2.1"), new BigDecimal("10.0"));
+
+        assertTrue("contains 9.1", r.contains(new BigDecimal("9.1")));
+        assertFalse("contains 10.1", r.contains(new BigDecimal("10.1")));
+        assertFalse("contains 8.0", r.contains(new BigDecimal("8.0")));
+        assertTrue("containsWithinBounds 8.0", r.containsWithinBounds(new BigDecimal("8.0")));
+        assertTrue("containsWithinBounds 9.9999", r.containsWithinBounds(new BigDecimal("9.9999")));
+        assertTrue("containsWithinBounds 10.0", r.containsWithinBounds(new BigDecimal("10.0")));
+        assertFalse("containsWithinBounds 10.0001", r.containsWithinBounds(new BigDecimal("10.0001")));
+    }
+
+    public void testContainsWithLikeNumbers() {
+        Range r = new ObjectRange(new Integer(1), new Short((short)3));
+        assertTrue("contains 2", r.contains(new Integer(2)));
+        r = new ObjectRange(new Float(1.0), new Double(3.0));
+        assertTrue("contains 2.0d", r.contains(new Double(2.0)));
+        assertTrue("contains 2.0g", r.contains(new BigDecimal(2.0)));
+        r = new ObjectRange(new BigDecimal(1.0), new BigDecimal(3.0));
+        assertTrue("contains 2.0d", r.contains(new Double(2.0)));
+        assertTrue("contains 2.0f", r.contains(new Float(2.0)));
+    }
+
+    public void testContainsWithIncompatibleType() {
+        Range r = new ObjectRange(new Integer(1), new Short((short)3));
+        assertFalse("shouldn't contain string", r.contains("String"));
+    }
+
+    public void testSubList() {
+        Range r = createRange(10, 20);
+
+        List s = r.subList(2, 4);
+
+        Range sr = (Range) s;
+
+        assertEquals("from", 12, sr.getFrom());
+        assertEquals("to", 13, sr.getTo());
+        assertEquals("size", 2, sr.size());
+
+        r = createRange(new BigDecimal("0.5"), new BigDecimal("8.5"));
+        assertEquals("size", 9, r.size());
+        s = r.subList(2, 5);
+        sr = (Range) s;
+
+        assertEquals("from", new BigDecimal("2.5"), sr.getFrom());
+        assertEquals("to", new BigDecimal("4.5"), sr.getTo());
+        assertTrue("contains 4.5", sr.contains(new BigDecimal("4.5")));
+        assertFalse("contains 5.5", sr.contains(new BigDecimal("5.5")));
+        assertEquals("size", 3, sr.size());
+
+    }
+
+    public void testHashCodeAndEquals() {
+        Range a = createRange(1, 11);
+        Range b = createRange(1, 11);
+        Range c = createRange(2, 11);
+
+        assertEquals("hashcode", a.hashCode(), b.hashCode());
+        assertTrue("hashcode", a.hashCode() != c.hashCode());
+
+        assertEquals("a and b", a, b);
+        assertFalse("a != c", a.equals(c));
+    }
+
+    public void testIterator() {
+        Range r = createRange(5, 11);
+
+        int i = 5;
+        for (Iterator it = r.iterator(); it.hasNext();) {
+            assertEquals("equals to " + i, new Integer(i), (Integer) (it.next()));
+            i++;
+        }
+
+        r = createRange(new BigDecimal("5.0"), new BigDecimal("11.0"));
+        BigDecimal one = new BigDecimal("1.0");
+
+        BigDecimal val = new BigDecimal("5.0");
+        for (Iterator it = r.iterator(); it.hasNext();) {
+            assertEquals("equals to " + val, val, (BigDecimal) (it.next()));
+            val = val.add(one);
+        }
+    }
+
+    protected Range createRange(int from, int to) {
+        return new ObjectRange(new Integer(from), new Integer(to));
+    }
+
+    protected Range createRange(BigDecimal from, BigDecimal to) {
+        return new ObjectRange(from, to);
+    }
+
+    protected void assertEquals(String msg, int expected, Object value) {
+        assertEquals(msg, new Integer(expected), value);
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/groovy/blob/849af362/src/test/groovy/lang/RangeTest.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/lang/RangeTest.java b/src/test/groovy/lang/RangeTest.java
deleted file mode 100644
index 4d63a81..0000000
--- a/src/test/groovy/lang/RangeTest.java
+++ /dev/null
@@ -1,217 +0,0 @@
-/*
- *  Licensed to the Apache Software Foundation (ASF) under one
- *  or more contributor license agreements.  See the NOTICE file
- *  distributed with this work for additional information
- *  regarding copyright ownership.  The ASF licenses this file
- *  to you 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 groovy.lang;
-
-import junit.framework.TestCase;
-
-import java.math.BigDecimal;
-import java.util.Iterator;
-import java.util.List;
-
-/**
- * @author James Strachan
- */
-public class RangeTest extends TestCase {
-
-    public void testSize() {
-        Range r = createRange(0, 10);
-        assertEquals("Size of " + r, 11, r.size());
-        r = createRange(0, 1);
-        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());
-    }
-
-    public void testProperties() {
-        Range r = createRange(0, 10);
-        assertEquals("from", 0, r.getFrom());
-        assertEquals("to", 10, r.getTo());
-    }
-
-    public void testGet() {
-        Range r = createRange(10, 20);
-        for (int i = 0; i < 10; i++) {
-            Integer value = (Integer) r.get(i);
-            assertEquals("Item at index: " + i, i + 10, value.intValue());
-        }
-
-        r = createRange(new BigDecimal("3.2"), new BigDecimal("9.9"));
-        for (int i = 0; i < r.size(); i++) {
-            BigDecimal value = (BigDecimal) r.get(i);
-            assertEquals("Item at index: " + i, new BigDecimal("3.2").add(new BigDecimal("" + i)), value);
-        }
-    }
-
-    public void testNullForFromOrToIsIllegal() {
-        Comparable dontcare = new Integer(0);
-        try {
-            new ObjectRange((Comparable)null, dontcare);
-            fail("Should have thrown IllegalArgumentException");
-        }
-        catch (IllegalArgumentException e) {
-            // worked
-        }
-    }
-
-    public void testGetOutOfRange() {
-        Range r = createRange(10, 20);
-
-        try {
-            r.get(-1);
-            fail("Should have thrown IndexOutOfBoundsException");
-        }
-        catch (IndexOutOfBoundsException e) {
-            // worked
-        }
-        try {
-            r.get(11);
-            fail("Should have thrown IndexOutOfBoundsException");
-        }
-        catch (IndexOutOfBoundsException e) {
-            // worked
-        }
-
-        r = createRange(new BigDecimal("-4.3"), new BigDecimal("1.4"));
-
-        try {
-            r.get(-1);
-            fail("Should have thrown IndexOutOfBoundsException");
-        }
-        catch (IndexOutOfBoundsException e) {
-            // worked
-        }
-        try {
-            r.get(7);
-            fail("Should have thrown IndexOutOfBoundsException");
-        }
-        catch (IndexOutOfBoundsException e) {
-            // worked
-        }
-
-    }
-
-    public void testContains() {
-        Range r = createRange(10, 20);
-
-        assertTrue("contains 11", r.contains(new Integer(11)));
-        assertTrue("contains 10", r.contains(new Integer(10)));
-        assertTrue("contains 19", r.contains(new Integer(19)));
-        assertFalse("contains 9", r.contains(new Integer(9)));
-        assertFalse("contains 21", r.contains(new Integer(21)));
-        assertFalse("contains 100", r.contains(new Integer(100)));
-        assertFalse("contains -1", r.contains(new Integer(-1)));
-
-        r = createRange(new BigDecimal("2.1"), new BigDecimal("10.0"));
-
-        assertTrue("contains 9.1", r.contains(new BigDecimal("9.1")));
-        assertFalse("contains 10.1", r.contains(new BigDecimal("10.1")));
-        assertFalse("contains 8.0", r.contains(new BigDecimal("8.0")));
-        assertTrue("containsWithinBounds 8.0", r.containsWithinBounds(new BigDecimal("8.0")));
-        assertTrue("containsWithinBounds 9.9999", r.containsWithinBounds(new BigDecimal("9.9999")));
-        assertTrue("containsWithinBounds 10.0", r.containsWithinBounds(new BigDecimal("10.0")));
-        assertFalse("containsWithinBounds 10.0001", r.containsWithinBounds(new BigDecimal("10.0001")));
-    }
-
-    public void testContainsWithLikeNumbers() {
-        Range r = new ObjectRange(new Integer(1), new Short((short)3));
-        assertTrue("contains 2", r.contains(new Integer(2)));
-        r = new ObjectRange(new Float(1.0), new Double(3.0));
-        assertTrue("contains 2.0d", r.contains(new Double(2.0)));
-        assertTrue("contains 2.0g", r.contains(new BigDecimal(2.0)));
-        r = new ObjectRange(new BigDecimal(1.0), new BigDecimal(3.0));
-        assertTrue("contains 2.0d", r.contains(new Double(2.0)));
-        assertTrue("contains 2.0f", r.contains(new Float(2.0)));
-    }
-
-    public void testContainsWithIncompatibleType() {
-        Range r = new ObjectRange(new Integer(1), new Short((short)3));
-        assertFalse("shouldn't contain string", r.contains("String"));
-    }
-
-    public void testSubList() {
-        Range r = createRange(10, 20);
-
-        List s = r.subList(2, 4);
-
-        Range sr = (Range) s;
-
-        assertEquals("from", 12, sr.getFrom());
-        assertEquals("to", 13, sr.getTo());
-        assertEquals("size", 2, sr.size());
-
-        r = createRange(new BigDecimal("0.5"), new BigDecimal("8.5"));
-        assertEquals("size", 9, r.size());
-        s = r.subList(2, 5);
-        sr = (Range) s;
-
-        assertEquals("from", new BigDecimal("2.5"), sr.getFrom());
-        assertEquals("to", new BigDecimal("4.5"), sr.getTo());
-        assertTrue("contains 4.5", sr.contains(new BigDecimal("4.5")));
-        assertFalse("contains 5.5", sr.contains(new BigDecimal("5.5")));
-        assertEquals("size", 3, sr.size());
-
-    }
-
-    public void testHashCodeAndEquals() {
-        Range a = createRange(1, 11);
-        Range b = createRange(1, 11);
-        Range c = createRange(2, 11);
-
-        assertEquals("hashcode", a.hashCode(), b.hashCode());
-        assertTrue("hashcode", a.hashCode() != c.hashCode());
-
-        assertEquals("a and b", a, b);
-        assertFalse("a != c", a.equals(c));
-    }
-
-    public void testIterator() {
-        Range r = createRange(5, 11);
-
-        int i = 5;
-        for (Iterator it = r.iterator(); it.hasNext();) {
-            assertEquals("equals to " + i, new Integer(i), (Integer) (it.next()));
-            i++;
-        }
-
-        r = createRange(new BigDecimal("5.0"), new BigDecimal("11.0"));
-        BigDecimal one = new BigDecimal("1.0");
-
-        BigDecimal val = new BigDecimal("5.0");
-        for (Iterator it = r.iterator(); it.hasNext();) {
-            assertEquals("equals to " + val, val, (BigDecimal) (it.next()));
-            val = val.add(one);
-        }
-    }
-
-    protected Range createRange(int from, int to) {
-        return new ObjectRange(new Integer(from), new Integer(to));
-    }
-
-    protected Range createRange(BigDecimal from, BigDecimal to) {
-        return new ObjectRange(from, to);
-    }
-
-    protected void assertEquals(String msg, int expected, Object value) {
-        assertEquals(msg, new Integer(expected), value);
-    }
-
-}


[3/4] groovy git commit: GROOVY-7585: Accept normalization if both arguments end up numbers, throw error when starting range with Number or String and ending with something else

Posted by pa...@apache.org.
GROOVY-7585: Accept normalization if both arguments end up numbers, throw error when starting range with Number or String and ending with something else


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

Branch: refs/heads/master
Commit: da40a68232bba804ae51670597a5d38e4ea7cfd9
Parents: 320500c
Author: Thibault Kruse <th...@gmx.de>
Authored: Thu Oct 15 11:19:53 2015 +0200
Committer: paulk <pa...@asert.com.au>
Committed: Tue Jun 28 17:05:28 2016 +1000

----------------------------------------------------------------------
 src/main/groovy/lang/ObjectRange.java     | 34 ++++++++++++++++++--------
 src/test/groovy/lang/ObjectRangeTest.java |  4 +--
 2 files changed, 26 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/da40a682/src/main/groovy/lang/ObjectRange.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/ObjectRange.java b/src/main/groovy/lang/ObjectRange.java
index f196c93..f8c2a3a 100644
--- a/src/main/groovy/lang/ObjectRange.java
+++ b/src/main/groovy/lang/ObjectRange.java
@@ -124,29 +124,43 @@ public class ObjectRange extends AbstractList implements Range {
             larger = ((Integer) larger).longValue();
         }
 
-        // TODO: should we care about different types here?
-        if (smaller.getClass() == larger.getClass()) {
+        /*
+        areReversed() already does an implicit type compatibility check
+        based on DefaultTypeTransformation.compareToWithEqualityCheck() for mixed classes
+        but it is only invoked if reverse == null.
+        So Object Range has to perform those type checks for consistency even when not calling
+        compareToWithEqualityCheck(), and ObjectRange has
+        to use the normalized value used in a successful comparison in
+        compareToWithEqualityCheck(). Currently that means Chars and single-char Strings
+        are evaluated as the char's charValue (an integer) when compared to numbers.
+        So '7'..'9' should produce ['7', '8', '9'], whereas ['7'..9] and [7..'9'] should produce [55, 56, 57].
+        if classes match, or both numericals, no checks possible / necessary
+        */
+        if (smaller.getClass() == larger.getClass() ||
+                (smaller instanceof Number && larger instanceof Number)) {
             this.from = smaller;
             this.to = larger;
         } else {
             // Convenience hack: try convert single-char strings to ints
             Comparable tempfrom = normaliseStringType(smaller);
             Comparable tempto = normaliseStringType(larger);
-            if (tempfrom.getClass() != tempto.getClass()) {
+            // if after normalizing both are numbers, assume intended range was numbers
+            if (tempfrom instanceof Number && tempto instanceof Number) {
+                this.from = tempfrom;
+                this.to = tempto;
+            } else {
                 // 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
+                // throw exception when starting with known class, and thus "from" cannot be advanced over "to".
+                // Note if start is 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))) {
+                Comparable start = this.reverse ? larger : smaller;
+                if (start instanceof String || start instanceof Number) {
+                    // starting with number will never reach a non-number, same for 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();

http://git-wip-us.apache.org/repos/asf/groovy/blob/da40a682/src/test/groovy/lang/ObjectRangeTest.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/lang/ObjectRangeTest.java b/src/test/groovy/lang/ObjectRangeTest.java
index a98dd97..9463741 100644
--- a/src/test/groovy/lang/ObjectRangeTest.java
+++ b/src/test/groovy/lang/ObjectRangeTest.java
@@ -196,11 +196,11 @@ public class ObjectRangeTest extends TestCase {
 
         Range mixed = createRange('7',  59.5);
         assertEquals(5, mixed.size());
-        assertEquals(Arrays.asList('7', '8', '9', ':', ';'), mixed.step(1));
+        assertEquals(Arrays.asList(55, 56, 57, 58, 59), mixed.step(1));
 
         mixed = createRange('7', BigInteger.valueOf(59));
         assertEquals(5, mixed.size());
-        assertEquals(Arrays.asList('7', '8', '9', ':', ';'), mixed.step(1));
+        assertEquals(Arrays.asList(55, 56, 57, 58, 59), mixed.step(1));
     }
 
 


[4/4] groovy git commit: GROOVY-7585: ObjectRange strange semantics for mismatched arguments (closes #142)

Posted by pa...@apache.org.
GROOVY-7585: ObjectRange strange semantics for mismatched arguments (closes #142)


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

Branch: refs/heads/master
Commit: 19a9bf5d0c5bed12642f67f7a67d5bdca7089464
Parents: da40a68
Author: paulk <pa...@asert.com.au>
Authored: Tue Jun 28 17:03:05 2016 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Tue Jun 28 17:05:30 2016 +1000

----------------------------------------------------------------------
 src/main/groovy/lang/ObjectRange.java     | 24 +++++++++++-------------
 src/test/groovy/lang/ObjectRangeTest.java |  7 +++----
 2 files changed, 14 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/19a9bf5d/src/main/groovy/lang/ObjectRange.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/ObjectRange.java b/src/main/groovy/lang/ObjectRange.java
index f8c2a3a..f108433 100644
--- a/src/main/groovy/lang/ObjectRange.java
+++ b/src/main/groovy/lang/ObjectRange.java
@@ -125,16 +125,16 @@ public class ObjectRange extends AbstractList implements Range {
         }
 
         /*
-        areReversed() already does an implicit type compatibility check
-        based on DefaultTypeTransformation.compareToWithEqualityCheck() for mixed classes
-        but it is only invoked if reverse == null.
-        So Object Range has to perform those type checks for consistency even when not calling
-        compareToWithEqualityCheck(), and ObjectRange has
-        to use the normalized value used in a successful comparison in
-        compareToWithEqualityCheck(). Currently that means Chars and single-char Strings
-        are evaluated as the char's charValue (an integer) when compared to numbers.
-        So '7'..'9' should produce ['7', '8', '9'], whereas ['7'..9] and [7..'9'] should produce [55, 56, 57].
-        if classes match, or both numericals, no checks possible / necessary
+            areReversed() already does an implicit type compatibility check
+            based on DefaultTypeTransformation.compareToWithEqualityCheck() for mixed classes
+            but it is only invoked if reverse == null.
+            So Object Range has to perform those type checks for consistency even when not calling
+            compareToWithEqualityCheck(), and ObjectRange has
+            to use the normalized value used in a successful comparison in
+            compareToWithEqualityCheck(). Currently that means Chars and single-char Strings
+            are evaluated as the char's charValue (an integer) when compared to numbers.
+            So '7'..'9' should produce ['7', '8', '9'], whereas ['7'..9] and [7..'9'] should produce [55, 56, 57].
+            if classes match, or both numerical, no checks possible / necessary
         */
         if (smaller.getClass() == larger.getClass() ||
                 (smaller instanceof Number && larger instanceof Number)) {
@@ -449,9 +449,7 @@ public class ObjectRange extends AbstractList implements Range {
     }
 
     /**
-     * if operand is a Character or a String with one character, return that characters int value.
-     * @param operand
-     * @return
+     * if operand is a Character or a String with one character, return that character's int value.
      */
     private static Comparable normaliseStringType(final Comparable operand) {
         if (operand instanceof Character) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/19a9bf5d/src/test/groovy/lang/ObjectRangeTest.java
----------------------------------------------------------------------
diff --git a/src/test/groovy/lang/ObjectRangeTest.java b/src/test/groovy/lang/ObjectRangeTest.java
index 9463741..fc0e8bc 100644
--- a/src/test/groovy/lang/ObjectRangeTest.java
+++ b/src/test/groovy/lang/ObjectRangeTest.java
@@ -27,7 +27,7 @@ import java.util.Iterator;
 import java.util.List;
 
 /**
- * @author James Strachan
+ * Provides unit tests for the <code>ObjectRange</code> class.
  */
 public class ObjectRangeTest extends TestCase {
 
@@ -181,12 +181,14 @@ public class ObjectRangeTest extends TestCase {
         } catch (IllegalArgumentException e) {
             // pass
         }
+
         try {
             createRange("11", 11);
             fail();
         } catch (IllegalArgumentException e) {
             // pass
         }
+
         try {
             createRange(11, "11");
             fail();
@@ -203,7 +205,6 @@ public class ObjectRangeTest extends TestCase {
         assertEquals(Arrays.asList(55, 56, 57, 58, 59), mixed.step(1));
     }
 
-
     public void testContains() {
         Range r = createRange(10, 20);
 
@@ -288,7 +289,6 @@ public class ObjectRangeTest extends TestCase {
         assertTrue("contains 4.5", sr.contains(new BigDecimal("4.5")));
         assertFalse("contains 5.5", sr.contains(new BigDecimal("5.5")));
         assertEquals("size", 3, sr.size());
-
     }
 
     public void testHashCodeAndEquals() {
@@ -365,7 +365,6 @@ public class ObjectRangeTest extends TestCase {
         return new ObjectRange(from, to);
     }
 
-
     protected void assertEquals(String msg, int expected, Object value) {
         assertEquals(msg, new Integer(expected), value);
     }


[2/4] groovy git commit: 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

Posted by pa...@apache.org.
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 <th...@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);
     }