[GROOVY-2966] IllegalArgumentException from @Bindable AST Transformation with Integer properties

• Type: Bug
• Status: Closed
• Priority: Blocker
• Resolution: Fixed
• Fix Version/s: 1.5.7, 1.6-beta-2
• Component/s: groovy-jdk
• Environment: OpenSUSE 11.0 java 1.6.0_06 groovy 1.6 beta1
• Assignee: Jochen Theodorou
• Reporter: Chris Reeves
• Created: 22/Jul/08 12:38
• Updated: 22/Aug/08 10:25
• Resolved: 22/Aug/08 10:25

Description

If you decorate a class ( I suppose a property would do ) with @Bindable using the new 1.6 AST Transformations, Integer property changes that happen via setter will cause an IllegalArgumentException because the firePropertyChange method will be called using the String,int,int signature. If the property or the new value is null, obviously, this is not a legal state. Of course, if the field is initialized and never set to null, this never happens.

A test case for the groovy console:

// start

@groovy.beans.Bindable
class Test {
String foo
Integer bar
}

def t = new Test()

// this is ok
t.foo = 'foo'

// this is not
t.bar = 10

// end

the proposed fix for groovy.beans.BindableASTTransformation.java:

/**

• Creates a statement body similar to:
• <code>
• Object oldValue = field
• field = value
• pcsField.firePropertyChange("field", (Object) oldValue, value)
• </code>
  *
• @param propertyNode the field node for the property
• @param fieldExpression a field expression for setting the property value
• @return the created statement
  */
  protected Statement createBindableStatement(PropertyNode propertyNode, Expression fieldExpression) {
  // create statementBody
  BlockStatement bs = new BlockStatement();

bs.addStatement(new ExpressionStatement(
new DeclarationExpression(new VariableExpression("oldValue"),
Token.newSymbol(Types.EQUALS, 0, 0),
fieldExpression)));
bs.addStatement(new ExpressionStatement(
new BinaryExpression(fieldExpression,
Token.newSymbol(Types.EQUAL, 0, 0),
new VariableExpression("value"))));
bs.addStatement(new ExpressionStatement(new MethodCallExpression(
new FieldExpression(pcsField), "firePropertyChange",
new ArgumentListExpression(new Expression[]

{ new ConstantExpression(propertyNode.getName()), new CastExpression(ClassHelper.OBJECT_TYPE, new VariableExpression("oldValue"), true), new VariableExpression("value") }

))));

return bs;
}

Move to the slightly more standard/common/I-don't-really-know-if-it-matters-or-not way of firing property change events i.e. already have the property set to the new value in case the listener uses that in some way. Also, cast the "old" value to an Object so that it will select the proper method signature.

I am by no means an expert here, so there may be other changes needed to make this compatible with java 1.4, or whatever the compatibility target is, like casting both values, or doing some manual boxing. My AST may be horribly inefficient... I have no idea.