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 2021/08/17 18:18:21 UTC

[groovy] branch GROOVY_3_0_X updated (ba00810 -> b35092d)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.


    from ba00810  GROOVY-10087: STC: char can be implicitly boxed to Character at return
     new 57482cd  GROOVY-3015, GROOVY-4610: add GroovyInterceptable check to closure resolve strategy
     new d7a3dc8  GROOVY-3421: evaluate spread-map value only once
     new 21b30bf  GROOVY-5453: order category methods before taking first getter/setter
     new b35092d  GROOVY-7867: collection conversion: attach exception of constructor as suppressed exception

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/main/java/groovy/lang/MetaClassImpl.java       |  69 ++++---------
 .../groovy/classgen/AsmClassGenerator.java         |  13 +--
 .../groovy/runtime/GroovyCategorySupport.java      |  25 ++---
 .../groovy/runtime/metaclass/ClosureMetaClass.java |  11 ++
 .../runtime/metaclass/TransformMetaMethod.java     |  17 ++-
 .../typehandling/DefaultTypeTransformation.java    |   6 ++
 src/test/groovy/CategoryTest.groovy                |  34 ++++--
 src/test/groovy/GroovyInterceptableTest.groovy     | 106 ++++++++++++++-----
 src/test/groovy/bugs/Groovy7867.groovy             | 114 +++++++++++++++++++++
 .../groovy/operator/SpreadMapOperatorTest.groovy   |  26 +++--
 10 files changed, 298 insertions(+), 123 deletions(-)
 create mode 100644 src/test/groovy/bugs/Groovy7867.groovy

[groovy] 01/04: GROOVY-3015, GROOVY-4610: add GroovyInterceptable check to closure resolve strategy

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 57482cd27a8d496ce381fb8932899ed7ef99c214
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue May 18 08:41:04 2021 -0500

    GROOVY-3015, GROOVY-4610: add GroovyInterceptable check to closure
    resolve strategy
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/runtime/metaclass/TransformMetaMethod.java
---
 .../groovy/runtime/metaclass/ClosureMetaClass.java |  11 +++
 .../runtime/metaclass/TransformMetaMethod.java     |  17 +++-
 src/test/groovy/GroovyInterceptableTest.groovy     | 106 ++++++++++++++++-----
 3 files changed, 106 insertions(+), 28 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
index fafb786..e9e3b57 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
@@ -20,6 +20,7 @@ package org.codehaus.groovy.runtime.metaclass;
 
 import groovy.lang.Closure;
 import groovy.lang.ExpandoMetaClass;
+import groovy.lang.GroovyInterceptable;
 import groovy.lang.GroovyObject;
 import groovy.lang.GroovyRuntimeException;
 import groovy.lang.MetaBeanProperty;
@@ -204,6 +205,16 @@ public final class ClosureMetaClass extends MetaClassImpl {
                 if (method != null) return method;
             }
             return null;
