Call behavior on enums is inconsistent

I don't think if

Foo.A() and

a=Foo.A
a()

are even supposed to work as per groovy syntax.

If call is a member of enum Foo of type Closure as shown in the JIRA example, then what is supposed to work is:

Foo.A.call() and Foo.A.call.call() (same as f =

and calling it as f() and f.call()), and

a=Foo.A
a.call() and a.call.call()

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.

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.

I have readied the partial fix for supporting Foo.A() resolving to Foo.A.call() in the example given in the JIRA.

I am working on the 2nd half of the issue but I got a little confused by the previous comments.

So, this is something that needs to be supported, right? Or, there is some unsurity about it?

For the 2nd issue reported, it works if it is used as

Case 1) when a is not a dynamic variable
def z = Foo.A 
z() // prints A
z = Foo.B
z() // prints B

but not if

Case 2) when a is a dynamic variable
a=Foo.A
a()

Is the code in case 1 above an acceptable workaround?

I guess case 1 is what is implemented now.. case 2 will try to call the method named "a" and will then see that there is no such method, then it will get the property value a and see if it is a Closure, which it is not. So the call fails. The point that is open is, if we should look only at the property value. If we would simply try to execute the call method regardless the type, then it would work. I think a fix should go into that direction to complete the work the compiler does for local variables.

Ok, I will look into the issue 2 with the supplied information.

For now, I have applied the partial fix to solve the issue 1.

Now, Foo.A() gets resolved to Foo.A.call() for enum constant Foo.A.

Before the fix, the target object for the method call A() was class Foo and that's why Foo.A() was getting treated like a static call A() on class Foo, and it was failing because that method wasn't there.

Now, for Foo.A(), target is the enum constant Foo.A and it is like Foo.A() == <Foo.A>() == Foo.A.call().

I shouldn't have deleted the first part of my comment saying something like "adding a call transformation depending on the type of the compiler is a no go". Frankly I would say you should undo the commit. It goes not in the wrong direction and if case 2 is solved, then there is no need for your case1 fix too. I deleted that part of my comment because I thought that is the current state without your patch.

If Foo.A() is seen as a static method call, then what is wrong with that? Doesn't the normal method call logic look for a propoerty? Shouldn't A be a propoerty/field of Foo? Doesn't that mean that case 2 will solve case 1 automatically?

Before I go the undo commit path, just wanted to clear if there is some confusion among case 1/2 and issue 1/2 -

So, there are two issues reported:

For the enum:

enum Foo {
    A({ println("A") })
    Foo(Closure c) {
      call = c
    }
    final Closure call
}

Issue 1: Foo.A() does not get resolved to Foo.A.call(). - Right now, this is what I have tried to resolve. So, if enum Foo as a field named A, which is of type Foo (which meets the condition required for enum constants) and there is method expression like Foo.A(), then the expression gets modified to Foo.A.call(). Is that not correct? Do you mean to say that if enum Foo has a static method A() as well as an enum constant A, then there will be a problem? If that is the case, you mean static call thing is more preferable and is direction it should go in?

Issue 2: This issue says if we do
a = Foo.A
a(), then it does not work.

For this I said, that without any changes done for this issue, if we instead do
def a = Foo.A
a() - then it works and had checked if this workaround was accpetable.

I can still undo the commit but I thought I should make sure that there is no confusion between issue 1/2 and issue 2's case 1/case 2.

I will appreciate if you can tell in what kind of enum related scenarios, the fix would interfere.

I will try it your way anyway for the part-2 that is remaining.

Thanks.

for your issue 1: the static method supersedes the call convention. So yes.. if there is a static method A, then it should be called instead of doing Foo.A.call()

for issue 2: I think this is a workaround, but why letting have them define a local variable just for that? It would be better to then simply write Foo.A.call() then instead, or not? For DSLs it is better if it works without the workaround.

If you are going to do the fix as I suggested, then the potential impact is unclear to me. I would probably put that not into 1.5.x or 1.6.x, but only into 1.7, because of that.

Ok, I will first undo my changes from the 3 branches and then try to fix it as you have suggested.

And then if I am solve it that way, I will have the patch reviewed first and do the changes only on 1.7.

ok

Undid the changes made earlier.

Resolved.

Applied the new, reviewed fix on the branches 1.5.x, 1.6.x and trunk.