Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-2966

IllegalArgumentException from @Bindable AST Transformation with Integer properties

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.5.7, 1.6-beta-2
    • Component/s: groovy-jdk
    • Labels:
      None
    • Environment:
      OpenSUSE 11.0 java 1.6.0_06 groovy 1.6 beta1

      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.

        Attachments

          Activity

            People

            • Assignee:
              blackdrag Jochen Theodorou
              Reporter:
              evschris Chris Reeves
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: