groovy

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

Details

  • Type: Bug Bug
  • Status: Closed 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.
  • Number of attachments :
    4

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
    12/Nov/09 9:13 PM
    0.6 kB
    Roshan Dawrani
  2. groovy3878_mixin_varargs.patch
    12/Nov/09 4:23 AM
    1 kB
    Paul King
  3. NewDsl.java
    12/Nov/09 9:13 PM
    0.7 kB
    Roshan Dawrani
  4. TestNewDsl.groovy
    12/Nov/09 10:22 PM
    0.8 kB
    Roshan Dawrani

Activity

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")
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.
Hide
blackdrag blackdrag 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
blackdrag blackdrag 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
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.
Hide
blackdrag blackdrag added a comment -

looks good to me

Show
blackdrag blackdrag 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.
Hide
Aslak Hellesoy added a comment -

Thanks for fixing this guys!

Show
Aslak Hellesoy added a comment - Thanks for fixing this guys!

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: