MetaClassImpl invokes call() on binding variable even if it is not a Closure

I just noticed that the logic to call a closure in the binding is duplicated (MetaClassImpl and Script.invokeMethod). Can one of the two occurrences be removed?

I am not sure about the binding variable logic in Script.invokeMethod but the change to MetaClassImpl was done by me as part of GROOVY-3284, where it was mentioned that "for sure I can say, that call() is to be made even if x is no Closure".

Reproduce here some portion of the comments from GROOVY-3284

• A call like Foo.A() is not supposed to work because Foo.A is of type Foo and not Closure.
• you can't say it like that.... x() is supposed to resolve to x.call(), but this is a undocumented feature as we are not entirely sure if we really want to support this.

• x() is supposed to resolve to x.call() even if x is not of type Closure? Irrespective of type of x?
• kind of... as I said, it is not really specified. for sure I can say, that call() is to be made even if x is no Closure. I suspect in the MetaClass code where we test the field we test only for closures. A change here needs some experiments, because it may influence other parts.

let us change the example....

def foo = 5
foo()

do you still expect the methodmissingexception here? What if foo is a field? What if foo has no int, but a closure? What if a call() method has been added to int?

What I expect above all is consistent behavior. Therefore the duplication in MetaClassImpl and Script is dubious. When a call goes through MetaClassImpl, you get one behavior, and when it doesn't (e.g. because it goes through ClosureMetaClass), then you get another one. IMHO this needs to be fixed.

That sounds reasonable but the first thing is to decide which of the two behaviors is correct.

Unless the undocumented "resolve to x.call()" feature is made official, it seems logical to only invoke call() for closures. But I think we need Jochen's input here.

the logic for f() is:
(1) if there is a local varibale f, do f.call().
(2) if there is a method f(), invoke it
(3) if there is a property f, invoke f.call()

is this not consistent or is that problematic?

The inconsistency is the following: If the call goes through MetaClassImpl, the binding variable wins over Script.invokeMethod(). If it doesn't (e.g. because it goes through ClosureMetaClass), then it's the other way around.

so it is not about an existing method or variable, but for the case it does not exist... ok...

(1) if there is a local varibale f, do f.call().
(2) if there is a method f(), invoke it
(3) if there is a property f, invoke f.call()
(4) call invokeMethod()/methodMissing

Accroding to this scheme it is ok for the script variable to win over invokeMethod.

>so it is not about an existing method or variable, but for the case it does not exist... ok...
It's about the race between an existing script variable and Script.invokeMethod().

>Accroding to this scheme it is ok for the script variable to win over invokeMethod.
I don't see script variables mentioned in your logic, and intuitively I'd put them after invokeMethod(), just like the author of Script.invokeMethod(). Anyway, if a script variable should win over invokeMethod(), then the implementation of Script.invokeMethod() must change. Because right now, Script.invokeMethod() wins over a script variable as soon as the call occurs from within a closure (and for all other calls that don't go through MetaClassImpl).

Script variables are properties, so this goes in (3).

OK, then let's change Script.invokeMethod().

After rereading the issue I think we are in the state that we can close the issue already. If

(1) if there is a local varibale f, do f.call().
(2) if there is a method f(), invoke it
(3) if there is a property f, invoke f.call()
(4) call invokeMethod()/methodMissing

is acceptable, then the important point here is that f is found in step (3) in the binding. Script's invokeMethod is not asked, because that would have happened in (4), but because a property was found we do the call for it which fails, because Integer does not provide a call() method in this example. What if it would provide one? Wouldn't it be expected to be called? The only way to fix it in the way that you may think is right is to react to the missing method there.

If the method call is in a Closure, then yes, it behaves different. And that is the reason I don't close this issue. But this is because of the way the MOP is designed. So this will become a target for 2.0 instead