Implicit this handling in Closures

[KaiSchwier]

Method calls and property access using an implicit 'this' in Closures aren't handled correctly. They don't respect the delegation mechanics, but instead it always becomes an explicit 'this', resulting in wrong behaviour.

[kohsuke]

Do you have any simple script that illustrates the point?

[KaiSchwier]

Sure:

class DeleClass {
    def delegateMethod() { println("foo") }
}

def delegateTest() {
    { -> 
        delegate = new DeleClass();
        delegateMethod()
    }() 
}

def ownerTest() {
    { -> 
        delegate = new DeleClass();
        { -> delegateMethod() }()
    }() 
}

delegateTest();
ownerTest();

The delegateMethod call becomes a MethodCallExpression(implicitThis=true, VariableExpression("this"), methodName="delegateMethod") in the AST. The SandboxTransformer currently doesn't respect the implicitThis=true and changes it to an explicit "this" -> It tries to call delegateMethod on the Script-class, where it doesn't exist. delegateTest tests that the delegate is respected and ownerTest also tests, that the owner is respected.

My first thought was to modify the AST to something like this (you kinda need to get a reference to the closure within the closure?):

def delegateTest() {
    (closure1 = { -> 
        delegate = new DeleClass();
        closure1.delegateMethod()
    })() 
}

def ownerTest() {
    (closure2 = { -> 
        delegate = new DeleClass();
        (closure3 = { -> closure3.delegateMethod() })()
    })() 
}

But this isn't a working solution either because it is quite common to do

closureClone = closure.clone()
closureClone.delegate = something
closureClone()

Some notes:

• PropertyExpression (and thus AttributeExpression) can be implicitThis, too
• the delegation magic is in "public invokeMethod(Class, Object, String, Object[], boolean, boolean)" of MetaClassImpl
• It would be nice for the Checker to do the delegation magic, too, so the Intercepter sees the receiver on which the method is actually called in the end.

[KaiSchwier]

By the way, the setting of delegate doesn't get intercepted within the closure, because it becomes a BinaryExpression(VariableExpression(DynamicVariable("delegate")), ...) and VariableExpressions are currently ignored. In this case it actually is setting the property of the closure though. Don't know why Groovy does it that way... "closureClone.delegate = something" becomes a PropertyExpression as it should be.

[sergiufalcusan]

I have the same issue. Does anyone found a fix for this ?

[kohsuke]

There's some remaining issue captured in issue #11, which I'm tracking separately. Otherwise fixed for 1.5.