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/07/25 22:49:03 UTC
groovy git commit: GROOVY-7876 - ClassCastException when calling
DefaultTypeTransformation#compareEqual (additional refactoring closes #372)
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)
}