[1/4] groovy git commit: GROOVY-8252: AIOOBE in combination with ncurry and rcurry (closes #612)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[1/4] groovy git commit: GROOVY-8252: AIOOBE in combination with ncurry and rcurry (closes #612)

paulk
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_5_X be084b84d -> 683f7ed84


GROOVY-8252: AIOOBE in combination with ncurry and rcurry (closes #612)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/683f7ed8
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/683f7ed8
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/683f7ed8

Branch: refs/heads/GROOVY_2_5_X
Commit: 683f7ed843ae74a7e6e5e54775b563db2fe51286
Parents: f64f5e2
Author: paulk <[hidden email]>
Authored: Mon Oct 2 11:29:43 2017 +1000
Committer: paulk <[hidden email]>
Committed: Mon Oct 9 16:09:44 2017 +1000

----------------------------------------------------------------------
 src/main/groovy/lang/Closure.java               |  8 ++++++
 .../codehaus/groovy/runtime/CurriedClosure.java | 24 +++++++++++++++--
 src/test/groovy/ClosureCurryTest.groovy         | 28 +++++++++++++++++++-
 3 files changed, 57 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/683f7ed8/src/main/groovy/lang/Closure.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/Closure.java b/src/main/groovy/lang/Closure.java
index e6bd284..9e3d432 100644
--- a/src/main/groovy/lang/Closure.java
+++ b/src/main/groovy/lang/Closure.java
@@ -556,6 +556,10 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
      * assert halver(8) == 4
      * </pre>
      *
+     * The position of the curried parameters will be calculated lazily, for example,
+     * if two overloaded doCall methods are available, the supplied arguments plus the
+     * curried arguments will be concatenated and the result used for method selection.
+     *
      * @param arguments the arguments to bind
      * @return the new closure with its arguments bound
      * @see #curry(Object...)
@@ -600,6 +604,10 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
      * // [BEE, Cat, ant, dog]  Not found but would belong in position 3
      * </pre>
      *
+     * The position of the curried parameters will be calculated eagerly
+     * and implies all arguments prior to the specified n index are supplied.
+     * Default parameter values prior to the n index will not be available.
+     *
      * @param n the index from which to bind parameters (may be -ve in which case it will be normalized)
      * @param arguments the arguments to bind
      * @return the new closure with its arguments bound

http://git-wip-us.apache.org/repos/asf/groovy/blob/683f7ed8/src/main/org/codehaus/groovy/runtime/CurriedClosure.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/CurriedClosure.java b/src/main/org/codehaus/groovy/runtime/CurriedClosure.java
index 1d2d876..22a83cb 100644
--- a/src/main/org/codehaus/groovy/runtime/CurriedClosure.java
+++ b/src/main/org/codehaus/groovy/runtime/CurriedClosure.java
@@ -39,15 +39,25 @@ import groovy.lang.Closure;
  * assert new CurriedClosure(unitAdder, "six", "ty")("minutes") == "sixty minutes"
  * </pre>
  *
- * @author Jochen Theodorou
- * @author Paul King
+ * Notes:
+ * <ul>
+ *     <li>Caters for Groovy's lazy (rcurry) and eager (ncurry) calculation of argument position</li>
+ * </ul>
  */
 public final class CurriedClosure<V> extends Closure<V> {
 
     private final Object[] curriedParams;
+    private final int minParamsExpected;
     private int index;
     private Class varargType = null;
 
+    /**
+     * Creates the curried closure.
+     *
+     * @param index the position where the parameters should be injected (-ve for lazy)
+     * @param uncurriedClosure the closure to be called after the curried parameters are injected
+     * @param arguments the supplied parameters
+     */
     public CurriedClosure(int index, Closure<V> uncurriedClosure, Object... arguments) {
         super(uncurriedClosure.clone());
         curriedParams = arguments;
@@ -65,6 +75,9 @@ public final class CurriedClosure<V> extends Closure<V> {
             if (index < 0) {
                 // normalise
                 this.index += origMaxLen;
+                minParamsExpected = 0;
+            } else {
+                minParamsExpected = index + arguments.length;
             }
             if (maximumNumberOfParameters < 0) {
                 throw new IllegalArgumentException("Can't curry " + arguments.length + " arguments for a closure with " + origMaxLen + " parameters.");
@@ -77,6 +90,8 @@ public final class CurriedClosure<V> extends Closure<V> {
                 throw new IllegalArgumentException("To curry " + arguments.length + " argument(s) expect index range 0.." +
                         maximumNumberOfParameters + " but found " + index);
             }
+        } else {
+            minParamsExpected = 0;
         }
     }
 
@@ -98,8 +113,13 @@ public final class CurriedClosure<V> extends Closure<V> {
                 System.arraycopy(arguments, normalizedIndex, newCurriedParams, curriedParams.length + normalizedIndex, arguments.length - normalizedIndex);
             return newCurriedParams;
         }
+        if (curriedParams.length + arguments.length < minParamsExpected) {
+            throw new IllegalArgumentException("When currying expected at least " + index + " argument(s) to be supplied before known curried arguments but found " + arguments.length);
+        }
         final Object newCurriedParams[] = new Object[curriedParams.length + arguments.length];
         int newIndex = Math.min(index, curriedParams.length + arguments.length - 1);
+        // rcurried arguments are done lazily to allow normal method selection between overloaded alternatives
+        newIndex = Math.min(newIndex, arguments.length);
         System.arraycopy(arguments, 0, newCurriedParams, 0, newIndex);
         System.arraycopy(curriedParams, 0, newCurriedParams, newIndex, curriedParams.length);
         if (arguments.length - newIndex > 0)

http://git-wip-us.apache.org/repos/asf/groovy/blob/683f7ed8/src/test/groovy/ClosureCurryTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/ClosureCurryTest.groovy b/src/test/groovy/ClosureCurryTest.groovy
index fbd5d1d..5bdfb42 100644
--- a/src/test/groovy/ClosureCurryTest.groovy
+++ b/src/test/groovy/ClosureCurryTest.groovy
@@ -21,7 +21,7 @@ package groovy
 import org.codehaus.groovy.runtime.DefaultGroovyMethods
 
 /**
- * @author Hallvard Tr�tteberg
+ * Tests for curried closures
  */
 class ClosureCurryTest extends GroovyTestCase {
 
@@ -252,6 +252,7 @@ class ClosureCurryTest extends GroovyTestCase {
     private a(b,c){ "2Obj: $b $c" }
     private a(b){ "1Obj: $b" }
 
+
     void testCurryWithMethodClosure() {
         def c = (this.&a).curry(0)
         assert c(1,2) == '3Int: 0 1 2'
@@ -265,4 +266,29 @@ class ClosureCurryTest extends GroovyTestCase {
         assert b(1) == '2Obj: 1 0'
         assert b() == '1Obj: 0'
     }
+
+    void testInsufficientSuppliedArgsWhenUsingNCurry() {
+        def cl = { a, b, c = 'x' -> a + b + c }
+        assert cl.ncurry(1, 'b')('a') == 'abx'
+        // ncurry is done eagerly and implies a minimum number of params
+        shouldFail(IllegalArgumentException) {
+            cl.ncurry(1, 'b')()
+        }
+        // rcurry is done lazily
+        assert cl.rcurry('b', 'c')('a') == 'abc'
+        assert cl.rcurry('b', 'c')() == 'bcx'
+    }
+
+    void testCurryInComboWithDefaultArgs() {
+        def cl = { a, b, c='c', d='d' -> a + b + c + d }
+        assert 'abcd' == cl.ncurry(0, 'a')('b')
+        assert 'xycd' == cl.ncurry(1, 'y')('x')
+        assert 'abdd' == cl.ncurry(0, 'a').rcurry('d')('b')
+        assert 'abcx' == cl.ncurry(3, 'x')('a', 'b', 'c')
+        shouldFail(IllegalArgumentException) {
+            // ncurry is done eagerly and implies a minimum number of params
+            // default arguments are ignored prior to the ncurried argument
+            cl.ncurry(4, 'd')('a', 'b')
+        }
+    }
 }

Reply | Threaded
Open this post in threaded view
|

[2/4] groovy git commit: GROOVY-8347: @AutoFinal: Added support to auto-finalize closure parameters (support more targets and inner classes)

paulk
GROOVY-8347: @AutoFinal: Added support to auto-finalize closure parameters (support more targets and inner classes)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/cea520ae
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/cea520ae
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/cea520ae

Branch: refs/heads/GROOVY_2_5_X
Commit: cea520ae782b5d4f02275b3cbbe285b87487294e
Parents: 3891c9e
Author: paulk <[hidden email]>
Authored: Sun Oct 8 23:43:39 2017 +1000
Committer: paulk <[hidden email]>
Committed: Mon Oct 9 16:09:44 2017 +1000

----------------------------------------------------------------------
 src/main/groovy/transform/AutoFinal.java        |  18 +--
 .../transform/AutoFinalASTTransformation.java   | 140 ++++++++++++++-----
 2 files changed, 114 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/cea520ae/src/main/groovy/transform/AutoFinal.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/transform/AutoFinal.java b/src/main/groovy/transform/AutoFinal.java
index 8fb5310..1cc0ab6 100644
--- a/src/main/groovy/transform/AutoFinal.java
+++ b/src/main/groovy/transform/AutoFinal.java
@@ -30,14 +30,16 @@ import java.lang.annotation.Target;
  * Annotation to automatically add the final qualifier to method, constructor,
  * and closure parameters.
  * <p>The annotation can be placed at the class level in which case it applies to
- * all methods, constructors, and closures within the class, or on individual
- * methods or constructors.
- * <p>In general it will make the most sense to automatically apply the
- * annotation to all classes of a project
- * (groovyc --configscript; google "Customising The Groovy Compiler", or see {@link CompilerConfiguration}
- * so that one can be sure that all arguments will automatically be final,
+ * all methods, constructors, and closures within the class. It can also be applied
+ * to an individual method, constructor, field with a Closure initial value
+ * or a Closure assigned to a local variable.
+ * <p>If you wish to automatically apply the
+ * annotation to all classes of a project, consider using
+ * {@code groovyc --configscript}. Google "Customising The Groovy Compiler",
+ * or see {@link CompilerConfiguration} for further details.
+ * This will ensure that all arguments will automatically be final,
  * completely eliminating the need to clutter the code with final keywords
- * in any paramete list.
+ * in any parameter list.
  * <p>
  * <em>Example usage:</em>
  * <pre class="groovyTestCase">
@@ -80,7 +82,7 @@ import java.lang.annotation.Target;
  */
 @java.lang.annotation.Documented
 @Retention(RetentionPolicy.SOURCE)
-@Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.LOCAL_VARIABLE})
 @GroovyASTTransformationClass("org.codehaus.groovy.transform.AutoFinalASTTransformation")
 public @interface AutoFinal {
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/cea520ae/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java b/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
index 20e8d51..4055f72 100644
--- a/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
@@ -19,12 +19,23 @@
 package org.codehaus.groovy.transform;
 
 import groovy.transform.AutoFinal;
-import org.codehaus.groovy.ast.*;
+import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.AnnotatedNode;
+import org.codehaus.groovy.ast.AnnotationNode;
+import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.ConstructorNode;
+import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.InnerClassNode;
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
+import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
 
 import java.lang.reflect.Modifier;
+import java.util.Iterator;
 
 import static org.codehaus.groovy.ast.ClassHelper.make;
 
@@ -37,72 +48,129 @@ public class AutoFinalASTTransformation extends AbstractASTTransformation {
     private static final Class MY_CLASS = AutoFinal.class;
     private static final ClassNode MY_TYPE = make(MY_CLASS);
     private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
+    private AnnotatedNode candidate;
+
 
     public void visit(ASTNode[] nodes, SourceUnit source) {
         init(nodes, source);
-        processClassesConstructorsMethods(nodes, source);
-        processClosures(nodes, source);
+        final ClassCodeVisitorSupport visitor = createVisitor();
+        process(nodes, visitor);
     }
 
-    private void processClassesConstructorsMethods(ASTNode[] nodes, final SourceUnit unit) {
-        AnnotatedNode candidate = (AnnotatedNode) nodes[1];
+    private ClassCodeVisitorSupport createVisitor() {
+        return new ClassCodeVisitorSupport() {
+            @Override
+            public void visitClosureExpression(ClosureExpression expression) {
+                if (expression.isSynthetic()) {
+                    return;
+                }
+                Parameter[] origParams = expression.getParameters();
+                for (Parameter p : origParams) {
+                    p.setModifiers(p.getModifiers() | Modifier.FINAL);
+                }
+                super.visitClosureExpression(expression);
+            }
+
+            @Override
+            protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) {
+                if (hasNoExplicitAutoFinal(node) || candidate == node) {
+                    super.visitConstructorOrMethod(node, isConstructor);
+                }
+            }
+
+            @Override
+            public void visitField(FieldNode node) {
+                if (hasNoExplicitAutoFinal(node) || candidate == node) {
+                    super.visitField(node);
+                }
+            }
+
+            @Override
+            public void visitDeclarationExpression(DeclarationExpression expr) {
+                if (hasNoExplicitAutoFinal(expr) || candidate == expr) {
+                    super.visitDeclarationExpression(expr);
+                }
+            }
+
+            protected SourceUnit getSourceUnit() {
+                return sourceUnit;
+            }
+        };
+    }
+
+    private void process(ASTNode[] nodes, final ClassCodeVisitorSupport visitor) {
+        candidate = (AnnotatedNode) nodes[1];
         AnnotationNode node = (AnnotationNode) nodes[0];
         if (!MY_TYPE.equals(node.getClassNode())) return;
 
         if (candidate instanceof ClassNode) {
-            processClass((ClassNode) candidate);
+            ClassNode cNode = (ClassNode) candidate;
+            processClass(cNode, visitor);
         } else if (candidate instanceof MethodNode) {
             // handles constructors and methods
-            processConstructorOrMethod((MethodNode) candidate);
+            MethodNode mNode = (MethodNode) candidate;
+            processConstructorOrMethod(mNode, visitor);
+        } else if (candidate instanceof FieldNode) {
+            FieldNode fNode = (FieldNode) candidate;
+            processField(fNode, visitor);
+        } else if (candidate instanceof DeclarationExpression) {
+            DeclarationExpression de = (DeclarationExpression) candidate;
+            processLocalVariable(de, visitor);
         }
     }
 
-    private void processClosures(ASTNode[] nodes, final SourceUnit source) {
-        final ASTNode node = nodes[1];
-        if(node instanceof ClassNode) {
-            ClassNode annotatedClass = (ClassNode) node;
-
-            final ClassCodeVisitorSupport visitor = new ClassCodeVisitorSupport() {
-                @Override
-                public void visitClosureExpression(ClosureExpression expression) {
-                    if (expression.isSynthetic()) { return; }
-                    Parameter[] origParams = expression.getParameters();
-                    for (Parameter p : origParams) {
-                        p.setModifiers(p.getModifiers() | Modifier.FINAL);
-                    }
-                    super.visitClosureExpression(expression);
-                }
-
-                protected SourceUnit getSourceUnit() {
-                    return source;
-                }
-            };
-
-            visitor.visitClass(annotatedClass);
+    private void processLocalVariable(DeclarationExpression de, ClassCodeVisitorSupport visitor) {
+        if (de.getRightExpression() instanceof ClosureExpression) {
+            visitor.visitDeclarationExpression(de);
         }
     }
 
+    private void processField(FieldNode fNode, ClassCodeVisitorSupport visitor) {
+        if (fNode.hasInitialExpression() && fNode.getInitialExpression() instanceof ClosureExpression) {
+            visitor.visitField(fNode);
+        }
+    }
 
-
-    private void processClass(ClassNode cNode) {
+    private void processClass(ClassNode cNode, final ClassCodeVisitorSupport visitor) {
         if (cNode.isInterface()) {
             addError("Error processing interface '" + cNode.getName() +
                     "'. " + MY_TYPE_NAME + " only allowed for classes.", cNode);
             return;
         }
+
         for (ConstructorNode cn : cNode.getDeclaredConstructors()) {
-            processConstructorOrMethod(cn);
+            if (hasNoExplicitAutoFinal(cn)) {
+                processConstructorOrMethod(cn, visitor);
+            }
         }
+
         for (MethodNode mn : cNode.getAllDeclaredMethods()) {
-            processConstructorOrMethod(mn);
+            if (hasNoExplicitAutoFinal(mn)) {
+                processConstructorOrMethod(mn, visitor);
+            }
+        }
+
+        Iterator<InnerClassNode> it = cNode.getInnerClasses();
+        while (it.hasNext()) {
+            InnerClassNode in = it.next();
+            if (in.getAnnotations(MY_TYPE).isEmpty()) {
+                processClass(in, visitor);
+            }
         }
+
+        visitor.visitClass(cNode);
+    }
+
+    private boolean hasNoExplicitAutoFinal(AnnotatedNode node) {
+        return node.getAnnotations(MY_TYPE).isEmpty();
     }
 
-    private void processConstructorOrMethod(MethodNode node) {
-        if (node.isSynthetic()) return;
-        Parameter[] origParams = node.getParameters();
+    private void processConstructorOrMethod(MethodNode mNode, ClassCodeVisitorSupport visitor) {
+        if (mNode.isSynthetic()) return;
+        Parameter[] origParams = mNode.getParameters();
         for (Parameter p : origParams) {
             p.setModifiers(p.getModifiers() | Modifier.FINAL);
         }
+        visitor.visitMethod(mNode);
     }
 }

Reply | Threaded
Open this post in threaded view
|

[3/4] groovy git commit: GROOVY-8347: @AutoFinal: Added support to auto-finalize closure parameters (allow disabling)

paulk
In reply to this post by paulk
GROOVY-8347: @AutoFinal: Added support to auto-finalize closure parameters (allow disabling)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/f64f5e21
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/f64f5e21
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/f64f5e21

Branch: refs/heads/GROOVY_2_5_X
Commit: f64f5e214c9c27b1ee4b0b847845be6e24cea457
Parents: cea520a
Author: paulk <[hidden email]>
Authored: Mon Oct 9 11:43:23 2017 +1000
Committer: paulk <[hidden email]>
Committed: Mon Oct 9 16:09:44 2017 +1000

----------------------------------------------------------------------
 src/main/groovy/transform/AutoFinal.java        | 17 +++++-
 .../transform/AutoFinalASTTransformation.java   | 61 ++++++++++++--------
 2 files changed, 52 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/f64f5e21/src/main/groovy/transform/AutoFinal.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/transform/AutoFinal.java b/src/main/groovy/transform/AutoFinal.java
index 1cc0ab6..58ce3ba 100644
--- a/src/main/groovy/transform/AutoFinal.java
+++ b/src/main/groovy/transform/AutoFinal.java
@@ -30,9 +30,11 @@ import java.lang.annotation.Target;
  * Annotation to automatically add the final qualifier to method, constructor,
  * and closure parameters.
  * <p>The annotation can be placed at the class level in which case it applies to
- * all methods, constructors, and closures within the class. It can also be applied
- * to an individual method, constructor, field with a Closure initial value
- * or a Closure assigned to a local variable.
+ * all methods, constructors, and closures within the class and any inner classes.
+ * It can also be applied to an individual method, constructor, field with a
+ * Closure initial value or a Closure assigned to a local variable. In the case
+ * of fields (or local variables) it only adjusts the parameters of the referenced
+ * Closure not the field (or local variable) itself.
  * <p>If you wish to automatically apply the
  * annotation to all classes of a project, consider using
  * {@code groovyc --configscript}. Google "Customising The Groovy Compiler",
@@ -85,4 +87,13 @@ import java.lang.annotation.Target;
 @Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.LOCAL_VARIABLE})
 @GroovyASTTransformationClass("org.codehaus.groovy.transform.AutoFinalASTTransformation")
 public @interface AutoFinal {
+    /**
+     * Indicates that adding final to parameters should not be applied on this node.
+     * <p>Normally not required since leaving off the annotation will achieve the same affect.
+     * However, it can be useful for selectively disabling this annotation in just a small part
+     * of an otherwise annotated class. As an example, it would make sense to set this to {@code false} on
+     * a method which altered parameters in a class already marked as {@code @AutoFinal}.
+     * All nodes in the class except that single method would be processed.
+     */
+    boolean enabled() default true;
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/f64f5e21/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java b/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
index 4055f72..3717d24 100644
--- a/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
@@ -36,6 +36,7 @@ import org.codehaus.groovy.control.SourceUnit;
 
 import java.lang.reflect.Modifier;
 import java.util.Iterator;
+import java.util.List;
 
 import static org.codehaus.groovy.ast.ClassHelper.make;
 
@@ -104,34 +105,18 @@ public class AutoFinalASTTransformation extends AbstractASTTransformation {
         if (!MY_TYPE.equals(node.getClassNode())) return;
 
         if (candidate instanceof ClassNode) {
-            ClassNode cNode = (ClassNode) candidate;
-            processClass(cNode, visitor);
+            processClass((ClassNode) candidate, visitor);
         } else if (candidate instanceof MethodNode) {
-            // handles constructors and methods
-            MethodNode mNode = (MethodNode) candidate;
-            processConstructorOrMethod(mNode, visitor);
+            processConstructorOrMethod((MethodNode) candidate, visitor);
         } else if (candidate instanceof FieldNode) {
-            FieldNode fNode = (FieldNode) candidate;
-            processField(fNode, visitor);
+            processField((FieldNode) candidate, visitor);
         } else if (candidate instanceof DeclarationExpression) {
-            DeclarationExpression de = (DeclarationExpression) candidate;
-            processLocalVariable(de, visitor);
-        }
-    }
-
-    private void processLocalVariable(DeclarationExpression de, ClassCodeVisitorSupport visitor) {
-        if (de.getRightExpression() instanceof ClosureExpression) {
-            visitor.visitDeclarationExpression(de);
-        }
-    }
-
-    private void processField(FieldNode fNode, ClassCodeVisitorSupport visitor) {
-        if (fNode.hasInitialExpression() && fNode.getInitialExpression() instanceof ClosureExpression) {
-            visitor.visitField(fNode);
+            processLocalVariable((DeclarationExpression) candidate, visitor);
         }
     }
 
     private void processClass(ClassNode cNode, final ClassCodeVisitorSupport visitor) {
+        if (!isEnabled(cNode)) return;
         if (cNode.isInterface()) {
             addError("Error processing interface '" + cNode.getName() +
                     "'. " + MY_TYPE_NAME + " only allowed for classes.", cNode);
@@ -161,11 +146,22 @@ public class AutoFinalASTTransformation extends AbstractASTTransformation {
         visitor.visitClass(cNode);
     }
 
-    private boolean hasNoExplicitAutoFinal(AnnotatedNode node) {
-        return node.getAnnotations(MY_TYPE).isEmpty();
+    private void processLocalVariable(DeclarationExpression de, ClassCodeVisitorSupport visitor) {
+        if (!isEnabled(de)) return;
+        if (de.getRightExpression() instanceof ClosureExpression) {
+            visitor.visitDeclarationExpression(de);
+        }
+    }
+
+    private void processField(FieldNode fNode, ClassCodeVisitorSupport visitor) {
+        if (!isEnabled(fNode)) return;
+        if (fNode.hasInitialExpression() && fNode.getInitialExpression() instanceof ClosureExpression) {
+            visitor.visitField(fNode);
+        }
     }
 
     private void processConstructorOrMethod(MethodNode mNode, ClassCodeVisitorSupport visitor) {
+        if (!isEnabled(mNode)) return;
         if (mNode.isSynthetic()) return;
         Parameter[] origParams = mNode.getParameters();
         for (Parameter p : origParams) {
@@ -173,4 +169,23 @@ public class AutoFinalASTTransformation extends AbstractASTTransformation {
         }
         visitor.visitMethod(mNode);
     }
+
+    private boolean isEnabled(final AnnotatedNode node) {
+        if (node == null) return false;
+        List<AnnotationNode> annotations = node.getAnnotations(MY_TYPE);
+        if (annotations != null) {
+            // any explicit false for enabled disables functionality
+            // this allows, for example, configscript to set all
+            // classes to true and one class to be explicitly disabled
+            for (AnnotationNode anno : annotations) {
+                // abort if explicit false found
+                if (memberHasValue(anno, "enabled", false)) return false;
+            }
+        }
+        return true;
+    }
+
+    private boolean hasNoExplicitAutoFinal(AnnotatedNode node) {
+        return node.getAnnotations(MY_TYPE).isEmpty();
+    }
 }

Reply | Threaded
Open this post in threaded view
|

[4/4] groovy git commit: GROOVY-8347: @AutoFinal: Added support to auto-finalize closure parameters (closes #613)

paulk
In reply to this post by paulk
GROOVY-8347: @AutoFinal: Added support to auto-finalize closure parameters (closes #613)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/3891c9ea
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/3891c9ea
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/3891c9ea

Branch: refs/heads/GROOVY_2_5_X
Commit: 3891c9eab1a19fd1be8dbee09de4e5817da42b7b
Parents: be084b8
Author: mgroovy <[hidden email]>
Authored: Sat Sep 23 04:33:58 2017 +0200
Committer: paulk <[hidden email]>
Committed: Mon Oct 9 16:09:44 2017 +1000

----------------------------------------------------------------------
 src/main/groovy/transform/AutoFinal.java        |  29 ++--
 .../transform/AutoFinalASTTransformation.java   |  43 ++++--
 .../AutoFinalTransformBlackBoxTest.groovy       | 139 +++++++++++++++++++
 3 files changed, 194 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/3891c9ea/src/main/groovy/transform/AutoFinal.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/transform/AutoFinal.java b/src/main/groovy/transform/AutoFinal.java
index 5fb673f..8fb5310 100644
--- a/src/main/groovy/transform/AutoFinal.java
+++ b/src/main/groovy/transform/AutoFinal.java
@@ -18,6 +18,7 @@
  */
 package groovy.transform;
 
+import org.codehaus.groovy.control.CompilerConfiguration;
 import org.codehaus.groovy.transform.GroovyASTTransformationClass;
 
 import java.lang.annotation.ElementType;
@@ -26,12 +27,17 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Annotation to automatically add final to various syntactic structures,
- * saving you typing of some boilerplate code.
- * Initially, only method and constructor parameters are supported.
- * The annotation may be placed on any method or constructor.
- * It can also be placed at the class level in which case it applies to
- * all methods and constructors within the class.
+ * Annotation to automatically add the final qualifier to method, constructor,
+ * and closure parameters.
+ * <p>The annotation can be placed at the class level in which case it applies to
+ * all methods, constructors, and closures within the class, or on individual
+ * methods or constructors.
+ * <p>In general it will make the most sense to automatically apply the
+ * annotation to all classes of a project
+ * (groovyc --configscript; google "Customising The Groovy Compiler", or see {@link CompilerConfiguration}
+ * so that one can be sure that all arguments will automatically be final,
+ * completely eliminating the need to clutter the code with final keywords
+ * in any paramete list.
  * <p>
  * <em>Example usage:</em>
  * <pre class="groovyTestCase">
@@ -43,11 +49,12 @@ import java.lang.annotation.Target;
  *         this.last = last
  *     }
  *     String fullName(boolean reversed = false, String separator = ' ') {
- *         "${reversed ? last : first}$separator${reversed ? first : last}"
+ *         final concatCls = { String n0, String n1 -> "$n0$separator$n1" }
+ *         concatCls(reversed ? last : first, reversed ? first : last)
  *     }
  * }
  *
- * def js = new Person('John', 'Smith')
+ * final js = new Person('John', 'Smith')
  * assert js.fullName() == 'John Smith'
  * assert js.fullName(true, ', ') == 'Smith, John'
  * </pre>
@@ -55,7 +62,7 @@ import java.lang.annotation.Target;
  * equivalent to the following code:
  * <pre>
  * Person(final String first, final String last) {
- *     //...
+ *   ...
  * }
  * </pre>
  * And after normal default parameter processing takes place, the following overloaded methods will exist:
@@ -64,6 +71,10 @@ import java.lang.annotation.Target;
  * String fullName(final boolean reversed) { fullName(reversed, ' ') }
  * String fullName() { fullName(false) }
  * </pre>
+ * and the closure will have become:
+ * <pre>
+ * { final String n0, final String n1 -> "$n0$separator$n1" }
+ * </pre>
  *
  * @since 2.5.0
  */

http://git-wip-us.apache.org/repos/asf/groovy/blob/3891c9ea/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java b/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
index 36fc7b1..20e8d51 100644
--- a/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/AutoFinalASTTransformation.java
@@ -19,13 +19,8 @@
 package org.codehaus.groovy.transform;
 
 import groovy.transform.AutoFinal;
-import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.AnnotatedNode;
-import org.codehaus.groovy.ast.AnnotationNode;
-import org.codehaus.groovy.ast.ClassNode;
-import org.codehaus.groovy.ast.ConstructorNode;
-import org.codehaus.groovy.ast.MethodNode;
-import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.*;
+import org.codehaus.groovy.ast.expr.ClosureExpression;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
 
@@ -34,7 +29,7 @@ import java.lang.reflect.Modifier;
 import static org.codehaus.groovy.ast.ClassHelper.make;
 
 /**
- * Handles generation of code for the {@code @}AutoFinal annotation.
+ * Handles generation of code for the {@link AutoFinal} annotation.
  */
 @GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
 public class AutoFinalASTTransformation extends AbstractASTTransformation {
@@ -45,6 +40,11 @@ public class AutoFinalASTTransformation extends AbstractASTTransformation {
 
     public void visit(ASTNode[] nodes, SourceUnit source) {
         init(nodes, source);
+        processClassesConstructorsMethods(nodes, source);
+        processClosures(nodes, source);
+    }
+
+    private void processClassesConstructorsMethods(ASTNode[] nodes, final SourceUnit unit) {
         AnnotatedNode candidate = (AnnotatedNode) nodes[1];
         AnnotationNode node = (AnnotationNode) nodes[0];
         if (!MY_TYPE.equals(node.getClassNode())) return;
@@ -57,6 +57,33 @@ public class AutoFinalASTTransformation extends AbstractASTTransformation {
         }
     }
 
+    private void processClosures(ASTNode[] nodes, final SourceUnit source) {
+        final ASTNode node = nodes[1];
+        if(node instanceof ClassNode) {
+            ClassNode annotatedClass = (ClassNode) node;
+
+            final ClassCodeVisitorSupport visitor = new ClassCodeVisitorSupport() {
+                @Override
+                public void visitClosureExpression(ClosureExpression expression) {
+                    if (expression.isSynthetic()) { return; }
+                    Parameter[] origParams = expression.getParameters();
+                    for (Parameter p : origParams) {
+                        p.setModifiers(p.getModifiers() | Modifier.FINAL);
+                    }
+                    super.visitClosureExpression(expression);
+                }
+
+                protected SourceUnit getSourceUnit() {
+                    return source;
+                }
+            };
+
+            visitor.visitClass(annotatedClass);
+        }
+    }
+
+
+
     private void processClass(ClassNode cNode) {
         if (cNode.isInterface()) {
             addError("Error processing interface '" + cNode.getName() +

http://git-wip-us.apache.org/repos/asf/groovy/blob/3891c9ea/src/test/org/codehaus/groovy/transform/AutoFinalTransformBlackBoxTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/transform/AutoFinalTransformBlackBoxTest.groovy b/src/test/org/codehaus/groovy/transform/AutoFinalTransformBlackBoxTest.groovy
new file mode 100644
index 0000000..d9625ed
--- /dev/null
+++ b/src/test/org/codehaus/groovy/transform/AutoFinalTransformBlackBoxTest.groovy
@@ -0,0 +1,139 @@
+/*
+ *  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 org.codehaus.groovy.transform
+
+import gls.CompilableTestSupport
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+
+/**
+ * Tests for the {@code @AutoFinal} AST transform.
+ */
+
+// Execute single test:
+// gradlew :test --build-cache --tests org.codehaus.groovy.transform.AutoFinalTransformTest
+@RunWith(JUnit4)
+class AutoFinalTransformBlackBoxTest extends CompilableTestSupport {
+
+  @Test
+  void testAutoFinal_Closure() {
+    assertAutoFinalClassTestScript("param0", "String foo() { final cls = { String param0 -> param0 = 'abc'; param0 }; cls() }")
+  }
+
+  @Test
+  void testAutoFinal_ClosureInClosure() {
+    assertAutoFinalClassTestScript("param1", "String foo() { final cls0 = { String param0 -> final cls1 = { String param1 -> param1 = 'xyz'; param1 }; cls1() }; cls0() }")
+  }
+
+  @Test
+  void testAutoFinal_ClassMethod_Param0() {
+    assertAutoFinalClassTestScript("param0", "String foo(String param0, param1) {  param0 = 'abc'; param0 }")
+  }
+
+  @Test
+  void testAutoFinal_ClassMethod_Param1() {
+    assertAutoFinalClassTestScript("param1", "String foo(String param0, param1) {  param1 = new Object(); param1 }")
+  }
+
+  // Check default parameters are not negatively impacted by @AutoFinal
+  @Test
+  void testAutoFinalClassMethodDefaultParameters() {
+    final String classPart = """
+      String foo(String param0 = 'XyZ', param1 = Closure.IDENTITY) {
+        assert param0.equals('XyZ')
+        assert param1.is(Closure.IDENTITY)
+        return param0
+      }
+    """
+    final script = autoFinalTestScript(true, classPart, "final foo = new $autoFinalTestClassName(); foo.foo()" )
+    assert script.contains('@AutoFinal')
+    assertScript(script)
+  }
+
+
+
+
+  void assertAutoFinalClassTestScript(final String paramName, final String classPart) {
+    assertAutoFinalTestScriptWithAnnotation(paramName, classPart)
+    assertAutoFinalTestScriptWithoutAnnotation(paramName, classPart)
+  }
+
+  // Checks Groovy compiler behavior when putting the passed classPart into an @AutoFinal annotated class
+  void assertAutoFinalTestScriptWithAnnotation(final String paramName, final String classPart) {
+    final script = autoFinalTestScript(true, classPart)
+    assert script.contains('@AutoFinal')
+    final result = shouldNotCompile(script)
+    println "\nassertAutoFinalTestScript result: |$result|\n\n"
+    assert result.contains("The parameter [$paramName] is declared final but is reassigned")
+  }
+
+  void assertAutoFinalTestScriptWithoutAnnotation(final String paramName, final String classPart) {
+    final script = autoFinalTestScript(false, classPart)
+    assert !script.contains('@AutoFinal')
+    shouldCompile(script)
+  }
+
+  String autoFinalTestScript(final boolean autoFinalAnnotationQ, final String classPart, final String scriptPart = '') {
+    final String script = """
+            import groovy.transform.AutoFinal
+            import groovy.transform.ASTTest
+            import static org.codehaus.groovy.control.CompilePhase.SEMANTIC_ANALYSIS
+            import static java.lang.reflect.Modifier.isFinal
+
+            ${autoFinalAnnotationQ ? '@AutoFinal' : ''}
+            class $autoFinalTestClassName {
+                $classPart
+            }
+
+            $scriptPart
+        """
+    println "script: |$script|"
+    return script
+  }
+
+  String getAutoFinalTestClassName() {
+    'AutoFinalFoo'
+  }
+
+  /**
+   * Prints better readable, unabbreviated stack trace for passed Throwable
+   */
+  void printStackTrace(final Throwable throwable) {
+    println "${throwable.getClass().name}${throwable.message ? ": $throwable.message" : ""}"
+    throwable.stackTrace.each { println it }
+    final inner = throwable.cause
+    if(inner != null) {
+      println "Caused by........................................................................................."
+      printStackTrace(inner)
+    }
+  }
+
+  Throwable shouldThrow(final String script) {
+    try {
+      final GroovyClassLoader gcl = new GroovyClassLoader()
+      gcl.parseClass(script, getTestClassName())
+    }
+    catch(Throwable throwable) {
+      return throwable
+    }
+    throw new Exception("Script was expected to throw here!")
+  }
+}