groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sun...@apache.org
Subject [groovy] 01/02: GROOVY-9151: fix variable expressions referring to replaced parameters
Date Sat, 29 Jun 2019 16:07:14 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 546f034620d37d588e33408d0b9b8565c775282f
Author: Eric Milles <eric.milles@thomsonreuters.com>
AuthorDate: Sun Jun 9 10:52:13 2019 -0500

    GROOVY-9151: fix variable expressions referring to replaced parameters
---
 .../org/codehaus/groovy/classgen/Verifier.java     | 330 +++++++++++----------
 src/test/gls/innerClass/InnerClassTest.groovy      | 153 +++++++---
 src/test/gls/invocation/DefaultParamTest.groovy    | 186 +++++++-----
 3 files changed, 394 insertions(+), 275 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 79c200e..77642cd 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -47,7 +47,6 @@ import org.codehaus.groovy.ast.expr.CastExpression;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
-import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.FieldExpression;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
@@ -56,7 +55,6 @@ import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.ExpressionStatement;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
-import org.codehaus.groovy.ast.tools.GenericsUtils;
 import org.codehaus.groovy.ast.tools.PropertyNodeUtils;
 import org.codehaus.groovy.classgen.asm.BytecodeHelper;
 import org.codehaus.groovy.classgen.asm.MopWriter;
@@ -72,7 +70,6 @@ import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
 import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -81,6 +78,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import java.util.Set;
 
@@ -91,12 +89,18 @@ import static java.lang.reflect.Modifier.isStatic;
 import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
 import static org.apache.groovy.ast.tools.ExpressionUtils.transformInlineConstants;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.declS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
 
 /**
  * Verifies the AST node and adds any default AST code before bytecode generation occurs.
- *
+ * <p>
  * Checks include:
  * <ul>
  *     <li>Methods with duplicate signatures</li>
@@ -141,8 +145,8 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             new Parameter(ClassHelper.METACLASS_TYPE, "mc")
     };
 
-    private static final Class GENERATED_ANNOTATION = Generated.class;
-    private static final Class INTERNAL_ANNOTATION = Internal.class;
+    private static final Class<?> GENERATED_ANNOTATION = Generated.class;
+    private static final Class<?> INTERNAL_ANNOTATION = Internal.class;
 
     private ClassNode classNode;
     private MethodNode methodNode;
@@ -165,6 +169,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         metaClassField =
                 node.addField("metaClass", ACC_PRIVATE | ACC_TRANSIENT | ACC_SYNTHETIC, ClassHelper.METACLASS_TYPE,
                         new BytecodeExpression(ClassHelper.METACLASS_TYPE) {
+                            @Override
                             public void visit(MethodVisitor mv) {
                                 mv.visitVarInsn(ALOAD, 0);
                                 mv.visitMethodInsn(INVOKEVIRTUAL, classInternalName, "$getStaticMetaClass", "()Lgroovy/lang/MetaClass;", false);
@@ -197,11 +202,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         return null;
     }
 
-    /**
-     * walk the class
-     *
-     * @param node the node to visit
-     */
+    @Override
     public void visitClass(final ClassNode node) {
         this.classNode = node;
 
@@ -277,7 +278,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
             @Override
             public void variableNotAlwaysInitialized(final VariableExpression var) {
-                if (Modifier.isFinal(var.getAccessedVariable().getModifiers()))
+                if (isFinal(var.getAccessedVariable().getModifiers()))
                     throw new RuntimeParserException("The variable [" + var.getName() + "] may be uninitialized", var);
             }
         };
@@ -362,6 +363,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 Parameter.EMPTY_ARRAY,
                 ClassNode.EMPTY_ARRAY,
                 new BytecodeSequence(new BytecodeInstruction() {
+                    @Override
                     public void visit(MethodVisitor mv) {
                         mv.visitVarInsn(ALOAD, 0);
                         mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Object", "getClass", "()Ljava/lang/Class;", false);
@@ -397,7 +399,6 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                         mv.visitVarInsn(ALOAD, 1);
                         mv.visitMethodInsn(INVOKEVIRTUAL, "org/codehaus/groovy/reflection/ClassInfo", "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
                         mv.visitInsn(ARETURN);
-
                     }
                 })
         );
