ASTTransformationCustomizer improvement

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

ASTTransformationCustomizer improvement

melix
Hi,

In Groovy 1.8 we released the ASTTransformationCustomizer which allows applying AST transformations transparently to scripts. For example, we can do this :

customizer = new ASTTransformationCustomizer(Log)
configuration.addCompilationCustomizers(customizer)
shell = new GroovyShell(configuration)
result = shell.evaluate("""
            class MyClass {}
            new MyClass()
""")
assert result.log.class == Logger


However, there's a limitation because we cannot pass annotation parameters to the customizer. I didn't manage to get it for 1.8.0, but I'd like to add it to 1.8.1. It's quite simple to do for regular types, for example I can already do this (locally, not supported on trunk) :

customizer = new ASTTransformationCustomizer(Log)
customizer.annotationParameters = [value: 'logger']
configuration.addCompilationCustomizers(customizer)
shell = new GroovyShell(configuration)
result = shell.evaluate("""
            class MyClass {}
            new MyClass()
""")
assert result.logger.class == Logger


But I'm still failing with closure annotations. For example, imagine the following annotation :

@Around(pattern='find.*', before={ code before }, after = { code after })
class MyClass {
   def findByName() { ... }
}

If you want to add this annotation transparently through the customizer, we have a problem because if we write this :

customizer.annotationParameters = [ pattern: 'find.*', before: { code before }, after: {code after } ]

Then the closures are compiled, and we do not have access to the closure class node at runtime. I need to create, in this case, a ClosureExpression from the existing closure instance, in order to be able to add a member to the AnnotationNode which is generated by the customizer. As far as I can see, this is not possible but there are several workarounds :

1. do not support closure annotations values through the customizer
2. write the closure content inside a string, which will be converted to ClosureExpression by the customizer
3. make the user write an ASTNode (ClosureExpression) instead of the closure (through ASTBuilder or directly I suppose)

I'm not a big fan or solution 1. because I don't like partial support. I don't like 2. either because this is an ugly solution. I can imagine solution 3. as the best solution : if we detect the annotation parameter provided to the customizer is an instance of a Closure, then we can throw an error message specifying that (s)he should replace it with a ClosureExpression.

Unless you have a magical solution to use the closure directly :)

Cédric

Reply | Threaded
Open this post in threaded view
|

Re: ASTTransformationCustomizer improvement

melix
Here are two unit tests I added with the third solution. Let me know what you think (note that the chosen closure values are stupid but make unit tests easier to write ;)).

    void testLocalTransformationWithClosureAnnotationParameter() {
        // add @Contract({distance = 1 })
        customizer = new ASTTransformationCustomizer(Contract)
        final expression = new AstBuilder().buildFromCode(CompilePhase.CONVERSION) {->
            distance = 1
        }.expression[0]
        customizer.annotationParameters = [value: expression]
        configuration.addCompilationCustomizers(customizer)
        def shell = new GroovyShell(configuration)
        def result = shell.evaluate("""
            class MyClass {
                int distance
                MyClass() {}
            }
            new MyClass()
        """)
        assert result.distance == 1
    }

    void testLocalTransformationWithClassAnnotationParameter() {
        // add @ConditionalInterrupt(value={ true }, thrown=SecurityException)
        customizer = new ASTTransformationCustomizer(ConditionalInterrupt)
        final expression = new AstBuilder().buildFromCode(CompilePhase.CONVERSION) {->
            true
        }.expression[0]
        customizer.annotationParameters = [value:expression, thrown: SecurityException]
        configuration.addCompilationCustomizers(customizer)
        def shell = new GroovyShell(configuration)
        shouldFail(SecurityException) {
            shell.evaluate("""
                class MyClass {
                    void doIt() { }
                }
                new MyClass().doIt()
            """)
        }
    }


Le 09/06/2011 13:54, Cédric Champeau a écrit :
Hi,

In Groovy 1.8 we released the ASTTransformationCustomizer which allows applying AST transformations transparently to scripts. For example, we can do this :

customizer = new ASTTransformationCustomizer(Log)
configuration.addCompilationCustomizers(customizer)
shell = new GroovyShell(configuration)
result = shell.evaluate("""
            class MyClass {}
            new MyClass()
""")
assert result.log.class == Logger


However, there's a limitation because we cannot pass annotation parameters to the customizer. I didn't manage to get it for 1.8.0, but I'd like to add it to 1.8.1. It's quite simple to do for regular types, for example I can already do this (locally, not supported on trunk) :

customizer = new ASTTransformationCustomizer(Log)
customizer.annotationParameters = [value: 'logger']
configuration.addCompilationCustomizers(customizer)
shell = new GroovyShell(configuration)
result = shell.evaluate("""
            class MyClass {}
            new MyClass()
""")
assert result.logger.class == Logger


But I'm still failing with closure annotations. For example, imagine the following annotation :

@Around(pattern='find.*', before={ code before }, after = { code after })
class MyClass {
   def findByName() { ... }
}

If you want to add this annotation transparently through the customizer, we have a problem because if we write this :

customizer.annotationParameters = [ pattern: 'find.*', before: { code before }, after: {code after } ]

