[jira] [Comment Edited] (GROOVY-10816) @NamedVariant does not work on methods on a trait or default methods on an interface

["Paul King (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GROOVY-10816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17758548#comment-17758548 ] 

Paul King edited comment on GROOVY-10816 at 8/24/23 12:16 PM:
--------------------------------------------------------------

In terms of documentation, this falls under the general warning about mixing traits with AST transforms:

[https://groovy-lang.org/objectorientation.html#_compatibility_with_ast_transformations]

Background: There are multiple "intuitively obvious" behaviors you might expect from that code. One is that @NamedVariant is run on the trait, then the additional methods added would be weaved into classes implementing the trait. Another is that the annotation itself should be woven into the class implementing the trait and then run on that class. There is no correct answer as to which is correct. Depending on what the AST transform does, it makes sense for one or the other, or even further alternative semantics.

To cut a long story short, the transform is not being run on the interface generated for the trait, so no map variant will be woven into your class.
It is in fact being run on the trait helper class and there is a secondary interaction there to do with how default methods are handled.

To understand what is going on, you need to understand how Groovy implements default parameters. Basically, this:
{code:java}
def foo(String username = "sdf", String email = "ghj") {}
{code}
Is converted into this:
{code:java}
def foo(String username, String email) {}
def foo(String username) { foo(username, 'ghj') }
def foo() { foo('sdf') }
{code}
When this expansion is done, annotations are copied across all methods. This is harmless when on a class because we have already identified AST transforms prior to the expansion. When in a trait, the weaving is done prior to AST transforms being collected, so it will try to create 3 named variants Map methods for you, hence the duplicate error.

Further background: Before going into some workarounds, we have looked at ways you might give the AST processing some better information about what to do, e.g. you could annotate your transform with a marker annotation to direct a particular kind of expansion behavior. But we haven't been satisfied with a complete solution at this point in time. So, long story short, that code may work unchanged in a future Groovy version if we can get a smart enough solution.

So, to work around the issue, you can give up on default parameters, e.g. do the expansion yourself:
{code:java}
trait Foo {
  @NamedVariant
  def foo(String username, String email) { println "$username|$email" }
  def foo(String username) { foo(username, 'ghj') }
  def foo() { foo('sdf') }
}
{code}
But then to get around the fact that the map variant isn't in your interface, you have to do a manual hack and call the trait helper method manually:
{code}
class Bar implements Foo {
  def foo(LinkedHashMap m) { Foo$Trait$Helper.foo([$self:this, username:m.username ?: 'sdf', email:m.email ?: 'ghj']) }
}
{code}
You should note that the named variant is actually of the trait helper method which has an extra '$self' parameter, so there is a bit of added noise that you shouldn't normally see.

To be honest, with the extra work, you might as well just do the "named variant" version manually:
{code}
trait Foo {
  def foo(String username = 'sdf', String email = 'ghj') { println "$username|$email" }
  def foo(LinkedHashMap m) { foo(m.username ?: 'sdf', m.email ?: 'ghj') }
}
{code}



was (Author: paulk):
In terms of documentation, this falls under the general warning about mixing traits with AST transforms:

[https://groovy-lang.org/objectorientation.html#_compatibility_with_ast_transformations]

Background: There are multiple "intuitively obvious" behaviors you might expect from that code. One is that @NamedVariant is run on the trait, then the additional methods added would be weaved into classes implementing the trait. Another is that the annotation itself should be woven into the class implementing the trait and then run on that class. There is no correct answer as to which is correct. Depending on what the AST transform does, it makes sense for one or the other, or even further alternative semantics.

To cut a long story short, the transform is not being run on the interface generated for the trait, so no map variant will be woven into your class.
It is in fact being run on the trait helper class and there is a secondary interaction there to do with how default methods are handled.

To understand what is going on, you need to understand how Groovy implements default parameters. Basically, this:
{code:java}
def foo(String username = "sdf", String email = "ghj") {}
{code}
Is converted into this:
{code:java}
def foo(String username, String email) {}
def foo(String username) { foo(username, 'ghj') }
def foo() { foo('sdf') }
{code}
When this expansion is done, annotations are copied across all methods. This is harmless when on a class because we have already identified AST transforms prior to the expansion. When in a trait, the weaving is done prior to AST transforms being collected, so it will try to create 3 named variants Map methods for you, hence the duplicate error.

Further background: Before going into some workarounds, we have looked at ways you might give the AST processing some better information about what to do, e.g. you could annotate your transform with a marker annotation to direct a particular kind of expansion behavior. But we haven't been satisfied with a complete solution at this point in time. So, long story short, that code may work unchanged in a future Groovy version if we can get a smart enough solution.

So, to work around the issue, you can give up on default parameters, e.g. do the expansion yourself:
{code:java}
trait Foo {
  @NamedVariant
  def foo(String username, String email) { println "$username|$email" }
  def foo(String username) { foo(username, 'ghj') }
  def foo() { foo('sdf') }
}
{code}
But then to get around the fact that the map variant isn't in your interface, you have to do a manual hack and call the trait helper method manually:
{code}
class Bar implements Foo {
  def foo(LinkedHashMap m) { Foo$Trait$Helper.foo([$self:this, username:m.username ?: 'sdf', email:m.email ?: 'ghj']) }
}
{code}
To be honest, with the extra work, you might as well just do the "named variant" version manually:
{code}
trait Foo {
  def foo(String username = 'sdf', String email = 'ghj') { println "$username|$email" }
  def foo(LinkedHashMap m) { foo(m.username ?: 'sdf', m.email ?: 'ghj') }
}
{code}


> @NamedVariant does not work on methods on a trait or default methods on an interface
> ------------------------------------------------------------------------------------
>
>                 Key: GROOVY-10816
>                 URL: https://issues.apache.org/jira/browse/GROOVY-10816
>             Project: Groovy
>          Issue Type: Bug
>    Affects Versions: 4.0.6
>            Reporter: Robert Elliot
>            Priority: Minor
>              Labels: default-methods, named-parameters
>
> The following code fails to compile:
> {code:groovy}
> import groovy.transform.NamedVariant
> trait Foo {
>   @NamedVariant
>   def foo(String username = "sdf", String email = "ghj") {}
> }
> {code}
> with this error:
> {{Groovyc: Error during @NamedVariant processing. Class Foo$Trait$Helper already has a named-arg method of type [org.codehaus.groovy.ast.Parameter@15247533[name: namedArgs, type: java.util.Map, hasDefaultValue: false]]}}
> It works fine as a class.
> It fails as an interface with a default method.
> Perhaps this is intended, but if so I failed to find any documentation specifying that {{@NamedVariant}} only works with classes.



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