Repository: groovy Updated Branches: refs/heads/master 923fd338e -> 844b5b70c
GROOVY-7876 - ClassCastException when calling DefaultTypeTransformation#compareEqual (additional refactoring closes #372) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/844b5b70 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/844b5b70 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/844b5b70 Branch: refs/heads/master Commit: 844b5b70c520def50bc1e27fd4834ba5b645cc90 Parents: 923fd33 Author: paulk <pa...@asert.com.au> Authored: Tue Jul 26 00:53:39 2016 +1000 Committer: paulk <pa...@asert.com.au> Committed: Tue Jul 26 08:43:10 2016 +1000 ---------------------------------------------------------------------- src/main/groovy/lang/ObjectRange.java | 10 +++------- .../groovy/runtime/NumberAwareComparator.java | 2 ++ .../typehandling/DefaultTypeTransformation.java | 16 +++++++++------- src/test/groovy/bugs/Groovy7876Bug.groovy | 15 +++++++++++++-- .../DefaultTypeTransformationTest.groovy | 6 +++--- 5 files changed, 30 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/844b5b70/src/main/groovy/lang/ObjectRange.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/lang/ObjectRange.java b/src/main/groovy/lang/ObjectRange.java index d3581d4..d6e0f4e 100644 --- a/src/main/groovy/lang/ObjectRange.java +++ b/src/main/groovy/lang/ObjectRange.java @@ -196,8 +196,8 @@ public class ObjectRange extends AbstractList<Comparable> implements Range<Compa private static boolean areReversed(Comparable from, Comparable to) { try { return ScriptBytecodeAdapter.compareGreaterThan(from, to); - } catch (ClassCastException cce) { - throw new IllegalArgumentException("Unable to create range due to incompatible types: " + from.getClass().getSimpleName() + ".." + to.getClass().getSimpleName() + " (possible missing brackets around range?)", cce); + } catch (IllegalArgumentException iae) { + throw new IllegalArgumentException("Unable to create range due to incompatible types: " + from.getClass().getSimpleName() + ".." + to.getClass().getSimpleName() + " (possible missing brackets around range?)", iae); } } @@ -384,11 +384,7 @@ public class ObjectRange extends AbstractList<Comparable> implements Range<Compa return false; } while (iter.hasNext()) { - try { - if (DefaultTypeTransformation.compareEqual(value, iter.next())) return true; - } catch (ClassCastException e) { - return false; - } + if (DefaultTypeTransformation.compareEqual(value, iter.next())) return true; } return false; } http://git-wip-us.apache.org/repos/asf/groovy/blob/844b5b70/src/main/org/codehaus/groovy/runtime/NumberAwareComparator.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/runtime/NumberAwareComparator.java b/src/main/org/codehaus/groovy/runtime/NumberAwareComparator.java index c85cd04..f8aff32 100644 --- a/src/main/org/codehaus/groovy/runtime/NumberAwareComparator.java +++ b/src/main/org/codehaus/groovy/runtime/NumberAwareComparator.java @@ -36,6 +36,8 @@ public class NumberAwareComparator<T> implements Comparator<T> { /* ignore */ } catch (GroovyRuntimeException gre) { /* ignore */ + } catch (IllegalArgumentException iae) { + /* ignore */ } // since the object does not have a valid compareTo method // we compare using the hashcodes. null cases are handled by http://git-wip-us.apache.org/repos/asf/groovy/blob/844b5b70/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java b/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java index bffc6ba..c57d4d4 100644 --- a/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java +++ b/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java @@ -542,6 +542,7 @@ public class DefaultTypeTransformation { } private static int compareToWithEqualityCheck(Object left, Object right, boolean equalityCheckOnly) { + Exception cause = null; if (left == right) { return 0; } @@ -592,7 +593,7 @@ public class DefaultTypeTransformation { try { return comparable.compareTo(right); } catch (ClassCastException cce) { - if (!equalityCheckOnly) throw cce; + if (!equalityCheckOnly) cause = cce; } } } @@ -600,12 +601,13 @@ public class DefaultTypeTransformation { if (equalityCheckOnly) { return -1; // anything other than 0 } - throw new GroovyRuntimeException( - MessageFormat.format("Cannot compare {0} with value ''{1}'' and {2} with value ''{3}''", - left.getClass().getName(), - left, - right.getClass().getName(), - right)); + String message = MessageFormat.format("Cannot compare {0} with value ''{1}'' and {2} with value ''{3}''", + left.getClass().getName(), left, right.getClass().getName(), right); + if (cause != null) { + throw new IllegalArgumentException(message, cause); + } else { + throw new IllegalArgumentException(message); + } } public static boolean compareEqual(Object left, Object right) { http://git-wip-us.apache.org/repos/asf/groovy/blob/844b5b70/src/test/groovy/bugs/Groovy7876Bug.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/bugs/Groovy7876Bug.groovy b/src/test/groovy/bugs/Groovy7876Bug.groovy index 5ec368e..1ff9598 100644 --- a/src/test/groovy/bugs/Groovy7876Bug.groovy +++ b/src/test/groovy/bugs/Groovy7876Bug.groovy @@ -17,12 +17,13 @@ * under the License. * */ - package groovy.bugs class Groovy7876Bug extends GroovyTestCase { void testClassCastExceptionsFromCompareToShouldNotLeakOutOfEqualityCheck() { assertScript ''' + import static groovy.test.GroovyAssert.shouldFail + enum E1 {A, B, C} enum E2 {D, E, F} class Holder<T> implements Comparable<T> { @@ -32,8 +33,18 @@ class Groovy7876Bug extends GroovyTestCase { } def a = new Holder<E1>(E1.A) def d = new Holder<E2>(E2.D) - assert E1.A != E2.D // control + + // control cases + assert E1.A != E2.D + shouldFail(IllegalArgumentException) { + E1.A <=> E2.D + } + + // holder cases assert a != d // invokes compareTo + shouldFail(IllegalArgumentException) { + a <=> d + } ''' } } http://git-wip-us.apache.org/repos/asf/groovy/blob/844b5b70/src/test/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformationTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformationTest.groovy b/src/test/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformationTest.groovy index 84464dc..11bed2c 100644 --- a/src/test/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformationTest.groovy +++ b/src/test/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformationTest.groovy @@ -80,7 +80,7 @@ class DefaultTypeTransformationTest extends GroovyTestCase { assert compareTo(null, object1) == -1 assert compareTo(1, 1) == 0 - shouldFail(GroovyRuntimeException) { + shouldFail(IllegalArgumentException) { compareTo(object1, object2) } @@ -122,10 +122,10 @@ class DefaultTypeTransformationTest extends GroovyTestCase { } } - shouldFail(ClassCastException) { + shouldFail(IllegalArgumentException) { compareTo(1, "22") } - shouldFail(ClassCastException) { + shouldFail(IllegalArgumentException) { compareTo("22", 1) }