Then the closures are compiled, and we do not have access to the closure class node at runtime. I need to create, in this case, a ClosureExpression from the existing closure instance, in order to be able to add a member to the AnnotationNode which is generated by the customizer. As far as I can see, this is not possible but there are several workarounds :

1. do not support closure annotations values through the customizer
2. write the closure content inside a string, which will be converted to ClosureExpression by the customizer
3. make the user write an ASTNode (ClosureExpression) instead of the closure (through ASTBuilder or directly I suppose)

I'm not a big fan or solution 1. because I don't like partial support. I don't like 2. either because this is an ugly solution. I can imagine solution 3. as the best solution : if we detect the annotation parameter provided to the customizer is an instance of a Closure, then we can throw an error message specifying that (s)he should replace it with a ClosureExpression.

Unless you have a magical solution to use the closure directly :)

Cédric


Reply | Threaded
Open this post in threaded view
|

Re: ASTTransformationCustomizer improvement

aalmiray
In reply to this post by melix
One minor suggestion: support a Map constructor for the customizer

|customizer = new ASTTransformationCustomizer(Log, annotationParameters: [value: 'logger'])
configuration.addCompilationCustomizers(customizer)
shell = new GroovyShell(configuration)
result = shell.evaluate("""
             class MyClass {}
             new MyClass()
""")
assert result.logger.class == Logger|

or even better

|customizer = new ASTTransformationCustomizer(Log, value: 'logger')|

The first case requires a constructor of the form

    ASTTransformationCustomizer(Class, Map)

while the latter requires

    ASTTransformationCustomizer(Map, Class)

Cheers,
Andres
Reply | Threaded
Open this post in threaded view
|

Re: ASTTransformationCustomizer improvement

melix
Le 10/06/2011 14:21, aalmiray a écrit :
> or even better
>
> |customizer = new ASTTransformationCustomizer(Log, value: 'logger')|
>
> while the latter requires
>
>      ASTTransformationCustomizer(Map, Class)
Thanks Andres. I am curious : there must be a special handling there,
because I would have expected the signature to be
ASTTransformationCustomizer(Class,Map)...


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: ASTTransformationCustomizer improvement

Guillaume Laforge-4
Hi Cédric,

When you have a mix of named and non-named arguments, the call will always be for a signature with a map as first argument that will contain all the named-arguments, and all the other non-named arguments will follow, in the order they've been used in the call.

Guillaume

On Mon, Jun 13, 2011 at 22:00, Cédric Champeau <[hidden email]> wrote:
Le 10/06/2011 14:21, aalmiray a écrit :
or even better

|customizer = new ASTTransformationCustomizer(Log, value: 'logger')|

while the latter requires

    ASTTransformationCustomizer(Map, Class)
Thanks Andres. I am curious : there must be a special handling there, because I would have expected the signature to be ASTTransformationCustomizer(Class,Map)...



---------------------------------------------------------------------
To unsubscribe from this list, please visit:

  http://xircles.codehaus.org/manage_email





--
Guillaume Laforge
Groovy Project Manager
Head of Groovy Development at SpringSource
http://www.springsource.com/g2one
Reply | Threaded
Open this post in threaded view
|

Re: ASTTransformationCustomizer improvement

John Hurst-2
See for example


I found this a little confusing until I read up on it.

John Hurst
Wellington, New Zealand

On Tue, Jun 14, 2011 at 6:09 PM, Guillaume Laforge <[hidden email]> wrote:
Hi Cédric,

When you have a mix of named and non-named arguments, the call will always be for a signature with a map as first argument that will contain all the named-arguments, and all the other non-named arguments will follow, in the order they've been used in the call.

Guillaume


On Mon, Jun 13, 2011 at 22:00, Cédric Champeau <[hidden email]> wrote:
Le 10/06/2011 14:21, aalmiray a écrit :
or even better

|customizer = new ASTTransformationCustomizer(Log, value: 'logger')|

while the latter requires

    ASTTransformationCustomizer(Map, Class)
Thanks Andres. I am curious : there must be a special handling there, because I would have expected the signature to be ASTTransformationCustomizer(Class,Map)...



---------------------------------------------------------------------
To unsubscribe from this list, please visit:

  http://xircles.codehaus.org/manage_email





--
Guillaume Laforge
Groovy Project Manager
Head of Groovy Development at SpringSource
http://www.springsource.com/g2one



--
Life is interfering with my game
Reply | Threaded
Open this post in threaded view
|

Re: ASTTransformationCustomizer improvement

melix
In reply to this post by Guillaume Laforge-4
One little thing I missed. Always surprising :)

Le 14/06/2011 08:09, Guillaume Laforge a écrit :
Hi Cédric,

When you have a mix of named and non-named arguments, the call will always be for a signature with a map as first argument that will contain all the named-arguments, and all the other non-named arguments will follow, in the order they've been used in the call.

Guillaume

On Mon, Jun 13, 2011 at 22:00, Cédric Champeau <[hidden email]> wrote:
Le 10/06/2011 14:21, aalmiray a écrit :
or even better

|customizer = new ASTTransformationCustomizer(Log, value: 'logger')|

while the latter requires

    ASTTransformationCustomizer(Map, Class)
Thanks Andres. I am curious : there must be a special handling there, because I would have expected the signature to be ASTTransformationCustomizer(Class,Map)...