+        } else if (delegate instanceof GroovyInterceptable) {
+            MetaClass delegateMetaClass = lookupObjectMetaClass(delegate);
+            // GROOVY-3015: must route calls through GroovyObject#invokeMethod(String,Object)
+            MetaMethod interceptMethod = delegateMetaClass.pickMethod("invokeMethod", new Class[]{String.class, Object.class});
+            return new TransformMetaMethod(interceptMethod) {
+                @Override
+                public Object invoke(final Object object, final Object[] arguments) {
+                    return super.invoke(object, new Object[]{methodName, arguments});
+                }
+            };
         } else {
             MetaClass delegateMetaClass = lookupObjectMetaClass(delegate);
             MetaMethod method = delegateMetaClass.pickMethod(methodName, argClasses);
diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/TransformMetaMethod.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/TransformMetaMethod.java
index 408c53e..be0b288 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/TransformMetaMethod.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/TransformMetaMethod.java
@@ -25,10 +25,10 @@ import org.codehaus.groovy.reflection.CachedClass;
  * A MetaMethod implementation useful for implementing coercion based invocations
  */
 public class TransformMetaMethod extends MetaMethod {
-    
+
     private final MetaMethod metaMethod;
 
-    public TransformMetaMethod(MetaMethod metaMethod) {
+    public TransformMetaMethod(final MetaMethod metaMethod) {
         this.metaMethod = metaMethod;
         setParametersTypes(metaMethod.getParameterTypes());
         nativeParamTypes = metaMethod.getNativeParameterTypes();
@@ -50,7 +50,18 @@ public class TransformMetaMethod extends MetaMethod {
         return metaMethod.getDeclaringClass();
     }
 
-    public Object invoke(Object object, Object[] arguments) {
+    @Override
+    public Object invoke(final Object object, final Object[] arguments) {
         return metaMethod.invoke(object, arguments);
     }
+
+    @Override
+    public Object doMethodInvoke(final Object object, final Object[] arguments) {
+        // no coerceArgumentsToClasses
+        try {
+            return invoke(object, arguments);
+        } catch (final Exception ex) {
+            throw processDoMethodInvokeException(ex, object, arguments);
+        }
+    }
 }
diff --git a/src/test/groovy/GroovyInterceptableTest.groovy b/src/test/groovy/GroovyInterceptableTest.groovy
index a667bc0..0053a1d 100644
--- a/src/test/groovy/GroovyInterceptableTest.groovy
+++ b/src/test/groovy/GroovyInterceptableTest.groovy
@@ -21,44 +21,101 @@ package groovy
 import groovy.test.GroovyTestCase
 import org.codehaus.groovy.runtime.ReflectionMethodInvoker
 
-class GroovyInterceptableTest extends GroovyTestCase {
+final class GroovyInterceptableTest extends GroovyTestCase {
 
-    void testMethodInterception() {
+    void testMethodIntercept1() {
         def g = new GI()
         assert g.someInt() == 2806
         assert g.someUnexistingMethod() == 1
         assert g.toString() == "invokeMethodToString"
     }
 
-    void testProperties() {
+    void testMethodIntercept2() {
         def g = new GI()
         assert g.foo == 89
         g.foo = 90
         assert g.foo == 90
-        // should this be 1 or 90?
+        // Should this be 1 or 90?
         assert g.getFoo() == 1
     }
-    
-    void testCallMissingMethod() {
+
+    // GROOVY-3015
+    void testMethodIntercept3() {
+        String shared = '''\
+            import org.codehaus.groovy.runtime.InvokerHelper
+            import org.codehaus.groovy.runtime.StringBufferWriter
+            import static groovy.test.GroovyTestCase.assertEquals
+
+            class Traceable implements GroovyInterceptable {
+                private static int indent = 1
+                Writer writer = new PrintWriter(System.out)
+                Object invokeMethod(String name, Object args) {
+                    writer.write('\\n' + ('  ' * indent) + 'Enter ' + name)
+                    indent += 1
+                    def result = InvokerHelper.getMetaClass(this).invokeMethod(this, name, args)
+                    indent -= 1
+                    writer.write('\\n' + ('  ' * indent) + 'Leave ' + name)
+                    return result
+                }
+            }
+
+            class Whatever extends Traceable {
+                int inner() { return 1 }
+                int outer() { return inner() }
+                int shouldTraceOuterAndInnerMethod() { return outer() }
+                def shouldTraceOuterAndInnerClosure = { -> return outer() }
+            }
+
+            def log = new StringBuffer()
+            def obj = new Whatever(writer: new StringBufferWriter(log))
+        '''
+
+        assertScript shared + '''
+            obj.shouldTraceOuterAndInnerMethod()
+
+            assertEquals """
+            |  Enter shouldTraceOuterAndInnerMethod
+            |    Enter outer
+            |      Enter inner
+            |      Leave inner
+            |    Leave outer
+            |  Leave shouldTraceOuterAndInnerMethod""".stripMargin(), log.toString()
+        '''
+
+        assertScript shared + '''
+            obj.shouldTraceOuterAndInnerClosure()
+
+            assertEquals """
+            |  Enter shouldTraceOuterAndInnerClosure
+            |    Enter outer
+            |      Enter inner
+            |      Leave inner
+            |    Leave outer
+            |  Leave shouldTraceOuterAndInnerClosure""".stripMargin(), log.toString()
+        '''
+    }
+
+    void testMissingMethod1() {
         def obj = new GI2()
         shouldFail { obj.notAMethod() }
-        assert 'missing' == obj.result 
+        assert 'missing' == obj.result
     }
- 
-    void testCallMissingMethodFromInstance() {
+
+    void testMissingMethod2() {
         def obj = new GI2()
         shouldFail { obj.method() }
         assert 'missing' == obj.result
-   }
+    }
 }
 
-class GI implements GroovyInterceptable {
+//------------------------------------------------------------------------------
 
+class GI implements GroovyInterceptable {
     def foo = 89
-
     int someInt() { 2806 }
+    @Override
     String toString() { "originalToString" }
-
+    @Override
     Object invokeMethod(String name, Object args) {
         if ("toString" == name)
             return "invokeMethodToString"
@@ -69,17 +126,16 @@ class GI implements GroovyInterceptable {
     }
 }
 
-
 class GI2 implements GroovyInterceptable {
-  def result = ""
-  def invokeMethod(String name, args) {
-    def metaMethod = Foo.metaClass.getMetaMethod(name, args)
-    if (metaMethod != null) return metaMethod.invoke(this, args)
-    result += "missing"
-    throw new MissingMethodException(name, Foo.class, args)
-  }
-  
-  def method() {
-      notAMethod()
-  }
+    def result = ""
+    @Override
+    def invokeMethod(String name, args) {
+        def metaMethod = Foo.metaClass.getMetaMethod(name, args)
+        if (metaMethod != null) return metaMethod.invoke(this, args)
+        result += "missing"
+        throw new MissingMethodException(name, Foo.class, args)
+    }
+    def method() {
+        notAMethod()
+    }
 }

[groovy] 03/04: GROOVY-5453: order category methods before taking first getter/setter

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 21b30bf690af3bf7558b140a0bf10f775e2d66f2
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jun 3 15:28:00 2021 -0500

    GROOVY-5453: order category methods before taking first getter/setter
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java
---
 src/main/java/groovy/lang/MetaClassImpl.java       | 69 ++++++----------------
 .../groovy/runtime/GroovyCategorySupport.java      | 25 +++-----
 src/test/groovy/CategoryTest.groovy                | 34 +++++++----
 3 files changed, 51 insertions(+), 77 deletions(-)

diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 1b71bc0..7742b8b 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2158,61 +2158,30 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
         return tuple(method, mp);
     }
 
-    private static MetaMethod getCategoryMethodMissing(Class sender) {
-        List possibleGenericMethods = GroovyCategorySupport.getCategoryMethods(METHOD_MISSING);
-        if (possibleGenericMethods != null) {
-            for (Object possibleGenericMethod : possibleGenericMethods) {
-                MetaMethod mmethod = (MetaMethod) possibleGenericMethod;
-                if (!mmethod.getDeclaringClass().getTheClass().isAssignableFrom(sender))
-                    continue;
-
-                CachedClass[] paramTypes = mmethod.getParameterTypes();
-                if (paramTypes.length == 2 && paramTypes[0].getTheClass() == String.class) {
-                    return mmethod;
-                }
-            }
-        }
-        return null;
+    private static CategoryMethod getCategoryMethodMissing(final Class<?> sender) {
+        return findCategoryMethod(METHOD_MISSING, sender, params ->
+            params.length == 2 && params[0].getTheClass() == String.class
+        );
     }
 
-    private static MetaMethod getCategoryMethodGetter(Class sender, String name, boolean useLongVersion) {
-        List possibleGenericMethods = GroovyCategorySupport.getCategoryMethods(name);
-        if (possibleGenericMethods != null) {
-            for (Object possibleGenericMethod : possibleGenericMethods) {
-                MetaMethod mmethod = (MetaMethod) possibleGenericMethod;
-                if (!mmethod.getDeclaringClass().getTheClass().isAssignableFrom(sender))
-                    continue;
-
-                CachedClass[] paramTypes = mmethod.getParameterTypes();
-                if (useLongVersion) {
-                    if (paramTypes.length == 1 && paramTypes[0].getTheClass() == String.class) {
-                        return mmethod;
-                    }
-                } else {
-                    if (paramTypes.length == 0) return mmethod;
-                }
-            }
-        }
-        return null;
+    private static CategoryMethod getCategoryMethodGetter(final Class<?> sender, final String name, final boolean useLongVersion) {
+        return findCategoryMethod(name, sender, params ->
+            useLongVersion ? params.length == 1 && params[0].getTheClass() == String.class : params.length == 0
+        );
     }
 
-    private static MetaMethod getCategoryMethodSetter(Class sender, String name, boolean useLongVersion) {
-        List possibleGenericMethods = GroovyCategorySupport.getCategoryMethods(name);
-        if (possibleGenericMethods != null) {
-            for (Object possibleGenericMethod : possibleGenericMethods) {
-                MetaMethod mmethod = (MetaMethod) possibleGenericMethod;
-                if (!mmethod.getDeclaringClass().getTheClass().isAssignableFrom(sender))
-                    continue;
+    private static CategoryMethod getCategoryMethodSetter(final Class<?> sender, final String name, final boolean useLongVersion) {
+        return findCategoryMethod(name, sender, params ->
+            useLongVersion ? params.length == 2 && params[0].getTheClass() == String.class : params.length == 1
+        );
+    }
 
-                CachedClass[] paramTypes = mmethod.getParameterTypes();
-                if (useLongVersion) {
-                    if (paramTypes.length == 2 && paramTypes[0].getTheClass() == String.class) {
-                        return mmethod;
-                    }
-                } else {
-                    if (paramTypes.length == 1) return mmethod;
-                }
-            }
+    private static CategoryMethod findCategoryMethod(final String name, final Class<?> sender, final java.util.function.Predicate<CachedClass[]> paramFilter) {
+        List<CategoryMethod> categoryMethods = GroovyCategorySupport.getCategoryMethods(name);
+        if (categoryMethods != null) {
+            return categoryMethods.stream().filter(categoryMethod ->
+                categoryMethod.getDeclaringClass().isAssignableFrom(sender) && paramFilter.test(categoryMethod.getParameterTypes())
+            ).sorted().findFirst().orElse(null);
         }
         return null;
     }
diff --git a/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java b/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java
index c96780f..ff872fc 100644
--- a/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java
+++ b/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java
@@ -228,27 +228,18 @@ public class GroovyCategorySupport {
         /**
          * Sort by most specific to least specific.
          *
-         * @param o the object to compare against
+         * @param that the object to compare against
          */
-        public int compareTo(Object o) {
-            CategoryMethod thatMethod = (CategoryMethod) o;
+        @Override
+        public int compareTo(final Object that) {
             Class thisClass = metaClass;
-            Class thatClass = thatMethod.metaClass;
+            Class thatClass = ((CategoryMethod) that).metaClass;
+
             if (thisClass == thatClass) return 0;
-            if (isChildOfParent(thisClass, thatClass)) return -1;
-            if (isChildOfParent(thatClass, thisClass)) return 1;
-            return 0;
-        }
+            if (thisClass.isAssignableFrom(thatClass)) return 1;
+            if (thatClass.isAssignableFrom(thisClass)) return -1;
 
-        private boolean isChildOfParent(Class candidateChild, Class candidateParent) {
-            Class loop = candidateChild;
-            while(loop != null && loop != Object.class) {
-                loop = loop.getSuperclass();
-                if (loop == candidateParent) {
-                    return true;
-                }
-            }
-            return false;
+            return 0;
         }
     }
 
diff --git a/src/test/groovy/CategoryTest.groovy b/src/test/groovy/CategoryTest.groovy
index 1dbbb99..2a7f73f 100644
--- a/src/test/groovy/CategoryTest.groovy
+++ b/src/test/groovy/CategoryTest.groovy
@@ -180,15 +180,15 @@ final class CategoryTest extends GroovyTestCase {
         """
     }
 
-
     def foo(x){x.bar()}
+
     void testMethodHiding1() {
         def x = new X()
         assert foo(x) == 1
         use (XCat) {
-	        assert foo(x) == 2
-	        def t = Thread.start {assert foo(x)==1}
-	        t.join()
+            assert foo(x) == 2
+            def t = Thread.start {assert foo(x)==1}
+            t.join()
         }
         assert foo(x) == 1
         def t = Thread.start {use (XCat2){assert foo(x)==3}}
@@ -200,12 +200,12 @@ final class CategoryTest extends GroovyTestCase {
         def x = new X()
         assert foo(x) == 1
         use (XCat) {
-	        assert foo(x) == 2
-        	def t = Thread.start {use (XCat2){assert foo(x)==3}}
-        	t.join()
-        	assert foo(x) == 2
-	        t = Thread.start {assert foo(x)==1}
-	        t.join()
+            assert foo(x) == 2
+            def t = Thread.start {use (XCat2){assert foo(x)==3}}
+            t.join()
+            assert foo(x) == 2
+            t = Thread.start {assert foo(x)==1}
+            t.join()
         }
         assert foo(x) == 1
         def t = Thread.start {use (XCat2){assert foo(x)==3}}
@@ -248,6 +248,20 @@ final class CategoryTest extends GroovyTestCase {
         '''
     }
 
+    // GROOVY-5453
+    void testOverloadedGetterMethod() {
+        assertScript '''
+            class Cat {
+                static getFoo(String s) {'String'}
+                static getFoo(CharSequence s) {'CharSequence'}
+            }
+            use (Cat) {
+                assert 'abc'.getFoo() == 'String'
+                assert 'abc'.foo      == 'String'
+            }
+        '''
+    }
+
     // GROOVY-3867
     void testPropertyMissing() {
         def x = new X()

[groovy] 02/04: GROOVY-3421: evaluate spread-map value only once

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit d7a3dc8f72fef552df16bb50fbb1ececaa885645
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 6 17:09:36 2021 -0500

    GROOVY-3421: evaluate spread-map value only once
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
---
 .../groovy/classgen/AsmClassGenerator.java         | 13 ++++-------
 .../groovy/operator/SpreadMapOperatorTest.groovy   | 26 ++++++++++++++--------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index e0530be..22a677e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -116,6 +116,7 @@ import java.io.PrintWriter;
 import java.io.Writer;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -764,15 +765,9 @@ public class AsmClassGenerator extends ClassGenerator {
 
     @Override
     public void visitSpreadMapExpression(final SpreadMapExpression expression) {
-        Expression subExpression = expression.getExpression();
-        // to not record the underlying MapExpression twice,
-        // we disable the assertion tracker
-        // see https://issues.apache.org/jira/browse/GROOVY-3421
-        controller.getAssertionWriter().disableTracker();
-        subExpression.visit(this);
-        controller.getOperandStack().box();
-        spreadMap.call(controller.getMethodVisitor());
-        controller.getAssertionWriter().reenableTracker();
+        // GROOVY-3421: SpreadMapExpression is key expression and contains value
+        callX(ClassHelper.make(Collections.class), "emptyMap").visit(this);
+        spreadMap.call(controller.getMethodVisitor()); // dummy SpreadMap
         controller.getOperandStack().replace(ClassHelper.OBJECT_TYPE);
     }
 
diff --git a/src/test/groovy/operator/SpreadMapOperatorTest.groovy b/src/test/groovy/operator/SpreadMapOperatorTest.groovy
index 855f71d..47f1632 100644
--- a/src/test/groovy/operator/SpreadMapOperatorTest.groovy
+++ b/src/test/groovy/operator/SpreadMapOperatorTest.groovy
@@ -100,18 +100,27 @@ class SpreadMapOperatorTest extends GroovyTestCase {
 
     void testSpecialSpreadMapIndexNotation() {
         assertScript '''
-        @groovy.transform.ToString
-        class Person { String name; int age }
+            @groovy.transform.ToString
+            class Person { String name; int age }
 
-        assert Person[ name:'Dave', age:32 ].toString() == 'Person(Dave, 32)'
+            assert Person[ name:'Dave', age:32 ].toString() == 'Person(Dave, 32)'
 
-        def timMap = [ name:'Tim', age:49 ]
-        assert Person[ *:timMap ].toString() == 'Person(Tim, 49)'
+            def timMap = [ name:'Tim', age:49 ]
+            assert Person[ *:timMap ].toString() == 'Person(Tim, 49)'
 
-        assert Person[ *:[ name:'John', age:29 ] ].toString() == 'Person(John, 29)'
+            assert Person[ *:[ name:'John', age:29 ] ].toString() == 'Person(John, 29)'
 
-        def ppl = [ [ name:'Tim', age:49 ], [ name:'Dave', age:32 ], [ name:'Steve', age:18 ] ]
-        assert ppl.collect { Person [ *:it ] }*.age == [49, 32, 18]
+            def ppl = [ [ name:'Tim', age:49 ], [ name:'Dave', age:32 ], [ name:'Steve', age:18 ] ]
+            assert ppl.collect { Person [ *:it ] }*.age == [49, 32, 18]
+        '''
+    }
+
+    // GROOVY-3421
+    void testSpreadMapSingleEval() {
+        assertScript '''
+            int i = 1
+            Map map = [a:i, *:[b:++i]]
+            assert map == [a: 1, b: 2]
         '''
     }
 
@@ -131,4 +140,3 @@ class SpreadMapOperatorTest extends GroovyTestCase {
         // Call with one usual argument, one named argument, one spread list argument, and one spread map argument
     }
 }
-

[groovy] 04/04: GROOVY-7867: collection conversion: attach exception of constructor as suppressed exception

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit b35092d1c446fdd650e567092734b746b4f8fa72
Author: nineninesevenfour <75...@users.noreply.github.com>
AuthorDate: Mon Jul 12 16:30:49 2021 +0200

    GROOVY-7867: collection conversion: attach exception of constructor as suppressed exception
    
    https://issues.apache.org/jira/browse/GROOVY-7867
    
    Signed-off-by: Harald Fassler <ha...@gmail.com>
---
 .../typehandling/DefaultTypeTransformation.java    |   6 ++
 src/test/groovy/bugs/Groovy7867.groovy             | 114 +++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java b/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
index 36264af..aa65f33 100644
--- a/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
+++ b/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
@@ -370,6 +370,7 @@ public class DefaultTypeTransformation {
         }
 
         Exception nested = null;
+        Exception suppressed = null;
         if (args != null) {
             try {
                 return InvokerHelper.invokeConstructorOf(type, args);
@@ -386,6 +387,8 @@ public class DefaultTypeTransformation {
                         // as the caller has more context to be able to throw a more
                         // meaningful exception (but stash to get message later)
                         nested = e;
+                        // keep the original exception as suppressed exception to allow easier failure analysis
+                        suppressed = ex;
                     }
                 } else {
                     nested = e;
@@ -404,6 +407,9 @@ public class DefaultTypeTransformation {
         } else {
             gce = new GroovyCastException(object, type);
         }
+        if (suppressed != null) {
+            gce.addSuppressed(suppressed);
+        }
         throw gce;
     }
 
diff --git a/src/test/groovy/bugs/Groovy7867.groovy b/src/test/groovy/bugs/Groovy7867.groovy
new file mode 100644
index 0000000..536ea28
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7867.groovy
@@ -0,0 +1,114 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+/**
+ * Test changes related to https://issues.apache.org/jira/browse/GROOVY-7867
+ */
+final class Groovy7867 {
+
+    @Test
+    void testIntendedBehaviour() {
+        // coerce calling constructor with two integers by throwing an Exception
+        assertScript '''
+            class SingletonList extends Vector {
+                SingletonList(Collection c) {
+                    super(c)
+                    if (c.size() != 1) throw new IllegalStateException()
+                }
+                SingletonList(int capacity, int increment) {
+                    super(capacity, increment)
+                }
+            }
+            
+            def myList = [10, 5] as SingletonList
+            assert myList.size() == 0
+            assert myList.capacity() == 10
+        '''
+    }
+
+    @Test
+    void testRetrievingSuppressedException() {
+        // for easier failure analysis it can be crucial to get hold of the original exception
+        assertScript '''
+            import org.codehaus.groovy.runtime.typehandling.GroovyCastException
+            
+            class SingletonList extends Vector {
+                SingletonList(Collection c) {
+                    super(c)
+                    if (c.size() != 1) {
+                        throw new IllegalArgumentException("expected SingletonList to be initialized with exactly one element")
+                    }
+                }
+            }
+            
+            // not exactly one argument --> (user defined) exception
+            boolean caught = false
+            try {
+                def myList = [10, 5] as SingletonList
+            } catch (ex) {
+                caught = true
+                assert ex instanceof GroovyCastException
+                assert ex.suppressed.length == 1
+                assert ex.suppressed[0] instanceof IllegalArgumentException
+                assert ex.suppressed[0].message == "expected SingletonList to be initialized with exactly one element"
+            }
+            assert caught == true
+            
+            // exactly one argument --> OK
+            caught = false
+            try {
+                def myList = [42] as SingletonList
+                assert myList.size() == 1
+                assert myList[0] == 42
+            } catch (ex) {
+                caught = true
+            }
+            assert caught == false
+        '''
+    }
+
+    @Test
+    void testFallbackToNoArgsConstructor() {
+        // the no-arg constructor is the second fallback which comes before calling the constructor with two integers
+        assertScript '''
+            class SingletonList extends Vector {
+                SingletonList() {
+                    super()
+                }
+                SingletonList(Collection c) {
+                    super(c)
+                    if (c.size() != 1) throw new IllegalStateException()
+                }
+                SingletonList(int capacity, int increment) {
+                    super(capacity, increment)
+                }
+            }
+            
+            def myList = [10, 5] as SingletonList
+            assert myList.size() == 2
+            assert myList[0] == 10
+            assert myList[1] == 5
+        '''
+    }
+}