You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/02/12 17:18:14 UTC

[groovy] 06/08: minor edits

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

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

commit 430c98193b7608580a45c20f79b46afb5f8aa373
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Feb 8 12:52:46 2020 -0600

    minor edits
    
    (cherry picked from commit d7cd86e8b6895379a76ab339021f1410e986b2fb)
---
 src/main/java/groovy/lang/GroovyObjectSupport.java |  19 ++-
 src/main/java/groovy/lang/ProxyMetaClass.java      |   2 +-
 src/main/java/groovy/util/BuilderSupport.java      | 140 ++++++++++-----------
 src/test/groovy/bugs/Groovy9387.groovy             |  53 +++++---
 4 files changed, 114 insertions(+), 100 deletions(-)

diff --git a/src/main/java/groovy/lang/GroovyObjectSupport.java b/src/main/java/groovy/lang/GroovyObjectSupport.java
index 57b2552..6c48cf3 100644
--- a/src/main/java/groovy/lang/GroovyObjectSupport.java
+++ b/src/main/java/groovy/lang/GroovyObjectSupport.java
@@ -20,27 +20,24 @@ package groovy.lang;
 
 import org.codehaus.groovy.runtime.InvokerHelper;
 
+import java.util.Optional;
+
 /**
- * A useful base class for Java objects wishing to be Groovy objects
+ * Base class for Java objects wishing to be Groovy objects.
  */
 public abstract class GroovyObjectSupport implements GroovyObject {
 
     // never persist the MetaClass
-    private transient MetaClass metaClass;
-
-    public GroovyObjectSupport() {
-        this.metaClass = getDefaultMetaClass();
-    }
+    private transient MetaClass metaClass = getDefaultMetaClass();
 
+    @Override
     public MetaClass getMetaClass() {
         return this.metaClass;
     }
 
-    public void setMetaClass(MetaClass metaClass) {
-        this.metaClass =
-                null == metaClass
-                        ? getDefaultMetaClass()
-                        : metaClass;
+    @Override
+    public void setMetaClass(/*@Nullable*/ final MetaClass metaClass) {
+        this.metaClass = Optional.ofNullable(metaClass).orElseGet(this::getDefaultMetaClass);
     }
 
     private MetaClass getDefaultMetaClass() {
diff --git a/src/main/java/groovy/lang/ProxyMetaClass.java b/src/main/java/groovy/lang/ProxyMetaClass.java
index 06dcf4f..e3d788d 100644
--- a/src/main/java/groovy/lang/ProxyMetaClass.java
+++ b/src/main/java/groovy/lang/ProxyMetaClass.java
@@ -213,7 +213,7 @@ public class ProxyMetaClass extends MetaClassImpl implements AdaptingMetaClass {
     }
 
     private Object doCall(final Object object, final String methodName, final Object[] arguments, final Interceptor interceptor, final Supplier<Object> howToInvoke) {
-        if (null == interceptor) {
+        if (interceptor == null) {
             return howToInvoke.get();
         }
         Object result = interceptor.beforeInvoke(object, methodName, arguments);
diff --git a/src/main/java/groovy/util/BuilderSupport.java b/src/main/java/groovy/util/BuilderSupport.java
index ceccd87..95bc051 100644
--- a/src/main/java/groovy/util/BuilderSupport.java
+++ b/src/main/java/groovy/util/BuilderSupport.java
@@ -22,13 +22,15 @@ import groovy.lang.Closure;
 import groovy.lang.GroovyObjectSupport;
 import groovy.lang.GroovyRuntimeException;
 import groovy.lang.MissingMethodException;
+import groovy.transform.stc.ClosureParams;
+import groovy.transform.stc.SimpleType;
 import org.codehaus.groovy.runtime.InvokerHelper;
 
 import java.util.List;
 import java.util.Map;
 
 /**
- * An abstract base class for creating arbitrary nested trees of objects or events
+ * Base class for creating arbitrary nested trees of objects or events.
  */
 public abstract class BuilderSupport extends GroovyObjectSupport {
 
@@ -40,94 +42,101 @@ public abstract class BuilderSupport extends GroovyObjectSupport {
         this.proxyBuilder = this;
     }
 
-    public BuilderSupport(BuilderSupport proxyBuilder) {
-        this(null, proxyBuilder);
+    public BuilderSupport(final BuilderSupport proxyBuilder) {
+        this.proxyBuilder = proxyBuilder;
     }
 
-    public BuilderSupport(Closure nameMappingClosure, BuilderSupport proxyBuilder) {
+    public BuilderSupport(@ClosureParams(value=SimpleType.class, options="java.lang.String") final Closure nameMappingClosure, final BuilderSupport proxyBuilder) {
         this.nameMappingClosure = nameMappingClosure;
         this.proxyBuilder = proxyBuilder;
     }
 
+    protected Object getCurrent() {
+        return current;
+    }
+
+    protected void setCurrent(final Object current) {
+        this.current = current;
+    }
+
     /**
-     * Convenience method when no arguments are required
+     * Convenience method when no arguments are required.
      *
      * @param methodName the name of the method to invoke
      * @return the result of the call
      */
-    public Object invokeMethod(String methodName) {
+    public Object invokeMethod(final String methodName) {
         return invokeMethod(methodName, null);
     }
 
-    public Object invokeMethod(String methodName, Object args) {
+    public Object invokeMethod(final String methodName, final Object args) {
         Object name = getName(methodName);
         return doInvokeMethod(methodName, name, args);
     }
 
-    protected Object doInvokeMethod(String methodName, Object name, Object args) {
+    protected Object doInvokeMethod(final String methodName, final Object name, final Object args) {
         Object node = null;
         Closure closure = null;
         List list = InvokerHelper.asList(args);
 
-        //System.out.println("Called invokeMethod with name: " + name + " arguments: " + list);
-
         switch (list.size()) {
             case 0:
                 node = proxyBuilder.createNode(name);
                 break;
-            case 1: {
-                Object object = list.get(0);
-                if (object instanceof Map) {
-                    node = proxyBuilder.createNode(name, (Map) object);
-                } else if (object instanceof Closure) {
-                    closure = (Closure) object;
-                    node = proxyBuilder.createNode(name);
-                } else {
-                    node = proxyBuilder.createNode(name, object);
+            case 1:
+                {
+                    Object object = list.get(0);
+                    if (object instanceof Map) {
+                        node = proxyBuilder.createNode(name, (Map) object);
+                    } else if (object instanceof Closure) {
+                        closure = (Closure) object;
+                        node = proxyBuilder.createNode(name);
+                    } else {
+                        node = proxyBuilder.createNode(name, object);
+                    }
                 }
-            }
-            break;
-            case 2: {
-                Object object1 = list.get(0);
-                Object object2 = list.get(1);
-                if (object1 instanceof Map) {
-                    if (object2 instanceof Closure) {
-                        closure = (Closure) object2;
-                        node = proxyBuilder.createNode(name, (Map) object1);
+                break;
+            case 2:
+                {
+                    Object object1 = list.get(0);
+                    Object object2 = list.get(1);
+                    if (object1 instanceof Map) {
+                        if (object2 instanceof Closure) {
+                            closure = (Closure) object2;
+                            node = proxyBuilder.createNode(name, (Map) object1);
+                        } else {
+                            node = proxyBuilder.createNode(name, (Map) object1, object2);
+                        }
                     } else {
-                        node = proxyBuilder.createNode(name, (Map) object1, object2);
+                        if (object2 instanceof Closure) {
+                            closure = (Closure) object2;
+                            node = proxyBuilder.createNode(name, object1);
+                        } else if (object2 instanceof Map) {
+                            node = proxyBuilder.createNode(name, (Map) object2, object1);
+                        } else {
+                            throw new MissingMethodException(name.toString(), getClass(), list.toArray(), false);
+                        }
                     }
-                } else {
-                    if (object2 instanceof Closure) {
-                        closure = (Closure) object2;
-                        node = proxyBuilder.createNode(name, object1);
-                    } else if (object2 instanceof Map) {
-                        node = proxyBuilder.createNode(name, (Map) object2, object1);
+                }
+                break;
+            case 3:
+                {
+                    Object arg0 = list.get(0);
+                    Object arg1 = list.get(1);
+                    Object arg2 = list.get(2);
+                    if (arg0 instanceof Map && arg2 instanceof Closure) {
+                        closure = (Closure) arg2;
+                        node = proxyBuilder.createNode(name, (Map) arg0, arg1);
+                    } else if (arg1 instanceof Map && arg2 instanceof Closure) {
+                        closure = (Closure) arg2;
+                        node = proxyBuilder.createNode(name, (Map) arg1, arg0);
                     } else {
                         throw new MissingMethodException(name.toString(), getClass(), list.toArray(), false);
                     }
                 }
-            }
-            break;
-            case 3: {
-                Object arg0 = list.get(0);
-                Object arg1 = list.get(1);
-                Object arg2 = list.get(2);
-                if (arg0 instanceof Map && arg2 instanceof Closure) {
-                    closure = (Closure) arg2;
-                    node = proxyBuilder.createNode(name, (Map) arg0, arg1);
-                } else if (arg1 instanceof Map && arg2 instanceof Closure) {
-                    closure = (Closure) arg2;
-                    node = proxyBuilder.createNode(name, (Map) arg1, arg0);
-                } else {
-                    throw new MissingMethodException(name.toString(), getClass(), list.toArray(), false);
-                }
-            }
-            break;
-            default: {
+                break;
+            default:
                 throw new MissingMethodException(name.toString(), getClass(), list.toArray(), false);
-            }
-
         }
 
         if (current != null) {
@@ -157,13 +166,13 @@ public abstract class BuilderSupport extends GroovyObjectSupport {
      * builder-trees and switch in different kinds of builders.
      * This method should call the setDelegate() method on the closure
      * which by default passes in this but if node is-a builder
-     * we could pass that in instead (or do something wacky too)
+     * we could pass that in instead (or do something wacky too).
      *
      * @param closure the closure on which to call setDelegate()
      * @param node    the node value that we've just created, which could be
      *                a builder
      */
-    protected void setClosureDelegate(Closure closure, Object node) {
+    protected void setClosureDelegate(final Closure closure, final Object node) {
         closure.setDelegate(this);
     }
 
@@ -184,14 +193,13 @@ public abstract class BuilderSupport extends GroovyObjectSupport {
      * @param methodName the name of the desired method
      * @return the object representing the name
      */
-    protected Object getName(String methodName) {
+    protected Object getName(final String methodName) {
         if (nameMappingClosure != null) {
             return nameMappingClosure.call(methodName);
         }
         return methodName;
     }
 
-
     /**
      * A hook to allow nodes to be processed once they have had all of their
      * children applied.
@@ -199,27 +207,19 @@ public abstract class BuilderSupport extends GroovyObjectSupport {
      * @param node   the current node being processed
      * @param parent the parent of the node being processed
      */
-    protected void nodeCompleted(Object parent, Object node) {
+    protected void nodeCompleted(final Object parent, final Object node) {
     }
 
     /**
      * A hook to allow nodes to be processed once they have had all of their
      * children applied and allows the actual node object that represents
-     * the Markup element to be changed
+     * the Markup element to be changed.
      *
      * @param node   the current node being processed
      * @param parent the parent of the node being processed
      * @return the node, possibly new, that represents the markup element
      */
-    protected Object postNodeCompletion(Object parent, Object node) {
+    protected Object postNodeCompletion(final Object parent, final Object node) {
         return node;
     }
-
-    protected Object getCurrent() {
-        return current;
-    }
-
-    protected void setCurrent(Object current) {
-        this.current = current;
-    }
 }
diff --git a/src/test/groovy/bugs/Groovy9387.groovy b/src/test/groovy/bugs/Groovy9387.groovy
index 53c4792..ef80987 100644
--- a/src/test/groovy/bugs/Groovy9387.groovy
+++ b/src/test/groovy/bugs/Groovy9387.groovy
@@ -27,18 +27,42 @@ import static groovy.test.GroovyAssert.assertScript
 @CompileStatic
 final class Groovy9387 {
 
-    @Test @NotYetImplemented
-    void testThisSetProperty() {
-        assertScript '''
-            class C extends BuilderSupport {
-                def createNode(a, b) {}
-                def createNode(a) {}
-                def createNode(a, Map b) {}
-                def createNode(a, Map b, c) {}
-                void setParent(a, b) {}
+    private static final String SUPPORT_ADAPTER = '''
+        class BuilderSupportAdapter extends BuilderSupport {
+            def createNode(a, b) {}
+            def createNode(a) {}
+            def createNode(a, Map b) {}
+            def createNode(a, Map b, c) {}
+            void setParent(a, b) {}
+        }
+    '''
 
+    @Test
+    void testThisPropertySet() {
+        assertScript SUPPORT_ADAPTER + '''
+            class C extends BuilderSupportAdapter {
                 String value = 'abc'
+                void test() {
+                    def b = { -> this.'value' = 'def' }
+                    b()
+                }
+            }
+
+            def c = new C()
+            def v = c.value
+            assert v == 'abc'
 
+            c.test()
+            v = c.value
+            assert v == 'def'
+        '''
+    }
+
+    @Test @NotYetImplemented
+    void testThisSetProperty() {
+        assertScript SUPPORT_ADAPTER + '''
+            class C extends BuilderSupportAdapter {
+                String value = 'abc'
                 void test() {
                     def b = { -> this.setProperty('value', 'def') }
                     b()
@@ -57,17 +81,10 @@ final class Groovy9387 {
 
     @Test
     void testThatSetProperty() {
-        assertScript '''
-            class C extends BuilderSupport {
-                def createNode(a, b) {}
-                def createNode(a) {}
-                def createNode(a, Map b) {}
-                def createNode(a, Map b, c) {}
-                void setParent(a, b) {}
-
+        assertScript SUPPORT_ADAPTER + '''
+            class C extends BuilderSupportAdapter {
                 private C that = this
                 String value = 'abc'
-
                 void test() {
                     def b = { -> that.setProperty('value', 'def') }
                     b()