Groovy
  1. Groovy
  2. GROOVY-3878

Can't call Java varargs method when it's mixed in to toplevel

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.5, 1.7-beta-2
    • Fix Version/s: 1.6.6, 1.7-rc-1
    • Component/s: None
    • Labels:
      None
    • Environment:
      Java 1.5, OS X Leopard.

      Description

      Take this Java class:

      Dsl.java
      public class Dsl {
          public static void novarargs(java.util.List s) {
              System.out.println("novarargs ok");
          }
      
          public static void varargs(Object... s) {
              System.out.println("varargs ok");
          }
      }
      

      Run it with Groovy:

      UseDsl.groovy
      this.metaClass.mixin(Dsl)
      
      Dsl.novarargs(["a", "b"])
      novarargs(["a", "b"])
      
      Dsl.varargs("a", "b")
      varargs("a", "b")    // fails here
      

      The last one fails:

      javac Dsl.java && groovy UseDsl.groovy 
      
      Caught: java.lang.IllegalArgumentException: wrong number of arguments
      	at UseDsl.run(UseDsl.groovy:7)
      
      1. 3678_v17x.txt
        0.6 kB
        Roshan Dawrani
      2. groovy3878_mixin_varargs.patch
        1 kB
        paulk_asert
      3. NewDsl.java
        0.7 kB
        Roshan Dawrani
      4. TestNewDsl.groovy
        0.8 kB
        Roshan Dawrani

        Activity

        Aslak Hellesoy created issue -
        Hide
        Paul King added a comment - - edited

        One ugly workaround until fixed:

        varargs(["a", "b"] as Object[])
        

        Also noted that normal category usage appears to work fine:

        use(Dsl) {
            varargs("a", "b")
        }
        
        Show
        Paul King added a comment - - edited One ugly workaround until fixed: varargs([ "a" , "b" ] as Object []) Also noted that normal category usage appears to work fine: use(Dsl) { varargs( "a" , "b" ) }
        Hide
        Paul King added a comment - - edited

        Just debugging what is happening (using the above script):

        println this.metaClass.methods.grep(~'.*vararg.*').join('\n')
        

        yields:

        org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaMethod@1cd2d49[name: varargs params: [class [Ljava.lang.Object;] returns: void owner: class ConsoleScript9]
        org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaMethod@12a09d5[name: novarargs params: [interface java.util.List] returns: void owner: class ConsoleScript9]
        
        Show
        Paul King added a comment - - edited Just debugging what is happening (using the above script): println this .metaClass.methods.grep(~'.*vararg.*').join('\n') yields: org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaMethod@1cd2d49[name: varargs params: [class [Ljava.lang.Object;] returns: void owner: class ConsoleScript9] org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaMethod@12a09d5[name: novarargs params: [interface java.util.List] returns: void owner: class ConsoleScript9]
        Hide
        Paul King added a comment -

        In fact, once this is run:

        varargs(["a", "b"] as Object[])
        

        then this works from that point on:

        varargs("a", "b")
        
        Show
        Paul King added a comment - In fact, once this is run: varargs([ "a" , "b" ] as Object []) then this works from that point on: varargs( "a" , "b" )
        Paul King made changes -
        Field Original Value New Value
        Assignee Paul King [ paulk_asert ]
        Hide
        Paul King added a comment -

        Planning on applying the attached patch in a day or two unless someone can think of a better way to fix this.

        Show
        Paul King added a comment - Planning on applying the attached patch in a day or two unless someone can think of a better way to fix this.
        Paul King made changes -
        Attachment groovy3878_mixin_varargs.patch [ 45803 ]
        Hide
        Jochen Theodorou added a comment -

        Paul, that might fix varargs(Object... s), but it does not fix varargs(String... s) or varargs(Object a, Object... s) or most other cases. So I am for not applying that patch, it leads to the false assumption the bug has been fixed and will later cause problems again

        Show
        Jochen Theodorou added a comment - Paul, that might fix varargs(Object... s), but it does not fix varargs(String... s) or varargs(Object a, Object... s) or most other cases. So I am for not applying that patch, it leads to the false assumption the bug has been fixed and will later cause problems again
        Hide
        Paul King added a comment -

        Yes, I found the first case you mentioned straight after posting the previous patch. I assumed the second case was not broken but it too is broken. Looks better to hook into some existing code if we can.

        Show
        Paul King added a comment - Yes, I found the first case you mentioned straight after posting the previous patch. I assumed the second case was not broken but it too is broken. Looks better to hook into some existing code if we can.
        Hide
        Roshan Dawrani added a comment -

        Attaching an alternative patch for review and its test files.

        Test can be run as:

        javac NewDsl.java && groovy TestNewDsl.groovy
        
        Show
        Roshan Dawrani added a comment - Attaching an alternative patch for review and its test files. Test can be run as: javac NewDsl.java && groovy TestNewDsl.groovy
        Roshan Dawrani made changes -
        Attachment TestNewDsl.groovy [ 45821 ]
        Attachment NewDsl.java [ 45820 ]
        Attachment 3678_v17x.txt [ 45819 ]
        Roshan Dawrani made changes -
        Attachment TestNewDsl.groovy [ 45821 ]
        Hide
        Roshan Dawrani added a comment -

        Attaching a slightly easier version of TestNewDsl.groovy for review.

        Show
        Roshan Dawrani added a comment - Attaching a slightly easier version of TestNewDsl.groovy for review.
        Roshan Dawrani made changes -
        Attachment TestNewDsl.groovy [ 45822 ]
        Hide
        Jochen Theodorou added a comment -

        looks good to me

        Show
        Jochen Theodorou added a comment - looks good to me
        Hide
        Paul King added a comment -

        I had a similar solution to Roshan's though I had to do a pre-emptive call to method.getParameterTypes() in order for the testcase that I added to pass. The only difference is that my testcase uses a Groovy mixin class and is located in the same script. It would be nice to push that logic somewhere else but I haven't had time yet to investigate.

        Show
        Paul King added a comment - I had a similar solution to Roshan's though I had to do a pre-emptive call to method.getParameterTypes() in order for the testcase that I added to pass. The only difference is that my testcase uses a Groovy mixin class and is located in the same script. It would be nice to push that logic somewhere else but I haven't had time yet to investigate.
        Paul King made changes -
        Fix Version/s 1.6.6 [ 15781 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 1.7-rc-1 [ 14666 ]
        Resolution Fixed [ 1 ]
        Hide
        Aslak Hellesoy added a comment -

        Thanks for fixing this guys!

        Show
        Aslak Hellesoy added a comment - Thanks for fixing this guys!
        Paul King made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Mark Thomas made changes -
        Project Import Sun Apr 05 13:32:57 UTC 2015 [ 1428240777691 ]
        Mark Thomas made changes -
        Workflow jira [ 12732845 ] Default workflow, editable Closed status [ 12744688 ]
        Mark Thomas made changes -
        Project Import Mon Apr 06 02:11:23 UTC 2015 [ 1428286283443 ]
        Mark Thomas made changes -
        Workflow jira [ 12973657 ] Default workflow, editable Closed status [ 12980813 ]
        Mark Thomas made changes -
        Assignee paulk_asert [ paulk_asert ] Paul King [ paulk ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        1d 9h 49m 1 Paul King 13/Nov/09 07:20
        Resolved Resolved Closed Closed
        26d 10h 56m 1 Paul King 09/Dec/09 18:16

          People

          • Assignee:
            Paul King
            Reporter:
            Aslak Hellesoy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development