@@ -420,12 +421,13 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     Parameter.EMPTY_ARRAY,
                     ClassNode.EMPTY_ARRAY,
                     new BytecodeSequence(new BytecodeInstruction() {
+                        @Override
                         public void visit(MethodVisitor mv) {
                             Label nullLabel = new Label();
                             /*
                              *  the code is:
                              *  if (this.metaClass==null) {
-                             *      this.metaClass = this.$getStaticMetaClass
+                             *      this.metaClass = this.$getStaticMetaClass()
                              *      return this.metaClass
                              *  } else {
                              *      return this.metaClass
@@ -469,6 +471,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             } else {
                 List list = new ArrayList();
                 list.add(new BytecodeInstruction() {
+                    @Override
                     public void visit(MethodVisitor mv) {
                         /*
                          * the code is (meta class is stored in 1):
@@ -509,6 +512,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     ClassHelper.OBJECT_TYPE, INVOKE_METHOD_PARAMS,
                     ClassNode.EMPTY_ARRAY,
                     new BytecodeSequence(new BytecodeInstruction() {
+                        @Override
                         public void visit(MethodVisitor mv) {
                             mv.visitVarInsn(ALOAD, 0);
                             mv.visitMethodInsn(INVOKEVIRTUAL, classInternalName, "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
@@ -534,6 +538,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     GET_PROPERTY_PARAMS,
                     ClassNode.EMPTY_ARRAY,
                     new BytecodeSequence(new BytecodeInstruction() {
+                        @Override
                         public void visit(MethodVisitor mv) {
                             mv.visitVarInsn(ALOAD, 0);
                             mv.visitMethodInsn(INVOKEVIRTUAL, classInternalName, "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
@@ -558,6 +563,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     SET_PROPERTY_PARAMS,
                     ClassNode.EMPTY_ARRAY,
                     new BytecodeSequence(new BytecodeInstruction() {
+                        @Override
                         public void visit(MethodVisitor mv) {
                             mv.visitVarInsn(ALOAD, 0);
                             mv.visitMethodInsn(INVOKEVIRTUAL, classInternalName, "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
@@ -601,26 +607,28 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     }
 
     private static void checkReturnInObjectInitializer(List<Statement> init) {
-        CodeVisitorSupport cvs = new CodeVisitorSupport() {
+        GroovyCodeVisitor visitor = new CodeVisitorSupport() {
             @Override
             public void visitClosureExpression(ClosureExpression expression) {
                 // return is OK in closures in object initializers
             }
-
+            @Override
             public void visitReturnStatement(ReturnStatement statement) {
                 throw new RuntimeParserException("'return' is not allowed in object initializer", statement);
             }
         };
-        for (Statement stm : init) {
-            stm.visit(cvs);
+        for (Statement stmt : init) {
+            stmt.visit(visitor);
         }
     }
 
+    @Override
     public void visitConstructor(ConstructorNode node) {
-        CodeVisitorSupport checkSuper = new CodeVisitorSupport() {
+        GroovyCodeVisitor checkSuper = new CodeVisitorSupport() {
             boolean firstMethodCall = true;
             String type = null;
 
+            @Override
             public void visitMethodCallExpression(MethodCallExpression call) {
                 if (!firstMethodCall) return;
                 firstMethodCall = false;
@@ -633,6 +641,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 type = null;
             }
 
+            @Override
             public void visitConstructorCallExpression(ConstructorCallExpression call) {
                 if (!call.isSpecialCall()) return;
                 type = call.getText();
@@ -640,6 +649,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 type = null;
             }
 
+            @Override
             public void visitVariableExpression(VariableExpression expression) {
                 if (type == null) return;
                 String name = expression.getName();
@@ -656,6 +666,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         s.visit(checkSuper);
     }
 
+    @Override
     public void visitMethod(MethodNode node) {
         //GROOVY-3712 - if it's an MOP method, it's an error as they aren't supposed to exist before ACG is invoked
         if (MopWriter.isMopMethod(node.getName())) {
@@ -691,6 +702,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         adder.visitMethod(node);
     }
 
+    @Override
     public void visitField(FieldNode node) {
     }
 
@@ -704,6 +716,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         return true;
     }
 
+    @Override
     public void visitProperty(PropertyNode node) {
         String name = node.getName();
         FieldNode field = node.getField();
@@ -736,7 +749,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         int getterModifiers = accessorModifiers;
         // don't make static accessors final
         if (node.isStatic()) {
-            getterModifiers = ~Modifier.FINAL & getterModifiers;
+            getterModifiers = ~ACC_FINAL & getterModifiers;
         }
         if (getterBlock != null) {
             visitGetter(node, getterBlock, getterModifiers, getterName);
@@ -756,12 +769,12 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         }
     }
 
-    private void visitGetter(PropertyNode node, Statement getterBlock, int getterModifiers, String secondGetterName) {
-        MethodNode secondGetter =
-                new MethodNode(secondGetterName, getterModifiers, node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
-        secondGetter.setSynthetic(true);
-        addPropertyMethod(secondGetter);
-        visitMethod(secondGetter);
+    private void visitGetter(PropertyNode node, Statement getterBlock, int getterModifiers, String getterName) {
+        MethodNode getter =
+                new MethodNode(getterName, getterModifiers, node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
+        getter.setSynthetic(true);
+        addPropertyMethod(getter);
+        visitMethod(getter);
     }
 
     protected void addPropertyMethod(MethodNode method) {
@@ -795,111 +808,139 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         }
     }
 
+    @FunctionalInterface
     public interface DefaultArgsAction {
-        void call(ArgumentListExpression arguments, Parameter[] newParams, MethodNode method);
+        void call(ArgumentListExpression arguments, Parameter[] parameters, MethodNode method);
     }
 
     /**
-     * Creates a new helper method for each combination of default parameter expressions
+     * Creates a new method for each combination of default parameter expressions.
      */
-    protected void addDefaultParameterMethods(final ClassNode node) {
-        List methods = new ArrayList(node.getMethods());
-        addDefaultParameters(methods, new DefaultArgsAction() {
-            public void call(ArgumentListExpression arguments, Parameter[] newParams, MethodNode method) {
-                final BlockStatement code = new BlockStatement();
-
-                MethodNode newMethod = new MethodNode(method.getName(), method.getModifiers(), method.getReturnType(), newParams, method.getExceptions(), code);
-
-                // GROOVY-5681 and GROOVY-5632
-                for (Expression argument : arguments.getExpressions()) {
-                    if (argument instanceof CastExpression) {
-                        argument = ((CastExpression) argument).getExpression();
-                    }
-                    if (argument instanceof ConstructorCallExpression) {
-                        ClassNode type = argument.getType();
-                        if (type instanceof InnerClassNode && ((InnerClassNode) type).isAnonymous()) {
-                            type.setEnclosingMethod(newMethod);
-                        }
-                    }
+    protected void addDefaultParameterMethods(ClassNode type) {
+        List<MethodNode> methods = new ArrayList<>(type.getMethods());
+        addDefaultParameters(methods, (arguments, params, method) -> {
+            BlockStatement code = new BlockStatement();
 
-                    // check whether closure shared variables refer to params with default values (GROOVY-5632)
-                    if (argument instanceof ClosureExpression) {
-                        final List<Parameter> newMethodNodeParameters = Arrays.asList(newParams);
+            MethodNode newMethod = new MethodNode(method.getName(), method.getModifiers(), method.getReturnType(), params, method.getExceptions(), code);
 
-                        CodeVisitorSupport visitor = new CodeVisitorSupport() {
-                            @Override
-                            public void visitVariableExpression(VariableExpression expression) {
-                                Variable v = expression.getAccessedVariable();
-                                if (!(v instanceof Parameter)) return;
+            MethodNode oldMethod = type.getDeclaredMethod(method.getName(), params);
+            if (oldMethod != null) {
+                throw new RuntimeParserException(
+                        "The method with default parameters \"" + method.getTypeDescriptor() +
+                                "\" defines a method \"" + newMethod.getTypeDescriptor() +
+                                "\" that is already defined.",
+                        method);
+            }
 
-                                Parameter param = (Parameter) v;
-                                if (param.hasInitialExpression() && code.getVariableScope().getDeclaredVariable(param.getName()) == null && !newMethodNodeParameters.contains(param)) {
+            List<AnnotationNode> annotations = method.getAnnotations();
+            if (annotations != null && !annotations.isEmpty()) {
+                newMethod.addAnnotations(annotations);
+            }
+            newMethod.setGenericsTypes(method.getGenericsTypes());
+
+            // GROOVY-5632, GROOVY-9151: check for references to parameters that have been removed
+            GroovyCodeVisitor visitor = new CodeVisitorSupport() {
+                private boolean inClosure;
 
-                                    VariableExpression localVariable = new VariableExpression(param.getName(), ClassHelper.makeReference());
-                                    DeclarationExpression declarationExpression = new DeclarationExpression(localVariable, Token.newSymbol(Types.EQUAL, -1, -1), new ConstructorCallExpression(ClassHelper.makeReference(), param.getInitialExpression()));
+                @Override
+                public void visitClosureExpression(ClosureExpression e) {
+                    boolean prev = inClosure; inClosure = true;
+                    super.visitClosureExpression(e);
+                    inClosure = prev;
+                }
 
-                                    code.addStatement(new ExpressionStatement(declarationExpression));
-                                    code.getVariableScope().putDeclaredVariable(localVariable);
-                                }
+                @Override
+                public void visitVariableExpression(VariableExpression e) {
+                    if (e.getAccessedVariable() instanceof Parameter) {
+                        Parameter p = (Parameter) e.getAccessedVariable();
+                        if (p.hasInitialExpression() && !Arrays.asList(params).contains(p)) {
+                            VariableScope blockScope = code.getVariableScope();
+                            VariableExpression localVariable = (VariableExpression) blockScope.getDeclaredVariable(p.getName());
+                            if (localVariable == null) {
+                                // create a variable declaration so that the name can be found in the new method
+                                localVariable = localVarX(p.getName(), p.getType());
+                                localVariable.setModifiers(p.getModifiers());
+                                blockScope.putDeclaredVariable(localVariable);
+                                code.addStatement(declS(localVariable, p.getInitialExpression()));
+                            }
+                            if (!localVariable.isClosureSharedVariable()) {
+                                localVariable.setClosureSharedVariable(inClosure);
                             }
-                        };
+                        }
+                    }
+                }
+            };
+            visitor.visitArgumentlistExpression(arguments);
+
+            // if variable was created to capture an initial value expression, reference it in arguments as well
+            for (ListIterator<Expression> it = arguments.getExpressions().listIterator(); it.hasNext();) {
+                Expression argument = it.next();
+                if (argument instanceof CastExpression) {
+                    argument = ((CastExpression) argument).getExpression();
+                }
 
-                        visitor.visitClosureExpression((ClosureExpression) argument);
+                for (Parameter p : method.getParameters()) {
+                    if (p.hasInitialExpression() && p.getInitialExpression() == argument) {
+                        if (code.getVariableScope().getDeclaredVariable(p.getName()) != null) {
+                            it.set(varX(p.getName()));
+                        }
+                        break;
                     }
                 }
+            }
 
-                MethodCallExpression expression = new MethodCallExpression(VariableExpression.THIS_EXPRESSION, method.getName(), arguments);
-                expression.setMethodTarget(method);
-                expression.setImplicitThis(true);
+            // delegate to original method using arguments derived from defaults
+            MethodCallExpression call = callThisX(method.getName(), arguments);
+            call.setMethodTarget(method);
+            call.setImplicitThis(true);
 
-                if (method.isVoidMethod()) {
-                    code.addStatement(new ExpressionStatement(expression));
-                } else {
-                    code.addStatement(new ReturnStatement(expression));
-                }
+            if (method.isVoidMethod()) {
+                code.addStatement(new ExpressionStatement(call));
+            } else {
+                code.addStatement(new ReturnStatement(call));
+            }
 
-                List<AnnotationNode> annotations = method.getAnnotations();
-                if (annotations != null) {
-                    newMethod.addAnnotations(annotations);
-                }
-                MethodNode oldMethod = node.getDeclaredMethod(method.getName(), newParams);
-                if (oldMethod != null) {
-                    throw new RuntimeParserException(
-                            "The method with default parameters \"" + method.getTypeDescriptor() +
-                                    "\" defines a method \"" + newMethod.getTypeDescriptor() +
-                                    "\" that is already defined.",
-                            method);
+            // GROOVY-5681: set anon. inner enclosing method reference
+            visitor = new CodeVisitorSupport() {
+                @Override
+                public void visitConstructorCallExpression(ConstructorCallExpression call) {
+                    if (call.isUsingAnonymousInnerClass()) {
+                        call.getType().setEnclosingMethod(newMethod);
+                    }
+                    super.visitConstructorCallExpression(call);
                 }
-                addPropertyMethod(newMethod);
-                newMethod.setGenericsTypes(method.getGenericsTypes());
-                newMethod.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, true);
-            }
+            };
+            visitor.visitBlockStatement(code);
+
+            addPropertyMethod(newMethod);
+            newMethod.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, Boolean.TRUE);
         });
     }
 
-    protected void addDefaultParameterConstructors(final ClassNode node) {
-        List methods = new ArrayList(node.getDeclaredConstructors());
-        addDefaultParameters(methods, new DefaultArgsAction() {
-            public void call(ArgumentListExpression arguments, Parameter[] newParams, MethodNode method) {
-                ConstructorNode ctor = (ConstructorNode) method;
-                ConstructorCallExpression expression = new ConstructorCallExpression(ClassNode.THIS, arguments);
-                Statement code = new ExpressionStatement(expression);
-                addConstructor(newParams, ctor, code, node);
-            }
+    /**
+     * Creates a new constructor for each combination of default parameter expressions.
+     */
+    protected void addDefaultParameterConstructors(ClassNode type) {
+        List<ConstructorNode> constructors = new ArrayList<>(type.getDeclaredConstructors());
+        addDefaultParameters(constructors, (arguments, params, method) -> {
+            // delegate to original constructor using arguments derived from defaults
+            Statement code = new ExpressionStatement(new ConstructorCallExpression(ClassNode.THIS, arguments));
+            addConstructor(params, (ConstructorNode) method, code, type);
         });
     }
 
-    protected void addConstructor(Parameter[] newParams, ConstructorNode ctor, Statement code, ClassNode node) {
-        ConstructorNode genConstructor = node.addConstructor(ctor.getModifiers(), newParams, ctor.getExceptions(), code);
-        markAsGenerated(node, genConstructor);
+    protected void addConstructor(Parameter[] newParams, ConstructorNode ctor, Statement code, ClassNode type) {
+        ConstructorNode newConstructor = type.addConstructor(ctor.getModifiers(), newParams, ctor.getExceptions(), code);
+        newConstructor.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, Boolean.TRUE);
+        markAsGenerated(type, newConstructor);
+        // TODO: Copy annotations, etc.?
     }
 
     /**
-     * Creates a new helper method for each combination of default parameter expressions
+     * Creates a new helper method for each combination of default parameter expressions.
      */
-    protected void addDefaultParameters(List methods, DefaultArgsAction action) {
-        for (Object next : methods) {
-            MethodNode method = (MethodNode) next;
+    protected void addDefaultParameters(List<? extends MethodNode> methods, DefaultArgsAction action) {
+        for (MethodNode method : methods) {
             if (method.hasDefaultValue()) {
                 addDefaultParameters(action, method);
             }
@@ -908,70 +949,43 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
     protected void addDefaultParameters(DefaultArgsAction action, MethodNode method) {
         Parameter[] parameters = method.getParameters();
-        int counter = 0;
-        List paramValues = new ArrayList();
-        int size = parameters.length;
-        for (int i = size - 1; i >= 0; i--) {
-            Parameter parameter = parameters[i];
-            if (parameter != null && parameter.hasInitialExpression()) {
-                paramValues.add(i);
-                paramValues.add(
-                        new CastExpression(
-                                parameter.getType(),
-                                parameter.getInitialExpression()
-                        )
-                );
-                counter++;
-            }
-        }
+        long n = Arrays.stream(parameters).filter(Parameter::hasInitialExpression).count();
 
-        for (int j = 1; j <= counter; j++) {
-            Parameter[] newParams = new Parameter[parameters.length - j];
+        for (int i = 1; i <= n; i += 1) {
+            Parameter[] newParams = new Parameter[parameters.length - i];
             ArgumentListExpression arguments = new ArgumentListExpression();
             int index = 0;
-            int k = 1;
+            int j = 1;
             for (Parameter parameter : parameters) {
                 if (parameter == null) {
                     throw new GroovyBugError("Parameter should not be null for method " + methodNode.getName());
                 } else {
-                    if (k > counter - j && parameter.hasInitialExpression()) {
-                        arguments.addExpression(
-                                new CastExpression(
-                                        parameter.getType(),
-                                        parameter.getInitialExpression()
-                                )
-                        );
-                        k++;
-                    } else if (parameter.hasInitialExpression()) {
-                        index = addExpression(newParams, arguments, index, parameter);
-                        k++;
+                    Expression e;
+                    if (j > n - i && parameter.hasInitialExpression()) {
+                        e = parameter.getInitialExpression();
                     } else {
-                        index = addExpression(newParams, arguments, index, parameter);
+                        newParams[index++] = parameter;
+                        e = varX(parameter);
                     }
+
+                    arguments.addExpression(castX(parameter.getType(), e));
+
+                    if (parameter.hasInitialExpression()) j += 1;
                 }
             }
             action.call(arguments, newParams, method);
         }
 
         for (Parameter parameter : parameters) {
-            if (!parameter.hasInitialExpression()) continue; // GROOVY-8728 make idempotent
-            // remove default expression and store it as node metadata
-            parameter.putNodeMetaData(Verifier.INITIAL_EXPRESSION, parameter.getInitialExpression());
-            parameter.setInitialExpression(null);
+            if (parameter.hasInitialExpression()) {
+                // remove default expression and store it as node metadata
+                parameter.putNodeMetaData(Verifier.INITIAL_EXPRESSION,
+                        parameter.getInitialExpression());
+                parameter.setInitialExpression(null);
+            }
         }
     }
 
-    private int addExpression(Parameter[] newParams, ArgumentListExpression arguments, int index, Parameter parameter) {
-        newParams[index++] = parameter;
-        arguments.addExpression(
-                new CastExpression(
-                        parameter.getType(),
-                        new VariableExpression(parameter.getName())
-                )
-        );
-        return index;
-    }
-
     protected void addClosureCode(InnerClassNode node) {
         // add a new invoke
     }
@@ -1082,9 +1096,9 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     }
 
     /*
-    *  when InnerClassVisitor adds this.this$0 = $p$n, it adds it as a BlockStatement having that
-    *  ExpressionStatement
-    */
+     * When InnerClassVisitor adds <code>this.this$0 = $p$n</code>, it adds it
+     * as a BlockStatement having that ExpressionStatement.
+     */
     private Statement getImplicitThis$0StmtIfInnerClass(List<Statement> otherStatements) {
         if (!(classNode instanceof InnerClassNode)) return null;
         for (Statement stmt : otherStatements) {
@@ -1131,7 +1145,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         Expression expression = fieldNode.getInitialExpression();
         if (expression != null) {
             final FieldExpression fe = new FieldExpression(fieldNode);
-            if (fieldNode.getType().equals(ClassHelper.REFERENCE_TYPE) && ((fieldNode.getModifiers() & Opcodes.ACC_SYNTHETIC) != 0)) {
+            if (fieldNode.getType().equals(ClassHelper.REFERENCE_TYPE) && ((fieldNode.getModifiers() & ACC_SYNTHETIC) != 0)) {
                 fe.setUseReferenceDirectly(true);
             }
             ExpressionStatement statement =
@@ -1172,7 +1186,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     }
 
     /**
-     * Capitalizes the start of the given bean property name
+     * Capitalizes the start of the given bean property name.
      */
     public static String capitalize(String name) {
         return BeanUtils.capitalize(name);
@@ -1194,6 +1208,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
     protected Statement createSetterBlock(PropertyNode propertyNode, final FieldNode field) {
         return new BytecodeSequence(new BytecodeInstruction() {
+            @Override
             public void visit(MethodVisitor mv) {
                 if (field.isStatic()) {
                     BytecodeHelper.load(mv, field.getType(), 0);
@@ -1209,7 +1224,6 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     }
 
     public void visitGenericType(GenericsType genericsType) {
-
     }
 
     public static Long getTimestampFromFieldName(String fieldName) {
@@ -1337,7 +1351,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         if (!normalEqualParameters && !genericEqualParameters) return null;
 
         //correct to method level generics for the overriding method
-        genericsSpec = GenericsUtils.addMethodGenerics(overridingMethod, genericsSpec);
+        genericsSpec = addMethodGenerics(overridingMethod, genericsSpec);
 
         // return type
         ClassNode mr = overridingMethod.getReturnType();
@@ -1407,6 +1421,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         List instructions = new ArrayList(1);
         instructions.add(
                 new BytecodeInstruction() {
+                    @Override
                     public void visit(MethodVisitor mv) {
                         mv.visitVarInsn(ALOAD, 0);
                         Parameter[] para = oldMethod.getParameters();
@@ -1503,7 +1518,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     private static boolean moveOptimizedConstantsInitialization(final ClassNode node) {
         if (node.isInterface() && !Traits.isTrait(node)) return false;
 
-        final int mods = Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC;
+        final int mods = ACC_STATIC | ACC_SYNTHETIC | ACC_PUBLIC;
         String name = SWAP_INIT;
         BlockStatement methodCode = new BlockStatement();
 
@@ -1592,5 +1607,4 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             }
         }
     }
-
 }
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index 6889192..892412b 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -114,13 +114,13 @@ class InnerClassTest extends CompilableTestSupport {
     void testStaticInnerClass() {
         assertScript """
             import java.lang.reflect.Modifier
-        
+
             class A {
                 static class B{}
             }
             def x = new A.B()
             assert x != null
-            
+
             def mods = A.B.modifiers
             assert Modifier.isPublic(mods)
         """
@@ -386,8 +386,8 @@ class InnerClassTest extends CompilableTestSupport {
     void testClassOutputOrdering() {
         // this does actually not do much, but before this
         // change the inner class was tried to be executed
-        // because a class ordering bug. The main method 
-        // makes the Foo class executeable, but Foo$Bar is 
+        // because a class ordering bug. The main method
+        // makes the Foo class executeable, but Foo$Bar is
         // not. So if Foo$Bar is returned, asserScript will
         // fail. If Foo is returned, asserScript will not
         // fail.
@@ -473,7 +473,7 @@ class A {
     void testReferencedVariableInAIC() {
         assertScript """
             interface X{}
-            
+
             final double delta = 0.1
             (0 ..< 1).collect { n ->
                 new X () {
@@ -485,7 +485,7 @@ class A {
         """
         assertScript """
             interface X{}
-            
+
             final double delta1 = 0.1
             final double delta2 = 0.1
             (0 ..< 1).collect { n ->
@@ -511,51 +511,116 @@ class A {
         '''
     }
 
-    // GROOVY-5679
-    // GROOVY-5681
+    // GROOVY-5679, GROOVY-5681
     void testEnclosingMethodIsSet() {
-        new GroovyShell().evaluate '''import groovy.transform.ASTTest
-        import static org.codehaus.groovy.control.CompilePhase.*
-        import org.codehaus.groovy.ast.InnerClassNode
-        import org.codehaus.groovy.ast.expr.ConstructorCallExpression
-import org.codehaus.groovy.classgen.Verifier
-
-        class A {
-            int x
-
-            /*@ASTTest(phase=SEMANTIC_ANALYSIS, value={
-                def cce = lookup('inner')[0].expression
-                def icn = cce.type
-                assert icn instanceof InnerClassNode
-                assert icn.enclosingMethod == node
-            })
-            A() { inner: new Runnable() { void run() {} } }
+        assertScript '''
+            import groovy.transform.ASTTest
+            import org.codehaus.groovy.ast.InnerClassNode
+            import org.codehaus.groovy.ast.expr.ConstructorCallExpression
+            import static org.codehaus.groovy.classgen.Verifier.*
+            import static org.codehaus.groovy.control.CompilePhase.*
 
-            @ASTTest(phase=SEMANTIC_ANALYSIS, value={
-                def cce = lookup('inner')[0].expression
-                def icn = cce.type
-                assert icn instanceof InnerClassNode
-                assert icn.enclosingMethod == node
-            })
-            void foo() { inner: new Runnable() { void run() {} } }*/
+            class A {
+                int x
+
+                /*@ASTTest(phase=SEMANTIC_ANALYSIS, value={
+                    def cce = lookup('inner')[0].expression
+                    def icn = cce.type
+                    assert icn instanceof InnerClassNode
+                    assert icn.enclosingMethod == node
+                })
+                A() { inner: new Runnable() { void run() {} } }
+
+                @ASTTest(phase=SEMANTIC_ANALYSIS, value={
+                    def cce = lookup('inner')[0].expression
+                    def icn = cce.type
+                    assert icn instanceof InnerClassNode
+                    assert icn.enclosingMethod == node
+                })
+                void foo() { inner: new Runnable() { void run() {} } }*/
+
+                @ASTTest(phase=CLASS_GENERATION, value={
+                    def initialExpression = node.parameters[0].getNodeMetaData(INITIAL_EXPRESSION)
+                    assert initialExpression instanceof ConstructorCallExpression
+                    def icn = initialExpression.type
+                    assert icn instanceof InnerClassNode
+                    assert icn.enclosingMethod != null
+                    assert icn.enclosingMethod.name == 'bar'
+                    assert icn.enclosingMethod.parameters.length == 0 // ensure the enclosing method is bar(), not bar(Object)
+                })
+                void bar(action = new Runnable() { void run() { x = 123 }}) {
+                    action.run()
+                }
+            }
+            def a = new A()
+            a.bar()
+            assert a.x == 123
+        '''
+    }
+
+    // GROOVY-5681, GROOVY-9151
+    void testEnclosingMethodIsSet2() {
+        assertScript '''
+            import groovy.transform.ASTTest
+            import org.codehaus.groovy.ast.expr.*
+            import static org.codehaus.groovy.classgen.Verifier.*
+            import static org.codehaus.groovy.control.CompilePhase.*
 
             @ASTTest(phase=CLASS_GENERATION, value={
-                def initialExpression = node.parameters[0].getNodeMetaData(Verifier.INITIAL_EXPRESSION)
-                assert initialExpression instanceof ConstructorCallExpression
-                def icn = initialExpression.type
-                assert icn instanceof InnerClassNode
-                assert icn.enclosingMethod != null
-                assert icn.enclosingMethod.name == 'bar'
-                assert icn.enclosingMethod.parameters.length == 0 // ensure the enclosing method is bar(), not bar(Object)
+                def init = node.parameters[0].getNodeMetaData(INITIAL_EXPRESSION)
+                assert init instanceof MapExpression
+                assert init.mapEntryExpressions[0].valueExpression instanceof ConstructorCallExpression
+                def type = init.mapEntryExpressions[0].valueExpression.type
+
+                assert type.enclosingMethod != null
+                assert type.enclosingMethod.name == 'bar'
+                assert type.enclosingMethod.parameters.length == 0 // ensure the enclosing method is bar(), not bar(Map)
             })
-            void bar(action=new Runnable() { void run() { x = 123 }}) {
-                action.run()
+            void bar(Map args = [action: new Runnable() { void run() { result = 123 }}]) {
+                args.action.run()
             }
 
-        }
-        def a = new A()
-        a.bar()
-        assert a.x == 123
+            bar()
+        '''
+    }
+
+    // GROOVY-5681, GROOVY-9151
+    void testEnclosingMethodIsSet3() {
+        assertScript '''
+            import groovy.transform.ASTTest
+            import org.codehaus.groovy.ast.expr.*
+            import org.codehaus.groovy.ast.stmt.*
+            import static org.codehaus.groovy.classgen.Verifier.*
+            import static org.codehaus.groovy.control.CompilePhase.*
+
+            @ASTTest(phase=CLASS_GENERATION, value={
+                def init = node.parameters[0].getNodeMetaData(INITIAL_EXPRESSION)
+                assert init instanceof ConstructorCallExpression
+                assert init.type.enclosingMethod != null
+                assert init.type.enclosingMethod.name == 'bar'
+                assert init.type.enclosingMethod.parameters.length == 0 // ensure the enclosing method is bar(), not bar(Runnable)
+
+                assert init.type.getMethods('run')[0].code instanceof BlockStatement
+                assert init.type.getMethods('run')[0].code.statements[0] instanceof ExpressionStatement
+                assert init.type.getMethods('run')[0].code.statements[0].expression instanceof DeclarationExpression
+
+                init = init.type.getMethods('run')[0].code.statements[0].expression.rightExpression
+                assert init instanceof ConstructorCallExpression
+                assert init.isUsingAnonymousInnerClass()
+                assert init.type.enclosingMethod != null
+                assert init.type.enclosingMethod.name == 'run'
+                assert init.type.enclosingMethod.parameters.length == 0
+            })
+            void bar(Runnable runner = new Runnable() {
+                @Override void run() {
+                    def comparator = new Comparator<int>() {
+                        int compare(int one, int two) {
+                        }
+                    }
+                }
+            }) {
+                args.action.run()
+            }
         '''
     }
 
diff --git a/src/test/gls/invocation/DefaultParamTest.groovy b/src/test/gls/invocation/DefaultParamTest.groovy
index 228fc06..465af04 100644
--- a/src/test/gls/invocation/DefaultParamTest.groovy
+++ b/src/test/gls/invocation/DefaultParamTest.groovy
@@ -18,27 +18,28 @@
  */
 package gls.invocation
 
-import gls.CompilableTestSupport
+import groovy.transform.CompileStatic
 
-class DefaultParamTest extends CompilableTestSupport {
+@CompileStatic
+final class DefaultParamTest extends GroovyTestCase {
 
     void testDefaultParameterCausingDoubledMethod() {
         //GROOVY-2191
-        shouldNotCompile '''
+        shouldFail '''
             def foo(String one, String two = "two") {"$one $two"}
             def foo(String one, String two = "two", String three = "three") {"$one $two $three"}
         '''
 
-        shouldNotCompile '''
+        shouldFail '''
             def foo(String one, String two = "two", String three = "three") {"$one $two $three"}
             def foo(String one, String two = "two") {"$one $two"}
         '''
-            
-        shouldNotCompile '''
+
+        shouldFail '''
             def meth(Closure cl = null) {meth([:], cl)}
             def meth(Map args = [:], Closure cl = null) {}
         '''
-    } 
+    }
 
     void testDefaultParameters() {
         assertScript '''
@@ -49,18 +50,17 @@ class DefaultParamTest extends CompilableTestSupport {
             assert "X-Y-defC" == doSomething("X", "Y")
             assert "X-defB-defC" == doSomething("X")
         '''
-        shouldFail {
-            assertScript '''
-                def doSomething(a, b = 'defB', c = 'defC') {
-                    return a + "-" + b + "-" + c
-                }
-                doSomething()        
-            '''
-        }
+
+        shouldFail '''
+            def doSomething(a, b = 'defB', c = 'defC') {
+                return a + "-" + b + "-" + c
+            }
+            doSomething()
+        '''
     }
 
     void testDefaultTypedParameters() {
-        assertScript """
+        assertScript '''
             String doTypedSomething(String a = 'defA', String b = 'defB', String c = 'defC') {
                 a + "-" + b + "-" + c
             }
@@ -68,30 +68,29 @@ class DefaultParamTest extends CompilableTestSupport {
             assert "X-Y-defC" == doTypedSomething("X", "Y")
             assert "X-defB-defC" == doTypedSomething("X")
             assert "defA-defB-defC" == doTypedSomething()
-        """
+        '''
     }
 
     void testDefaultTypedParametersAnother() {
-        assertScript """
+        assertScript '''
             String doTypedSomethingAnother(String a = 'defA', String b = 'defB', String c) {
                 return a + "-" + b + "-" + c
             }
             assert "X-Y-Z" == doTypedSomethingAnother("X", "Y", "Z")
             assert "X-defB-Z" == doTypedSomethingAnother("X", "Z")
-            assert "defA-defB-Z" == doTypedSomethingAnother("Z")            
-        """
-        shouldFail {
-            assertScript """
-                String doTypedSomethingAnother(String a = 'defA', String b = 'defB', String c) {
-                    return a + "-" + b + "-" + c
-                }
-                doTypedSomethingAnother()
-            """
-        }
+            assert "defA-defB-Z" == doTypedSomethingAnother("Z")
+        '''
+
+        shouldFail '''
+            String doTypedSomethingAnother(String a = 'defA', String b = 'defB', String c) {
+                return a + "-" + b + "-" + c
+            }
+            doTypedSomethingAnother()
+        '''
     }
-    
+
     void testConstructor() {
-        assertScript """
+        assertScript '''
             class DefaultParamTestTestClass {
                 def j
                 DefaultParamTestTestClass(int i = 1){j=i}
@@ -100,24 +99,24 @@ class DefaultParamTest extends CompilableTestSupport {
             def foo = new DefaultParamTestTestClass()
             assert foo.j == 1
             foo = new DefaultParamTestTestClass(2)
-            assert foo.j == 2        
-        """
+            assert foo.j == 2
+        '''
     }
-    
+
     void testPrecendence() {
         // def meth(Closure cl = null) will produce a call meth(null)
         // since interfaces are prefered over normal classes and since
         // def meth(Map args, Closure cl = null) will produce a method
         // meth(Map) a simple call with meth(null) would normally call
-        // meth(Map). To ensure this will not happen the call has to 
-        // use a cast in the automatically created method. 
-        assertScript """
+        // meth(Map). To ensure this will not happen the call has to
+        // use a cast in the automatically created method.
+        assertScript '''
             def meth(Closure cl = null) {
               return '1' +meth([:], cl)
             }
 
             def meth(Map args, Closure cl = null) {
-                if(args==null) return "2" 
+                if(args==null) return "2"
                 return '2'+args.size()
             }
 
@@ -126,70 +125,111 @@ class DefaultParamTest extends CompilableTestSupport {
             assert meth {} == "120"
             assert meth(a:1) == "21"
             assert meth(a:1) {} == "21"
-        """
+        '''
     }
 
-    void testClosureSharedVariableRefersToDefaultParameter() {
-        assertScript """
-                def f1( int x = 3, fn={ -> x } ) {
-                    return fn()
+    // GROOVY-9151
+    void testMethodWithAllParametersDefaulted() {
+        assertScript '''
+            String greet(Object o = 'world', String s = o.toString()) {
+                "hello $s"
+            }
+            assert greet() == 'hello world'
+        '''
+    }
+
+    // GROOVY-9151
+    void _FIXME_testConstructorWithAllParametersDefaulted() {
+        assertScript '''
+            class Greeting {
+                Greeting(Object o = 'world', String s = o.toString()) {
+                    this.text = "hello $s"
                 }
+                String text
+            }
+            assert new Greeting().text == 'hello world'
+        '''
+    }
 
-                assert 3 == f1()
-                assert 42 == f1(42)
-            """
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter1() {
+        assertScript '''
+            def f1( int x = 3, fn={ -> x } ) {
+                return fn()
+            }
 
-        assertScript """
-               def f2( int x = 3, fn={ -> def c2 = { -> x }; c2.call() } ) {
-                   return fn()
-               }
+            assert 3 == f1()
+            assert 42 == f1(42)
+        '''
+    }
 
-               assert 42 == f2(42)
-               assert 42 == f2(42)
-               assert 84 == f2(42) { 84 }
-            """
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter2() {
+        assertScript '''
+           def f2( int x = 3, fn={ -> def c2 = { -> x }; c2.call() } ) {
+               return fn()
+           }
 
-        assertScript """
-               def f3(fn={ -> 42 }, fn2={ -> def c2 = { -> fn() }; c2.call() } ) {
-                   return fn2()
-               }
+           assert 42 == f2(42)
+           assert 42 == f2(42)
+           assert 84 == f2(42) { 84 }
+        '''
+    }
 
-               assert 42 == f3()
-               assert 84 == f3({ -> 84 })
-            """
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter3() {
+        assertScript '''
+           def f3(fn={ -> 42 }, fn2={ -> def c2 = { -> fn() }; c2.call() } ) {
+               return fn2()
+           }
+
+           assert 42 == f3()
+           assert 84 == f3({ -> 84 })
+        '''
+    }
 
-        assertScript """
-               def f4(def s = [1,2,3], fn = { -> s.size() }) {
-                   fn()
-               }
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter4() {
+        assertScript '''
+           def f4(def s = [1,2,3], fn = { -> s.size() }) {
+               fn()
+           }
 
-               assert 3 == f4()
-            """
+           assert 3 == f4()
+        '''
+    }
 
-        assertScript """
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter5() {
+        assertScript '''
            static <T extends Number> Integer f5(List<T> s = [1,2,3], fn = { -> (s*.intValue()).sum() }) {
                fn()
            }
 
            assert 6 == f5()
            assert 6 == f5([1.1, 2.1, 3.1])
-        """
+        '''
+    }
 
-        assertScript """
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter6() {
+        assertScript '''
            def f6(def s = [1,2,3], fn = { -> s.size() }, fn2 = { fn() + s.size() }) {
                fn2()
            }
 
            assert 6 == f6()
-        """
+        '''
+    }
 
-        assertScript """
+    // GROOVY-5632
+    void testClosureSharedVariableRefersToDefaultParameter7() {
+        assertScript '''
             def f7( int x = 3, int y = 39, fn={ -> x + y } ) {
                 return fn()
             }
 
             assert 42 == f7()
-        """
+        '''
     }
 }
-


Mime
View raw message