You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2023/02/15 18:54:26 UTC
[groovy] branch GROOVY_4_0_X updated: GROOVY-10929: method closure swallows `IllegalArgumentException`
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new 4fcb2c8600 GROOVY-10929: method closure swallows `IllegalArgumentException`
4fcb2c8600 is described below
commit 4fcb2c860088428f1e4cffefa2dbab590016dd53
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Feb 15 12:04:32 2023 -0600
GROOVY-10929: method closure swallows `IllegalArgumentException`
---
src/main/java/groovy/lang/MetaClassImpl.java | 6 +-
src/main/java/groovy/lang/MetaProperty.java | 4 +-
.../codehaus/groovy/reflection/CachedField.java | 16 +++---
src/test/groovy/bugs/MethodClosureTest.groovy | 64 ++++++++++++++--------
4 files changed, 53 insertions(+), 37 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 26aacac30e..49ebcdc674 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1015,12 +1015,10 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
boolean ownerIsClass = (owner instanceof Class);
Class ownerClass = ownerIsClass ? (Class) owner : owner.getClass();
final MetaClass ownerMetaClass = registry.getMetaClass(ownerClass);
-
try {
return ownerMetaClass.invokeMethod(ownerClass, owner, method, arguments, false, false);
-
- } catch (MissingMethodException | InvokerInvocationException | IllegalArgumentException e) { // TODO: What if method throws IllegalArgumentException?
- if (!ownerIsClass) {
+ } catch (GroovyRuntimeException e) { // GROOVY-10929: GroovyRuntimeException(cause:IllegalArgumentException) thrown for final fields
+ if (!(ownerIsClass && (e instanceof MissingMethodException || e instanceof InvokerInvocationException || e.getCause() instanceof IllegalArgumentException))) {
throw e;
}
if (MethodClosure.NEW.equals(method)) {
diff --git a/src/main/java/groovy/lang/MetaProperty.java b/src/main/java/groovy/lang/MetaProperty.java
index 757b78822d..39bb4dd877 100644
--- a/src/main/java/groovy/lang/MetaProperty.java
+++ b/src/main/java/groovy/lang/MetaProperty.java
@@ -42,12 +42,12 @@ public abstract class MetaProperty {
/**
* @return the property of the given object
- * @throws Exception if the property could not be evaluated
+ * @throws RuntimeException if the property could not be evaluated
*/
public abstract Object getProperty(Object object);
/**
- * Sets the property on the given object to the new value
+ * Sets the property on the given object to the new value.
*
* @param object on which to set the property
* @param newValue the new value of the property
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedField.java b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
index 6c666aac71..f7ccc80634 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedField.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
@@ -44,6 +44,9 @@ public class CachedField extends MetaProperty {
return field.getDeclaringClass();
}
+ /**
+ * {@inheritDoc}
+ */
@Override
public int getModifiers() {
return field.getModifiers();
@@ -58,25 +61,20 @@ public class CachedField extends MetaProperty {
}
/**
- * @return the property of the given object
- * @throws RuntimeException if the property could not be evaluated
+ * {@inheritDoc}
*/
@Override
public Object getProperty(final Object object) {
makeAccessibleIfNecessary();
try {
return field.get(object);
- } catch (IllegalAccessException e) {
+ } catch (IllegalAccessException | IllegalArgumentException e) {
throw new GroovyRuntimeException("Cannot get the property '" + name + "'.", e);
}
}
/**
- * Sets the property on the given object to the new value
- *
- * @param object on which to set the property
- * @param newValue the new value of the property
- * @throws RuntimeException if the property could not be set
+ * {@inheritDoc}
*/
@Override
public void setProperty(final Object object, final Object newValue) {
@@ -87,7 +85,7 @@ public class CachedField extends MetaProperty {
Object goalValue = DefaultTypeTransformation.castToType(newValue, field.getType());
try {
field.set(object, goalValue);
- } catch (IllegalAccessException e) {
+ } catch (IllegalAccessException | IllegalArgumentException e) {
throw new GroovyRuntimeException("Cannot set the property '" + name + "'.", e);
}
}
diff --git a/src/test/groovy/bugs/MethodClosureTest.groovy b/src/test/groovy/bugs/MethodClosureTest.groovy
index 24c824c4b0..76b0047c67 100644
--- a/src/test/groovy/bugs/MethodClosureTest.groovy
+++ b/src/test/groovy/bugs/MethodClosureTest.groovy
@@ -18,47 +18,54 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
import org.codehaus.groovy.runtime.MethodClosure
+import org.junit.Test
-class MethodClosureTest extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+final class MethodClosureTest {
def aa(x) {
x
}
- static bb(it) { it}
+ static bb(x) {
+ x
+ }
+ @Test
void testMethodClosure() {
- def cl2 = String.&toUpperCase // Class.instanceMethod
- assert cl2 instanceof Closure
- assert cl2 instanceof MethodClosure
-
- assert ["xx", "yy"].collect(cl2) == ["XX","YY"]
-
- Class[] c1 = [ Exception.class, Throwable.class ]
- Class[] c2 = [ IllegalStateException.class ]
-
- def cl = this.&aa // instance.instanceMethod
+ def closure = this.&aa // instance.instanceMethod
+ assert closure instanceof Closure
+ assert closure instanceof MethodClosure
- assert cl instanceof Closure
- assert cl instanceof MethodClosure
+ Class[] c1 = [ Exception, Throwable ]
+ Class[] c2 = [ IllegalStateException ]
+ assert [c1, c2].collect(closure) == [c1,c2]
+ }
- assert [c1, c2].collect(cl) == [c1,c2]
+ @Test
+ void testMethodClosure2() {
+ def closure = String.&toUpperCase // Class.instanceMethod
+ assert closure instanceof Closure
+ assert closure instanceof MethodClosure
+ assert ["xx", "yy"].collect(closure) == ["XX","YY"]
}
- void testStaticMethodAccess() {
- def list = [1].collect (this.&bb)
+ @Test
+ void testStaticMethodClosure() {
+ def list = [1].collect(this.&bb)
assert list == [1]
- list = [1].collect (MethodClosureTest.&bb) // Class.staticMethod
+ list = [1].collect(MethodClosureTest.&bb) // Class.staticMethod
assert list == [1]
- def mct = new MethodClosureTest()
- list = [1].collect (mct.&bb) // instance.staticMethod
+ list = [1].collect(new MethodClosureTest().&bb) // instance.staticMethod
assert list == [1]
}
- void testShellVariable() {
+ @Test
+ void testShellVariableAccess() {
def shell = new GroovyShell()
assert shell.evaluate("x = String.&toUpperCase; x('abc')") == "ABC"
assert shell.evaluate("x = 'abc'.&toUpperCase; x()") == "ABC"
@@ -66,6 +73,7 @@ class MethodClosureTest extends GroovyTestCase {
assert shell.evaluate("x = 3.&parseInt; x('123')") == 123
}
+ @Test
void testMethodClosureWithCategory() {
assertScript '''
class Bar {
@@ -103,4 +111,16 @@ class MethodClosureTest extends GroovyTestCase {
}
'''
}
+
+ // GROOVY-10929
+ @Test
+ void testMethodClosureIllegalArgumentException() {
+ shouldFail IllegalArgumentException, '''
+ static create(Class type) {
+ throw new IllegalArgumentException("Class ${type.name} does not ...")
+ }
+ def closure = this.class.&create
+ closure(Object)
+ '''
+ }
}