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/27 10:09:38 UTC

groovy git commit: GROOVY-7875: IntRange fail fast on too large a range out by one

Repository: groovy
Updated Branches:
  refs/heads/master fd8932052 -> 5158fb56e


GROOVY-7875: IntRange fail fast on too large a range out by one


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

Branch: refs/heads/master
Commit: 5158fb56ee046c43bc9b336338036313d3df8893
Parents: fd89320
Author: paulk <pa...@asert.com.au>
Authored: Mon Jun 27 20:08:14 2016 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Mon Jun 27 20:08:14 2016 +1000

----------------------------------------------------------------------
 src/main/groovy/lang/IntRange.java       | 23 +++++++++++------------
 src/test/groovy/lang/IntRangeTest.groovy | 23 +++++++++++------------
 2 files changed, 22 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/5158fb56/src/main/groovy/lang/IntRange.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/IntRange.java b/src/main/groovy/lang/IntRange.java
index d51d514..0cb43ba 100644
--- a/src/main/groovy/lang/IntRange.java
+++ b/src/main/groovy/lang/IntRange.java
@@ -146,12 +146,7 @@ public class IntRange extends AbstractList<Integer> implements Range<Integer> {
             this.to = to;
             this.reverse = false;
         }
-
-        // size() in the Collection interface returns an integer, so ranges can have no more than Integer.MAX_VALUE elements
-        Long size = 0L + this.to - this.from;
-        if (size >= Integer.MAX_VALUE) {
-            throw new IllegalArgumentException("A range must have no more than " + Integer.MAX_VALUE + " elements but attempted " + size + " elements");
-        }
+        checkSize();
     }
 
     /**
@@ -172,12 +167,7 @@ public class IntRange extends AbstractList<Integer> implements Range<Integer> {
         this.from = from;
         this.to = to;
         this.reverse = reverse;
-
-        // size() in the Collection interface returns an integer, so ranges can have no more than Integer.MAX_VALUE elements
-        Long size = 0L + this.to - this.from;
-        if (size >= Integer.MAX_VALUE) {
-            throw new IllegalArgumentException("A range must have no more than " + Integer.MAX_VALUE + " elements but attempted " + size + " elements");
-        }
+        checkSize();
     }
 
     /**
@@ -192,6 +182,15 @@ public class IntRange extends AbstractList<Integer> implements Range<Integer> {
         this.to = to;
         this.inclusive = inclusive;
         this.reverse = false; // range may still be reversed, this value is ignored for inclusive-aware ranges
+        checkSize();
+    }
+
+    private void checkSize() {
+        // size() in the Collection interface returns an integer, so ranges can have no more than Integer.MAX_VALUE elements
+        Long size = (long) this.to - this.from + 1;
+        if (size > Integer.MAX_VALUE) {
+            throw new IllegalArgumentException("A range must have no more than " + Integer.MAX_VALUE + " elements but attempted " + size + " elements");
+        }
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/groovy/blob/5158fb56/src/test/groovy/lang/IntRangeTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/lang/IntRangeTest.groovy b/src/test/groovy/lang/IntRangeTest.groovy
index 15b988d..73c5a5e 100644
--- a/src/test/groovy/lang/IntRangeTest.groovy
+++ b/src/test/groovy/lang/IntRangeTest.groovy
@@ -20,18 +20,17 @@ package groovy.lang;
 
 /**
  * Provides unit tests for the <code>IntRange</code> class.
- *
- * @author James Strachan
  */
 class IntRangeTest extends GroovyTestCase {
 
     void testCreateTooBigRange() {
         try {
-            new IntRange(0, Integer.MAX_VALUE);
-            fail("too large range accepted");
+            assert new IntRange(1, Integer.MAX_VALUE).size() == Integer.MAX_VALUE // biggest allowed
+            new IntRange(0, Integer.MAX_VALUE) // too big
+            fail("too large range accepted")
         }
         catch (IllegalArgumentException ignore) {
-            assertTrue("expected exception thrown", true);
+            assert ignore.message == 'A range must have no more than 2147483647 elements but attempted 2147483648 elements'
         }
     }
 
@@ -40,11 +39,11 @@ class IntRangeTest extends GroovyTestCase {
      */
     void testInvalidArgumentsToConstructor() {
         try {
-            new IntRange(2, 1, true);
-            fail("invalid range created");
+            new IntRange(2, 1, true)
+            fail("invalid range created")
         }
         catch (IllegalArgumentException ignore) {
-            assertTrue("expected exception thrown", true);
+            assertTrue("expected exception thrown", true)
         }
     }
 
@@ -59,10 +58,10 @@ class IntRangeTest extends GroovyTestCase {
      * Tests getting the to and from values as <code>int</code>s.
      */
     void testGetToFromInt() {
-        final int from = 3, to = 7;
-        final IntRange range = new IntRange(from, to);
-        assertEquals("wrong 'from'", from, range.getFromInt());
-        assertEquals("wrong 'to'", to, range.getToInt());
+        final int from = 3, to = 7
+        final IntRange range = new IntRange(from, to)
+        assertEquals("wrong 'from'", from, range.getFromInt())
+        assertEquals("wrong 'to'", to, range.getToInt())
     }
 
     void test_Step_ShouldNotOverflowForIntegerMaxValue() {