[jira] [Resolved] (GROOVY-4097) MOP implementation does not handle method calls in relation with class objects properly!

["Eric Milles (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GROOVY-4097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Eric Milles resolved GROOVY-4097.
---------------------------------
    Resolution: Fixed

https://github.com/apache/groovy/commit/cbcf64e9a233cb7ad4c64e132acf83c781561f57

NOTE: {{getMetaMethod}} and others like it specify Object[] for argument types, but do indeed expect types.  Not sure if this is legacy, but it can't be changed to Class[].  The expectation is you call "getMetaMethod('getSomething',Class)" if looking for method that accepts class.

I cleaned up a couple places where indeed call arguments are passed and types are expected, so they will now check {{Class<Class>}} instead of whatever kind of class is given in arguments array.

As for the second issue, I can't quite find the right issue to link.  GROOVY-10820 speaks to the static compilation handling of similar situations.  I would be very careful adding static methods like "getSuperclass()" or "getCanonicalName()".  If you need, you can switch to static compilation to ensure the intended method is called.  Groovy prefers to call static method declared on given class over instance method of Class when receiver is a class instance.

Maybe it would be useful to generate a warning for any static method that is an instance method of Class.

> MOP implementation does not handle method calls in relation with class objects properly!
> ----------------------------------------------------------------------------------------
>
>                 Key: GROOVY-4097
>                 URL: https://issues.apache.org/jira/browse/GROOVY-4097
>             Project: Groovy
>          Issue Type: Bug
>          Components: groovy-runtime
>    Affects Versions: 1.7.0
>         Environment: Tested on an AMD Turion64 under WinXP with Eclipse Galileo, Java 6u17, will occur on any platform though since this are groovy source code bugs.
>            Reporter: Joscha Drechsler
>            Assignee: Eric Milles
>            Priority: Critical
>         Attachments: GroovyTestClass.groovy
>
>
> In a few places, the Meta Object Protocoll implementation uses parameter objects classes to look up the matching meta methods. But if one of these arguments already is of type Class, instead of using Class<Class<?>>, the original class parameter is used. This leads to a couple of bugs.
> My attached TestCase includes two methods, each demonstrating on of these bugs.
> h4. First example:
> The first method, testClassArguments(), presents a major issue.
> The TestClass has two methods: getSomething(String) and getSomething(Class).
> Using the MetaObjectProtocol, i first retrieve the MetaMethod of getSomething (line 23) for an argument set [String] (line 22). MetaClassImpl looks up this method by taking the classes of [String] (namely  [Class<String>]), and looking for a method with a signature, that matches [Class<String>]. This works fine, the result printed (line 24) is the correct method, and the assertion (line 25) passes.
> Now i do the same thing again, but this time using an argument set [Class<String>] (line 27). getMetaMethod returns a method (line 28). But, as one can see in the output (line 29), the method is once again getSomething(String): Since MetaClassImpl internally looks up the method, by taking the parameters classes, but in the process _skipping_ class transformation for class arguments, it once again looks for a method with signature [Class<String>] instead of [Class<Class<String>>]. Now, invoking getSomething(String) with Class<String> as parameter of course leads to an IllegalArgumentException, thereby failing the test.
> h4. Second example
> This second example, method testInvokeStaticMethod(), demonstrates, how a call to getSuperclass, a normal public instance method for Class<T>, is wrongly executed as a static method call on T.
> This bug does not occur, when using a default MetaClass: since all default MetaClasses are subclasses of MetaClassImpl, and the class org.codehaus.groovy.runtime.callsite.CallSiteArray has an instanceof test at the third line of it's method createCallStaticSite(CallSite,Class,Object[]), this does not happen for any of MetaClassImpl subclasses.
> So the first thing this test does is, to replace its MetaClass with a DelegatingMetaClass, which does nothing except forward all calls to the original MetaClassImpl, but it is not a subclass of MetaClassImpl, so the mentioned instanceof test does not work.
> Firstly, the test now retrieves it's own Class object (line 38), which works as expected.
> Now, the tests asserts, that its own superclass is GroovyTestCase (line 39), but this assertion fails. The actually returned value of the clazz.getSuperclass() call is "This was the wrong method!", which is returned by the test cases static getSuperclass() method (lines 42 to 44).
> So what happened here? That is pretty simple:
> A normal static call to the getSuperclass() method at lines 42 to 44 would be translated to invokeStaticMethod(GroovyTestCase.class, "getSuperclass", []) on the MetaClass of GroovyTestCase.
> The superclass call at line 39 *should* have been translated to invokeMethod(clazz, "getSuperclass", []) on the MetaClass of Class<T>.
> But what happened instead is, since clazz == GroovyTestCase.clazz, the call site was simply "mistaken" for a static call site, and delegated to the wrong MetaClass.
> h5. some final words
> though the displayed test cases may seem a little artificially constructed, i actually stumbled over both of these bugs in my project, after renaming a few java files to groovy. So please note that these are real issues, the examples only may seem that surreal because i reduced them to a minimum. This stuff does  regularly break code when doing some heavy MOP related work.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)