groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject groovy git commit: GROOVY-7969: Incorrect modifers on setter for volatile property with @Bindable/@Vetoable (closes #448)
Date Thu, 13 Oct 2016 07:28:36 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_4_X ddf523e92 -> 69ace4d8c


GROOVY-7969: Incorrect modifers on setter for volatile property with @Bindable/@Vetoable (closes
#448)


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

Branch: refs/heads/GROOVY_2_4_X
Commit: 69ace4d8c7d3963e1843df8e35806d40082b6889
Parents: ddf523e
Author: paulk <paulk@asert.com.au>
Authored: Wed Oct 12 21:42:17 2016 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Thu Oct 13 17:28:12 2016 +1000

----------------------------------------------------------------------
 .../groovy/beans/BindableASTTransformation.java |   3 +-
 .../groovy/beans/VetoableASTTransformation.java |   3 +-
 .../groovy/ast/tools/PropertyNodeUtils.java     |  43 ++++
 .../org/codehaus/groovy/classgen/Verifier.java  | 235 ++++++++++---------
 src/test/groovy/bugs/Groovy7969Bug.groovy       |  33 +++
 5 files changed, 204 insertions(+), 113 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/69ace4d8/src/main/groovy/beans/BindableASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/beans/BindableASTTransformation.java b/src/main/groovy/beans/BindableASTTransformation.java
index 31ac190..f987295 100644
--- a/src/main/groovy/beans/BindableASTTransformation.java
+++ b/src/main/groovy/beans/BindableASTTransformation.java
@@ -30,6 +30,7 @@ import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.ast.tools.PropertyNodeUtils;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.control.messages.SimpleMessage;
@@ -247,7 +248,7 @@ public class BindableASTTransformation implements ASTTransformation, Opcodes
{
     protected void createSetterMethod(ClassNode declaringClass, PropertyNode propertyNode,
String setterName, Statement setterBlock) {
         MethodNode setter = new MethodNode(
                 setterName,
-                propertyNode.getModifiers(),
+                PropertyNodeUtils.adjustPropertyModifiersForMethod(propertyNode),
                 ClassHelper.VOID_TYPE,
                 params(param(propertyNode.getType(), "value")),
                 ClassNode.EMPTY_ARRAY,

http://git-wip-us.apache.org/repos/asf/groovy/blob/69ace4d8/src/main/groovy/beans/VetoableASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/beans/VetoableASTTransformation.java b/src/main/groovy/beans/VetoableASTTransformation.java
index 67a1617..d7353fd 100644
--- a/src/main/groovy/beans/VetoableASTTransformation.java
+++ b/src/main/groovy/beans/VetoableASTTransformation.java
@@ -30,6 +30,7 @@ import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.ast.tools.PropertyNodeUtils;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.control.messages.SimpleMessage;
@@ -312,7 +313,7 @@ public class VetoableASTTransformation extends BindableASTTransformation
{
         ClassNode[] exceptions = {ClassHelper.make(PropertyVetoException.class)};
         MethodNode setter = new MethodNode(
                 setterName,
-                propertyNode.getModifiers(),
+                PropertyNodeUtils.adjustPropertyModifiersForMethod(propertyNode),
                 ClassHelper.VOID_TYPE,
                 params(param(propertyNode.getType(), "value")),
                 exceptions,

http://git-wip-us.apache.org/repos/asf/groovy/blob/69ace4d8/src/main/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java b/src/main/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
new file mode 100644
index 0000000..513144a
--- /dev/null
+++ b/src/main/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
@@ -0,0 +1,43 @@
+/*
+ *  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.ast.tools;
+
+import org.codehaus.groovy.ast.PropertyNode;
+
+import java.lang.reflect.Modifier;
+
+public class PropertyNodeUtils {
+    /**
+     * Fields within the AST that have no explicit visibility are deemed to be properties
+     * and represented by a PropertyNode. The Groovy compiler creates accessor methods and
+     * a backing field for such property nodes. During this process, all modifiers
+     * from the property are carried over to the backing field (so a property marked as
+     * {@code transient} will have a {@code transient} backing field) but when creating
+     * the accessor methods we don't carry over modifier values which don't make sense for
+     * methods (this includes VOLATILE and TRANSIENT) but other modifiers are carried over,
+     * for example {@code static}.
+     *
+     * @param propNode the original property node
+     * @return the modifiers which make sense for an accessor method
+     */
+    public static int adjustPropertyModifiersForMethod(PropertyNode propNode) {
+        // GROOVY-3726: clear volatile, transient modifiers so that they don't get applied
to methods
+        return ~(Modifier.TRANSIENT | Modifier.VOLATILE) & propNode.getModifiers();
+    }
+}

http://git-wip-us.apache.org/repos/asf/groovy/blob/69ace4d8/src/main/org/codehaus/groovy/classgen/Verifier.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/Verifier.java b/src/main/org/codehaus/groovy/classgen/Verifier.java
index c3d01e6..78e53d0 100644
--- a/src/main/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/org/codehaus/groovy/classgen/Verifier.java
@@ -21,6 +21,7 @@ package org.codehaus.groovy.classgen;
 import groovy.lang.GroovyClassLoader;
 import groovy.lang.GroovyObject;
 import groovy.lang.MetaClass;
+import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.*;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
@@ -39,6 +40,7 @@ import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
 import org.codehaus.groovy.ast.tools.ClassNodeUtils;
 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;
 import org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.ClassNodeSkip;
@@ -66,15 +68,35 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import static java.lang.reflect.Modifier.isAbstract;
+import static java.lang.reflect.Modifier.isFinal;
+import static java.lang.reflect.Modifier.isPrivate;
+import static java.lang.reflect.Modifier.isPublic;
+import static java.lang.reflect.Modifier.isStatic;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.makeDescriptorWithoutReturnType;
 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 defaulted AST code before
- * bytecode generation occurs.
+ * Verifies the AST node and adds any default AST code before bytecode generation occurs.
  *
- * @author <a href="mailto:james@coredevelopers.net">James Strachan</a>
+ * Checks include:
+ * <ul>
+ *     <li>Methods with duplicate signatures</li>
+ *     <li>Duplicate interfaces</li>
+ *     <li>Reassigned final variables/parameters</li>
+ *     <li>Uninitialized variables</li>
+ *     <li>Bad code in object initializers or constructors</li>
+ *     <li>Mismatches in modifiers or return types between implementations and interfaces/abstract
classes</li>
+ * </ul>
+ *
+ * Added code includes:
+ * <ul>
+ *     <li>Methods needed to implement GroovyObject</li>
+ *     <li>Property accessor methods</li>
+ *     <li>Covariant methods</li>
+ *     <li>Additional methods/constructors as needed for default parameters</li>
+ * </ul>
  */
 public class Verifier implements GroovyClassVisitor, Opcodes {
 
@@ -121,12 +143,13 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         if (metaClassField != null) return metaClassField;
         final String classInternalName = BytecodeHelper.getClassInternalName(node);
         metaClassField =
-            node.addField("metaClass", ACC_PRIVATE | ACC_TRANSIENT | ACC_SYNTHETIC, ClassHelper.METACLASS_TYPE,
-                    new BytecodeExpression(ClassHelper.METACLASS_TYPE) {
-                        public void visit(MethodVisitor mv) {
-                            mv.visitVarInsn(ALOAD, 0);
-                            mv.visitMethodInsn(INVOKEVIRTUAL, classInternalName, "$getStaticMetaClass",
"()Lgroovy/lang/MetaClass;", false);
-                        }});
+                node.addField("metaClass", ACC_PRIVATE | ACC_TRANSIENT | ACC_SYNTHETIC, ClassHelper.METACLASS_TYPE,
+                        new BytecodeExpression(ClassHelper.METACLASS_TYPE) {
+                            public void visit(MethodVisitor mv) {
+                                mv.visitVarInsn(ALOAD, 0);
+                                mv.visitMethodInsn(INVOKEVIRTUAL, classInternalName, "$getStaticMetaClass",
"()Lgroovy/lang/MetaClass;", false);
+                            }
+                        });
         metaClassField.setSynthetic(true);
         return metaClassField;
     }
@@ -148,29 +171,29 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             if (current == null) break;
             ret = current.getDeclaredField("metaClass");
             if (ret == null) continue;
-            if (Modifier.isPrivate(ret.getModifiers())) continue;
+            if (isPrivate(ret.getModifiers())) continue;
             return ret;
         }
         return null;
     }
 
     /**
-     * add code to implement GroovyObject
+     * walk the class
      *
-     * @param node
+     * @param node the node to visit
      */
     public void visitClass(final ClassNode node) {
         this.classNode = node;
 
         if (Traits.isTrait(node) // maybe possible to have this true in joint compilation
mode
-                || ((classNode.getModifiers() & Opcodes.ACC_INTERFACE) > 0)) {
+                || classNode.isInterface()) {
             //interfaces have no constructors, but this code expects one,
             //so create a dummy and don't add it to the class node
             ConstructorNode dummy = new ConstructorNode(0, null);
             addInitialization(node, dummy);
             node.visitContents(this);
-            if (classNode.getNodeMetaData(ClassNodeSkip.class)==null) {
-                classNode.setNodeMetaData(ClassNodeSkip.class,true);
+            if (classNode.getNodeMetaData(ClassNodeSkip.class) == null) {
+                classNode.setNodeMetaData(ClassNodeSkip.class, true);
             }
             return;
         }
@@ -237,21 +260,21 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     private static FieldNode checkFieldDoesNotExist(ClassNode node, String fieldName) {
         FieldNode ret = node.getDeclaredField(fieldName);
         if (ret != null) {
-            if (    Modifier.isPublic(ret.getModifiers()) &&
-                    ret.getType().redirect()==ClassHelper.boolean_TYPE) {
+            if (isPublic(ret.getModifiers()) &&
+                    ret.getType().redirect() == ClassHelper.boolean_TYPE) {
                 return ret;
             }
             throw new RuntimeParserException("The class " + node.getName() +
-                    " cannot declare field '"+fieldName+"' as this" +
+                    " cannot declare field '" + fieldName + "' as this" +
                     " field is needed for internal groovy purposes", ret);
         }
         return null;
     }
 
     private static void addFastPathHelperFieldsAndHelperMethod(ClassNode node, final String
classInternalName, boolean knownSpecialCase) {
-        if (node.getNodeMetaData(ClassNodeSkip.class)!=null) return;
-        FieldNode stMCB = checkFieldDoesNotExist(node,STATIC_METACLASS_BOOL);
-        if (stMCB==null) {
+        if (node.getNodeMetaData(ClassNodeSkip.class) != null) return;
+        FieldNode stMCB = checkFieldDoesNotExist(node, STATIC_METACLASS_BOOL);
+        if (stMCB == null) {
             stMCB = node.addField(
                     STATIC_METACLASS_BOOL,
                     ACC_PUBLIC | ACC_STATIC | ACC_SYNTHETIC | ACC_TRANSIENT,
@@ -291,7 +314,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                         mv.visitVarInsn(ALOAD, 0);
                         mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Object", "getClass",
"()Ljava/lang/Class;", false);
                         if (BytecodeHelper.isClassLiteralPossible(node) || BytecodeHelper.isSameCompilationUnit(classNode,
node)) {
-                            BytecodeHelper.visitClassLiteral(mv,node);
+                            BytecodeHelper.visitClassLiteral(mv, node);
                         } else {
                             mv.visitMethodInsn(INVOKESTATIC, classInternalName, "$get$$class$"
+ classInternalName.replaceAll("\\/", "\\$"), "()Ljava/lang/Class;", false);
                         }
@@ -334,7 +357,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
         if (!node.hasMethod("getMetaClass", Parameter.EMPTY_ARRAY)) {
             metaClassField = setMetaClassFieldIfNotExists(node, metaClassField);
-            addMethod(node, !Modifier.isAbstract(node.getModifiers()),
+            addMethod(node, !isAbstract(node.getModifiers()),
                     "getMetaClass",
                     ACC_PUBLIC,
                     ClassHelper.METACLASS_TYPE,
@@ -379,7 +402,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         if (!node.hasMethod("setMetaClass", parameters)) {
             metaClassField = setMetaClassFieldIfNotExists(node, metaClassField);
             Statement setMetaClassCode;
-            if (Modifier.isFinal(metaClassField.getModifiers())) {
+            if (isFinal(metaClassField.getModifiers())) {
                 ConstantExpression text = new ConstantExpression("cannot set read-only meta
class");
                 ConstructorCallExpression cce = new ConstructorCallExpression(ClassHelper.make(IllegalArgumentException.class),
text);
                 setMetaClassCode = new ExpressionStatement(cce);
@@ -401,7 +424,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 setMetaClassCode = new BytecodeSequence(list);
             }
 
-            addMethod(node, !Modifier.isAbstract(node.getModifiers()),
+            addMethod(node, !isAbstract(node.getModifiers()),
                     "setMetaClass",
                     ACC_PUBLIC, ClassHelper.VOID_TYPE,
                     SET_METACLASS_PARAMS, ClassNode.EMPTY_ARRAY,
@@ -416,7 +439,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             blockScope.putReferencedLocalVariable(vMethods);
             blockScope.putReferencedLocalVariable(vArguments);
 
-            addMethod(node, !Modifier.isAbstract(node.getModifiers()),
+            addMethod(node, !isAbstract(node.getModifiers()),
                     "invokeMethod",
                     ACC_PUBLIC,
                     ClassHelper.OBJECT_TYPE, INVOKE_METHOD_PARAMS,
@@ -436,7 +459,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         }
 
         if (!node.hasMethod("getProperty", GET_PROPERTY_PARAMS)) {
-            addMethod(node, !Modifier.isAbstract(node.getModifiers()),
+            addMethod(node, !isAbstract(node.getModifiers()),
                     "getProperty",
                     ACC_PUBLIC,
                     ClassHelper.OBJECT_TYPE,
@@ -456,7 +479,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         }
 
         if (!node.hasMethod("setProperty", SET_PROPERTY_PARAMS)) {
-            addMethod(node, !Modifier.isAbstract(node.getModifiers()),
+            addMethod(node, !isAbstract(node.getModifiers()),
                     "setProperty",
                     ACC_PUBLIC,
                     ClassHelper.VOID_TYPE,
@@ -495,7 +518,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     protected void addTimeStamp(ClassNode node) {
     }
 
-    private static void checkReturnInObjectInitializer(List init) {
+    private static void checkReturnInObjectInitializer(List<Statement> init) {
         CodeVisitorSupport cvs = new CodeVisitorSupport() {
             @Override
             public void visitClosureExpression(ClosureExpression expression) {
@@ -506,8 +529,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 throw new RuntimeParserException("'return' is not allowed in object initializer",
statement);
             }
         };
-        for (Iterator iterator = init.iterator(); iterator.hasNext();) {
-            Statement stm = (Statement) iterator.next();
+        for (Statement stm : init) {
             stm.visit(cvs);
         }
     }
@@ -554,7 +576,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
     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())) {
+        if (MopWriter.isMopMethod(node.getName())) {
             throw new RuntimeParserException("Found unexpected MOP methods in the class node
for " + classNode.getName() +
                     "(" + node.getName() + ")", classNode);
         }
@@ -596,25 +618,18 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         // method is in current class, nothing to be done
         if (m.getDeclaringClass() == this.getClassNode()) return false;
         // do not overwrite final
-        if ((m.getModifiers() & ACC_FINAL) != 0) return false;
+        if (isFinal(m.getModifiers())) return false;
         return true;
     }
 
     public void visitProperty(PropertyNode node) {
         String name = node.getName();
         FieldNode field = node.getField();
-        int propNodeModifiers = node.getModifiers();
 
         String getterName = "get" + capitalize(name);
         String setterName = "set" + capitalize(name);
 
-        // GROOVY-3726: clear volatile, transient modifiers so that they don't get applied
to methods
-        if ((propNodeModifiers & Modifier.VOLATILE) != 0) {
-            propNodeModifiers -= Modifier.VOLATILE;
-        }
-        if ((propNodeModifiers & Modifier.TRANSIENT) != 0) {
-            propNodeModifiers -= Modifier.TRANSIENT;
-        }
+        int accessorModifiers = PropertyNodeUtils.adjustPropertyModifiersForMethod(node);
 
         Statement getterBlock = node.getGetterBlock();
         if (getterBlock == null) {
@@ -631,17 +646,15 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         if (setterBlock == null) {
             // 2nd arg false below: though not usual, allow setter with non-void return type
             MethodNode setter = classNode.getSetterMethod(setterName, false);
-            if (!node.isPrivate() &&
-                    (propNodeModifiers & ACC_FINAL) == 0 &&
-                    methodNeedsReplacement(setter)) {
+            if (!node.isPrivate() && !isFinal(accessorModifiers) && methodNeedsReplacement(setter))
{
                 setterBlock = createSetterBlock(node, field);
             }
         }
 
-        int getterModifiers = propNodeModifiers;
+        int getterModifiers = accessorModifiers;
         // don't make static accessors final
-        if (node.isStatic() && (propNodeModifiers & Modifier.FINAL) != 0) {
-            getterModifiers -= Modifier.FINAL;
+        if (node.isStatic()) {
+            getterModifiers = ~Modifier.FINAL & getterModifiers;
         }
         if (getterBlock != null) {
             MethodNode getter =
@@ -662,7 +675,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         if (setterBlock != null) {
             Parameter[] setterParameterTypes = {new Parameter(node.getType(), "value")};
             MethodNode setter =
-                    new MethodNode(setterName, propNodeModifiers, ClassHelper.VOID_TYPE,
setterParameterTypes, ClassNode.EMPTY_ARRAY, setterBlock);
+                    new MethodNode(setterName, accessorModifiers, ClassHelper.VOID_TYPE,
setterParameterTypes, ClassNode.EMPTY_ARRAY, setterBlock);
             setter.setSynthetic(true);
             addPropertyMethod(setter);
             visitMethod(setter);
@@ -673,15 +686,14 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         classNode.addMethod(method);
         // GROOVY-4415 / GROOVY-4645: check that there's no abstract method which corresponds
to this one
         List<MethodNode> abstractMethods = classNode.getAbstractMethods();
-        if (abstractMethods==null) return;
+        if (abstractMethods == null) return;
         String methodName = method.getName();
         Parameter[] parameters = method.getParameters();
         ClassNode methodReturnType = method.getReturnType();
         for (MethodNode node : abstractMethods) {
             if (!node.getDeclaringClass().equals(classNode)) continue;
-            if (node.getName().equals(methodName)
-                    && node.getParameters().length==parameters.length) {
-                if (parameters.length==1) {
+            if (node.getName().equals(methodName) && node.getParameters().length
== parameters.length) {
+                if (parameters.length == 1) {
                     // setter
                     ClassNode abstractMethodParameterType = node.getParameters()[0].getType();
                     ClassNode methodParameterType = parameters[0].getType();
@@ -700,9 +712,6 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         }
     }
 
-    // Implementation methods
-    //-------------------------------------------------------------------------
-
     public interface DefaultArgsAction {
         void call(ArgumentListExpression arguments, Parameter[] newParams, MethodNode method);
     }
@@ -731,7 +740,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     }
 
                     // check whether closure shared variables refer to params with default
values (GROOVY-5632)
-                    if (argument instanceof ClosureExpression)  {
+                    if (argument instanceof ClosureExpression) {
                         final List<Parameter> newMethodNodeParameters = Arrays.asList(newParams);
 
                         CodeVisitorSupport visitor = new CodeVisitorSupport() {
@@ -741,7 +750,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                                 if (!(v instanceof Parameter)) return;
 
                                 Parameter param = (Parameter) v;
-                                if (param.hasInitialExpression() && code.getVariableScope().getDeclaredVariable(param.getName())
== null && !newMethodNodeParameters.contains(param))  {
+                                if (param.hasInitialExpression() && code.getVariableScope().getDeclaredVariable(param.getName())
== null && !newMethodNodeParameters.contains(param)) {
 
                                     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()));
@@ -767,7 +776,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 }
 
                 List<AnnotationNode> annotations = method.getAnnotations();
-                if(annotations != null) {
+                if (annotations != null) {
                     newMethod.addAnnotations(annotations);
                 }
                 MethodNode oldMethod = node.getDeclaredMethod(method.getName(), newParams);
@@ -805,8 +814,8 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
      * Creates a new helper method for each combination of default parameter expressions
      */
     protected void addDefaultParameters(List methods, DefaultArgsAction action) {
-        for (Iterator iter = methods.iterator(); iter.hasNext();) {
-            MethodNode method = (MethodNode) iter.next();
+        for (Object next : methods) {
+            MethodNode method = (MethodNode) next;
             if (method.hasDefaultValue()) {
                 addDefaultParameters(action, method);
             }
@@ -821,7 +830,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         for (int i = size - 1; i >= 0; i--) {
             Parameter parameter = parameters[i];
             if (parameter != null && parameter.hasInitialExpression()) {
-                paramValues.add(Integer.valueOf(i));
+                paramValues.add(i);
                 paramValues.add(
                         new CastExpression(
                                 parameter.getType(),
@@ -837,32 +846,36 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             ArgumentListExpression arguments = new ArgumentListExpression();
             int index = 0;
             int k = 1;
-            for (int i = 0; i < parameters.length; i++) {
-                if (k > counter - j && parameters[i] != null && parameters[i].hasInitialExpression())
{
-                    arguments.addExpression(
-                            new CastExpression(
-                                    parameters[i].getType(),
-                                    parameters[i].getInitialExpression()
-                            )
-                    );
-                    k++;
-                } else if (parameters[i] != null && parameters[i].hasInitialExpression())
{
-                    newParams[index++] = parameters[i];
-                    arguments.addExpression(
-                            new CastExpression(
-                                    parameters[i].getType(),
-                                    new VariableExpression(parameters[i].getName())
-                            )
-                    );
-                    k++;
+            for (Parameter parameter : parameters) {
+                if (parameter == null) {
+                    throw new GroovyBugError("Parameter should not be null for method " +
methodNode.getName());
                 } else {
-                    newParams[index++] = parameters[i];
-                    arguments.addExpression(
-                            new CastExpression(
-                                    parameters[i].getType(),
-                                    new VariableExpression(parameters[i].getName())
-                            )
-                    );
+                    if (k > counter - j && parameter.hasInitialExpression()) {
+                        arguments.addExpression(
+                                new CastExpression(
+                                        parameter.getType(),
+                                        parameter.getInitialExpression()
+                                )
+                        );
+                        k++;
+                    } else if (parameter.hasInitialExpression()) {
+                        newParams[index++] = parameter;
+                        arguments.addExpression(
+                                new CastExpression(
+                                        parameter.getType(),
+                                        new VariableExpression(parameter.getName())
+                                )
+                        );
+                        k++;
+                    } else {
+                        newParams[index++] = parameter;
+                        arguments.addExpression(
+                                new CastExpression(
+                                        parameter.getType(),
+                                        new VariableExpression(parameter.getName())
+                                )
+                        );
+                    }
                 }
             }
             action.call(arguments, newParams, method);
@@ -895,7 +908,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                         }
                     });
 
-            List<Statement> swapCall= new ArrayList<Statement>(1);
+            List<Statement> swapCall = new ArrayList<Statement>(1);
             swapCall.add(seq);
             node.addStaticInitializerStatements(swapCall, true);
         }
@@ -967,7 +980,6 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         constructorNode.setCode(newBlock);
 
 
-
         if (!staticStatements.isEmpty()) {
             if (isEnum) {
                 /*
@@ -1122,7 +1134,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
         final Field[] fields = clazz.getFields();
         for (int i = 0; i != fields.length; ++i) {
-            if (Modifier.isStatic(fields[i].getModifiers())) {
+            if (isStatic(fields[i].getModifiers())) {
                 final String name = fields[i].getName();
                 if (name.startsWith(__TIMESTAMP__)) {
                     try {
@@ -1147,7 +1159,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
         List<MethodNode> declaredMethods = new ArrayList<MethodNode>(classNode.getMethods());
         // remove all static, private and package private methods
-        for (Iterator methodsIterator = declaredMethods.iterator(); methodsIterator.hasNext();)
{
+        for (Iterator methodsIterator = declaredMethods.iterator(); methodsIterator.hasNext();
) {
             MethodNode m = (MethodNode) methodsIterator.next();
             abstractMethods.remove(m.getTypeDescriptor());
             if (m.isStatic() || !(m.isPublic() || m.isProtected())) {
@@ -1272,7 +1284,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             boolean oldM = ClassHelper.isPrimitiveType(oldMethod.getReturnType());
             boolean newM = ClassHelper.isPrimitiveType(overridingMethod.getReturnType());
             if (oldM || newM) {
-                String message="";
+                String message = "";
                 if (oldM && newM) {
                     message = " with old and new method having different primitive return
types";
                 } else if (newM) {
@@ -1282,9 +1294,9 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 }
                 throw new RuntimeParserException(
                         "Cannot override method " +
-                            oldMethod.getTypeDescriptor() +
-                            " in " + oldMethod.getDeclaringClass().getName() +
-                            message,
+                                oldMethod.getTypeDescriptor() +
+                                " in " + oldMethod.getDeclaringClass().getName() +
+                                message,
                         overridingMethod);
             }
         }
@@ -1303,20 +1315,19 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         instructions.add(
                 new BytecodeInstruction() {
                     public void visit(MethodVisitor mv) {
-                        mv.visitVarInsn(ALOAD,0);
+                        mv.visitVarInsn(ALOAD, 0);
                         Parameter[] para = oldMethod.getParameters();
                         Parameter[] goal = overridingMethod.getParameters();
                         int doubleSlotOffset = 0;
                         for (int i = 0; i < para.length; i++) {
                             ClassNode type = para[i].getType();
-                            BytecodeHelper.load(mv, type, i+1+doubleSlotOffset);
-                            if (type.redirect()==ClassHelper.double_TYPE ||
-                                type.redirect()==ClassHelper.long_TYPE)
-                            {
+                            BytecodeHelper.load(mv, type, i + 1 + doubleSlotOffset);
+                            if (type.redirect() == ClassHelper.double_TYPE ||
+                                    type.redirect() == ClassHelper.long_TYPE) {
                                 doubleSlotOffset++;
                             }
                             if (!type.equals(goal[i].getType())) {
-                                BytecodeHelper.doCast(mv,goal[i].getType());
+                                BytecodeHelper.doCast(mv, goal[i].getType());
                             }
                         }
                         mv.visitMethodInsn(INVOKEVIRTUAL, BytecodeHelper.getClassInternalName(classNode),
overridingMethod.getName(), BytecodeHelper.getMethodDescriptor(overridingMethod.getReturnType(),
overridingMethod.getParameters()), false);
@@ -1341,7 +1352,9 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     }
 
     private boolean isArrayAssignable(ClassNode node, ClassNode testNode) {
-        if (node.isArray() && testNode.isArray()) { return isArrayAssignable(node.getComponentType(),
testNode.getComponentType()); }
+        if (node.isArray() && testNode.isArray()) {
+            return isArrayAssignable(node.getComponentType(), testNode.getComponentType());
+        }
         return isAssignable(node, testNode);
     }
 
@@ -1360,8 +1373,8 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     }
 
     private void storeMissingCovariantMethods(Collection methods, MethodNode method, Map
methodsToAdd, Map genericsSpec, boolean ignoreError) {
-        for (Object method1 : methods) {
-            MethodNode toOverride = (MethodNode) method1;
+        for (Object next : methods) {
+            MethodNode toOverride = (MethodNode) next;
             MethodNode bridgeMethod = getCovariantImplementation(toOverride, method, genericsSpec,
ignoreError);
             if (bridgeMethod == null) continue;
             methodsToAdd.put(bridgeMethod.getTypeDescriptor(), bridgeMethod);
@@ -1397,7 +1410,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 = Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC;
         String name = SWAP_INIT;
         BlockStatement methodCode = new BlockStatement();
 
@@ -1405,7 +1418,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         boolean swapInitRequired = false;
         for (FieldNode fn : node.getFields()) {
             if (!fn.isStatic() || !fn.isSynthetic() || !fn.getName().startsWith("$const$"))
continue;
-            if (fn.getInitialExpression()==null) continue;
+            if (fn.getInitialExpression() == null) continue;
             final FieldExpression fe = new FieldExpression(fn);
             if (fn.getType().equals(ClassHelper.REFERENCE_TYPE)) fe.setUseReferenceDirectly(true);
             ConstantExpression init = (ConstantExpression) fn.getInitialExpression();
@@ -1435,8 +1448,8 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
      * Some constant expressions are optimized to return primitive types, but not all primitives
are
      * handled. This method guarantees to return a similar constant expression but with a
primitive type
      * instead of a boxed type.
-     *
-     * Additionaly, single char strings are converted to 'char' types.
+     * <p/>
+     * Additionally, single char strings are converted to 'char' types.
      *
      * @param constantExpression a constant expression
      * @return the same instance of constant expression if the type is already primitive,
or a primitive
@@ -1444,12 +1457,12 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
      */
     public static ConstantExpression transformToPrimitiveConstantIfPossible(ConstantExpression
constantExpression) {
         Object value = constantExpression.getValue();
-        if (value ==null) return constantExpression;
+        if (value == null) return constantExpression;
         ConstantExpression result;
         ClassNode type = constantExpression.getType();
         if (ClassHelper.isPrimitiveType(type)) return constantExpression;
-        if (value instanceof String && ((String)value).length()==1) {
-            result = new ConstantExpression(((String)value).charAt(0));
+        if (value instanceof String && ((String) value).length() == 1) {
+            result = new ConstantExpression(((String) value).charAt(0));
             result.setType(ClassHelper.char_TYPE);
         } else {
             type = ClassHelper.getUnwrapper(type);
@@ -1465,7 +1478,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
 
         public SwapInitStatement() {
             super(new SwapInitInstruction());
-            ((SwapInitInstruction)getInstructions().get(0)).statement = this;
+            ((SwapInitInstruction) getInstructions().get(0)).statement = this;
         }
 
         @Override

http://git-wip-us.apache.org/repos/asf/groovy/blob/69ace4d8/src/test/groovy/bugs/Groovy7969Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy7969Bug.groovy b/src/test/groovy/bugs/Groovy7969Bug.groovy
new file mode 100644
index 0000000..edcaed4
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7969Bug.groovy
@@ -0,0 +1,33 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+class Groovy7969Bug extends GroovyTestCase {
+    void testBindablePropertySettersHaveValidModifiersForMethod() {
+        assertScript """
+            import groovy.beans.Bindable
+
+            @Bindable class Bar {
+                volatile Date then
+            }
+
+            assert Bar.getMethod('setThen', Date).modifiers == 1
+        """
+    }
+}


Mime
View raw message