groovy
  1. groovy
  2. GROOVY-3405

static methods added through metaClass can't have default values for parameters

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.6.1, 1.7-beta-1
    • Component/s: class generator
    • Labels:
      None
    • Environment:
      This is seen in groovy 1.6.0 on Leopard:

      % groovy -version
      Groovy Version: 1.6.0 JVM: 1.6.0_07
    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      I'm running into an issue with Groovy 1.6 trying to add a static method to the metaClass where the closure for the method has some parameters with default values.

      When trying to call the method and not supplying some default values, it doesn't find the newly added method and instead throws a MissingMethodException.

      Default parameters work fine with a class where the static method is defined directly on the class, both through using a closure as well as a normal method definition:

      class MyClass {
      def testInstance(first = "foo")

      { return "$first" }

      static testStatic(first = "foo") { return "$first" }

      static testStaticClosure =

      { first = "foo" -> return "$first" }

      static testStaticTwoParms(first = "foo", second = "bar")

      { return "$first - $second" }

      static testStaticTwoParmsClosure =

      { first = "foo", second = "bar" -> return "$first - $second" }


      }

      assert "foo" == new MyClass().testInstance()
      assert "foo" == MyClass.testStatic()
      assert "foo" == MyClass.testStaticClosure()
      assert "foo - bar" == MyClass.testStaticTwoParms()
      assert "foo - bar" == MyClass.testStaticTwoParmsClosure()

      Using the metaClass, it works fine to add instance methods with single or multiple defaulted values:

      String.metaClass.testInstance =

      { first = "foo" -> return "$first" }

      assert "foo" == "".testInstance() // works as expected

      String.metaClass.testInstanceTwoParams = { first = "foo", second = "bar" -> return "$first - $second" }

      assert "foo - bar" == "".testInstanceTwoParams() // also works as expected


      But when a static method is added to a class via the metaClass, a single default parameter doesn't use the default value for the paramter, it has a null value:

      String.metaClass.'static'.testStatic = { first = "foo" -> return "$first" }

      assert "qux" == "".testStatic("qux") // works if you specify a parameter
      assert "foo" == "".testStatic() // Fails! "foo" isn't returned, instead it returns a null value

      It gets weirder if you try to specify two defaulted parameters on the static closure. Instead of just not assigning values to the parameters, it doesn't even find the method and throws a MissingMethodException:

      String.metaClass.'static'.testStaticTwoParams =

      { first = "foo", second = "bar" -> return "$first - $second" }

      assert "baz - qux" == "".testStaticTwoParams("baz", "qux") // works if you specify parameters
      assert "baz - bar" == "".testStaticTwoParams("baz") // Fails with MissingMethodException!
      assert "foo - bar" == "".testStaticTwoParams() // Fails with MissingMethodException! Not finding method with defaulted pararameters

      This is seen in groovy 1.6.0:

      % groovy -version
      Groovy Version: 1.6.0 JVM: 1.6.0_07

      I also tested this code against 1.5.7 and it exhibits the same behavior, but is also broken for metaClass added instance methods with defaulted values, which work in 1.6. Hopefully whatever was changed in 1.6 to fix that can also be applied to class level static methods.

        Activity

        Hide
        Roshan Dawrani added a comment -

        Hi,
        Attaching a patch to solve the reported issue.

        There was a problem with registering closure with default parameters as static methods with EMC.

        So, say, if a closure has one parameter, which has default value, it had 2 doCall() methods - doCall() and doCall(Object).

        If this closure is registered as an instance method "x" with EMC, then metaClass gets x() as well as x(Object) pointing to doCall()/doCall(Object) internally.

        Now, if this closure gets registered as a static method "y", then metaClass was by mistake registering only y(Object) and not y(). This was because it was using the same closure parameter types when trying to register both y() and y(Object) and it was successfully adding y(Object) but registering of y() was failing because even for it, it was seeing the same parameter types.

        The fix makes ClosureStaticMetaMethod instances with correct parameterTypes and all combinations of parameter-types get added appropriately. So, for a closure with 2 parameter values with default values that maps to static property z added through metaClass, all z() / z(Object) / z(Object/Object) get registered now.

        Example 1:

        def cl = { first = "first" -> "$first"}
        String.metaClass.'static'.cl1 = cl
        
        // only cl1(Object) is successully registered but because of 1 parameter special handling, call goes through with null value
        println "".cl1()
        

        Example 2:

        def cl = { first = "first", second = "second" -> "$first - $second"}
        String.metaClass.'static'.cl1 = cl
        
        // only cl1(Object, Object) is successully registered, so failes with no method found excaption
        println "".cl1()
        

        Please let me know if the fix looks ok. If yes, I will add a test case and check-in.

        rgds,
        Roshan

        Show
        Roshan Dawrani added a comment - Hi, Attaching a patch to solve the reported issue. There was a problem with registering closure with default parameters as static methods with EMC. So, say, if a closure has one parameter, which has default value, it had 2 doCall() methods - doCall() and doCall(Object). If this closure is registered as an instance method "x" with EMC, then metaClass gets x() as well as x(Object) pointing to doCall()/doCall(Object) internally. Now, if this closure gets registered as a static method "y", then metaClass was by mistake registering only y(Object) and not y(). This was because it was using the same closure parameter types when trying to register both y() and y(Object) and it was successfully adding y(Object) but registering of y() was failing because even for it, it was seeing the same parameter types. The fix makes ClosureStaticMetaMethod instances with correct parameterTypes and all combinations of parameter-types get added appropriately. So, for a closure with 2 parameter values with default values that maps to static property z added through metaClass, all z() / z(Object) / z(Object/Object) get registered now. Example 1: def cl = { first = "first" -> "$first" } String .metaClass.' static '.cl1 = cl // only cl1( Object ) is successully registered but because of 1 parameter special handling, call goes through with null value println "".cl1() Example 2: def cl = { first = "first" , second = "second" -> "$first - $second" } String .metaClass.' static '.cl1 = cl // only cl1( Object , Object ) is successully registered, so failes with no method found excaption println "".cl1() Please let me know if the fix looks ok. If yes, I will add a test case and check-in. rgds, Roshan
        Hide
        Roshan Dawrani added a comment -

        The examples in the previous code are to highlight the issue with some explanation but before the fix . After the fix, they behave correctly.

        Show
        Roshan Dawrani added a comment - The examples in the previous code are to highlight the issue with some explanation but before the fix . After the fix, they behave correctly.
        Hide
        Roshan Dawrani added a comment -

        Hi Jochen, please let me know if you got the chance to look at the fix patch. Actually it is a straightforward one.

        Show
        Roshan Dawrani added a comment - Hi Jochen, please let me know if you got the chance to look at the fix patch. Actually it is a straightforward one.
        Hide
        blackdrag blackdrag added a comment -

        looks ok to me

        Show
        blackdrag blackdrag added a comment - looks ok to me
        Hide
        Roshan Dawrani added a comment -

        Fixed on 1.6.1 and trunk.

        Show
        Roshan Dawrani added a comment - Fixed on 1.6.1 and trunk.

          People

          • Assignee:
            Roshan Dawrani
            Reporter:
            Ted Naleid
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: