Repository: groovy
Updated Branches:
  refs/heads/master e914966d7 -> 0515ca52c


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/0515ca52
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/0515ca52
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/0515ca52

Branch: refs/heads/master
Commit: 0515ca52c06560eaaa1a20391025cd3d9476627a
Parents: e914966
Author: paulk <pa...@asert.com.au>
Authored: Wed Oct 12 21:42:17 2016 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Thu Oct 13 16:43:46 2016 +1000

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


http://git-wip-us.apache.org/repos/asf/groovy/blob/0515ca52/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/0515ca52/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/0515ca52/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/0515ca52/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 58018ff..382c9a2 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:ja...@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;
         }
@@ -224,17 +247,17 @@ public class Verifier implements GroovyClassVisitor, 
Opcodes {
                 if (var instanceof VariableExpression) {
                     var = ((VariableExpression) var).getAccessedVariable();
                 }
-                if (var instanceof VariableExpression && 
Modifier.isFinal(var.getModifiers())) {
-                    throw new RuntimeParserException("The variable 
["+var.getName()+"] is declared final but is reassigned",bexp);
+                if (var instanceof VariableExpression && 
isFinal(var.getModifiers())) {
+                    throw new RuntimeParserException("The variable [" + 
var.getName() + "] is declared final but is reassigned", bexp);
                 }
-                if (var instanceof Parameter && 
Modifier.isFinal(var.getModifiers())) {
-                    throw new RuntimeParserException("The parameter 
["+var.getName()+"] is declared final but is reassigned",bexp);
+                if (var instanceof Parameter && isFinal(var.getModifiers())) {
+                    throw new RuntimeParserException("The parameter [" + 
var.getName() + "] is declared final but is reassigned", bexp);
                 }
             }
 
             @Override
             public void variableNotAlwaysInitialized(final VariableExpression 
var) {
-                throw new RuntimeParserException("The variable 
["+var.getName()+"] may be uninitialized", var);
+                throw new RuntimeParserException("The variable [" + 
var.getName() + "] may be uninitialized", var);
             }
         };
     }
@@ -267,21 +290,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,
@@ -321,7 +344,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);
                         }
@@ -364,7 +387,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,
@@ -409,7 +432,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);
@@ -431,7 +454,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,
@@ -446,7 +469,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,
@@ -466,7 +489,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,
@@ -486,7 +509,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,
@@ -525,7 +548,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) {
@@ -536,8 +559,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);
         }
     }
@@ -584,7 +606,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);
         }
@@ -626,25 +648,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) {
@@ -661,17 +676,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 =
@@ -692,7 +705,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);
@@ -703,15 +716,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();
@@ -730,9 +742,6 @@ public class Verifier implements GroovyClassVisitor, 
Opcodes {
         }
     }
 
-    // Implementation methods
-    //-------------------------------------------------------------------------
-
     public interface DefaultArgsAction {
         void call(ArgumentListExpression arguments, Parameter[] newParams, 
MethodNode method);
     }
@@ -761,7 +770,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() {
@@ -771,7 +780,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()));
@@ -797,7 +806,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);
@@ -835,8 +844,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);
             }
@@ -851,7 +860,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(),
@@ -867,32 +876,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);
@@ -925,7 +938,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);
         }
@@ -997,7 +1010,6 @@ public class Verifier implements GroovyClassVisitor, 
Opcodes {
         constructorNode.setCode(newBlock);
 
 
-
         if (!staticStatements.isEmpty()) {
             if (isEnum) {
                 /*
@@ -1163,7 +1175,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())) {
                 Long timestamp = 
getTimestampFromFieldName(fields[i].getName());
                 if (timestamp != null) {
                     return timestamp;
@@ -1184,7 +1196,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())) {
@@ -1309,7 +1321,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) {
@@ -1319,9 +1331,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);
             }
         }
@@ -1340,20 +1352,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);
@@ -1378,7 +1389,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);
     }
 
@@ -1397,8 +1410,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);
@@ -1434,7 +1447,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();
 
@@ -1442,7 +1455,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();
@@ -1472,8 +1485,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
@@ -1481,12 +1494,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);
@@ -1502,7 +1515,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/0515ca52/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
+        """
+    }
+}

Reply via email to