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); }