You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2016/10/13 07:28:36 UTC
groovy git commit: GROOVY-7969: Incorrect modifers on setter for
volatile property with @Bindable/@Vetoable (closes #448)
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 <pa...@asert.com.au>
Authored: Wed Oct 12 21:42:17 2016 +1000
Committer: paulk <pa...@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
+ """
+ }
+}