[scala] field annotations, getters/setters and BeanProperty

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[scala] field annotations, getters/setters and BeanProperty

Lukas Rytz
Hi all,

wrapping up the discussions and tickets about placement of field annotations. There seems
to be consensus that by default, field annotations should not be copied to any synthetic
methods (getter / setter, beanGetter / -Setter). This would already solve some of the problems
and allow people to do the correct thing manually.

Then there were some proposals to make the placement of annotations configurable:

 a) using nested annotations, i.e.
   @Getter(new Id()) val x

 b) by having a list of defaults (e.g. properties files) specifying
    annotation X goes to field
    annotation Y goes to getter

 c) by specifying the target of annotations on package / class level, e.g.
    @SetterAnnotation(classOf[X]) package object foo

    package foo
    class C { @X var f = 0 }

    => annotation X goes to the setter for every field in any class inside foo


a) needs to be done at every definition.
b) needs to be done once.
c) needs to be done once per project.


The corresponding tickets are:
 - http://lampsvn.epfl.ch/trac/scala/ticket/1846 (we need a way to control placement of annotations)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2245 (proposes to use sensible defaults)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2252 (annotations should not be copied to any synthetic method by default)


Let me know if I missed or misunderstood something.

For now, I think solution c) looks the most promising. Additionally, we could use the information given
by the annotations java.lang.annotation.Target.

Cheers: Lukas
Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Josh Suereth
I'd personally prefer (a), especially if I could event have it look like @Getter(@Id), or even @Getter { val a = @Id }.  I think this gives me the most flexibility and bang-for-my buck.

C would also work out for most of my projects.

Is there anyway we could use (a) + (c) in tandem?  I.e. (c) sets the default location for an annotation, but I can override with annotations from (a)?


- Josh

On Fri, Aug 14, 2009 at 8:15 AM, Lukas Rytz <[hidden email]> wrote:
Hi all,

wrapping up the discussions and tickets about placement of field annotations. There seems
to be consensus that by default, field annotations should not be copied to any synthetic
methods (getter / setter, beanGetter / -Setter). This would already solve some of the problems
and allow people to do the correct thing manually.

Then there were some proposals to make the placement of annotations configurable:

 a) using nested annotations, i.e.
   @Getter(new Id()) val x

 b) by having a list of defaults (e.g. properties files) specifying
    annotation X goes to field
    annotation Y goes to getter

 c) by specifying the target of annotations on package / class level, e.g.
    @SetterAnnotation(classOf[X]) package object foo

    package foo
    class C { @X var f = 0 }

    => annotation X goes to the setter for every field in any class inside foo


a) needs to be done at every definition.
b) needs to be done once.
c) needs to be done once per project.


The corresponding tickets are:
 - http://lampsvn.epfl.ch/trac/scala/ticket/1846 (we need a way to control placement of annotations)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2245 (proposes to use sensible defaults)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2252 (annotations should not be copied to any synthetic method by default)


Let me know if I missed or misunderstood something.

For now, I think solution c) looks the most promising. Additionally, we could use the information given
by the annotations java.lang.annotation.Target.

Cheers: Lukas

Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Kevin Wright-4
Can we use generics/manifests to simplify solution c by losing the classOf[] notation?
Combined this with allowing overrides all the way down to member level and I think we have a winner...



    @SetterAnnotation[X] package object foo

    package foo
    class C { @X var f = 0 }

    class D {
        @GetterAnnotation[X]
        @X var g = 0
    }


Out of curiosity, does anyone know if it's supported to have a package object for _root_ ?  That would be an obvious choice to put some defaults.



On Fri, Aug 14, 2009 at 1:21 PM, Josh Suereth <[hidden email]> wrote:
I'd personally prefer (a), especially if I could event have it look like @Getter(@Id), or even @Getter { val a = @Id }.  I think this gives me the most flexibility and bang-for-my buck.

C would also work out for most of my projects.

Is there anyway we could use (a) + (c) in tandem?  I.e. (c) sets the default location for an annotation, but I can override with annotations from (a)?


- Josh


On Fri, Aug 14, 2009 at 8:15 AM, Lukas Rytz <[hidden email]> wrote:
Hi all,

wrapping up the discussions and tickets about placement of field annotations. There seems
to be consensus that by default, field annotations should not be copied to any synthetic
methods (getter / setter, beanGetter / -Setter). This would already solve some of the problems
and allow people to do the correct thing manually.

Then there were some proposals to make the placement of annotations configurable:

 a) using nested annotations, i.e.
   @Getter(new Id()) val x

 b) by having a list of defaults (e.g. properties files) specifying
    annotation X goes to field
    annotation Y goes to getter

 c) by specifying the target of annotations on package / class level, e.g.
    @SetterAnnotation(classOf[X]) package object foo

    package foo
    class C { @X var f = 0 }

    => annotation X goes to the setter for every field in any class inside foo


a) needs to be done at every definition.
b) needs to be done once.
c) needs to be done once per project.


The corresponding tickets are:
 - http://lampsvn.epfl.ch/trac/scala/ticket/1846 (we need a way to control placement of annotations)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2245 (proposes to use sensible defaults)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2252 (annotations should not be copied to any synthetic method by default)


Let me know if I missed or misunderstood something.

For now, I think solution c) looks the most promising. Additionally, we could use the information given
by the annotations java.lang.annotation.Target.

Cheers: Lukas


Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Lukas Rytz
On Fri, Aug 14, 2009 at 14:30, Kevin Wright <[hidden email]> wrote:
Can we use generics/manifests to simplify solution c by losing the classOf[] notation?

sure, my code was just to show the idea, no syntax proposal
 


There seems
to be consensus that by default, field annotations should not be copied to any synthetic
methods (getter / setter, beanGetter / -Setter).
 
somebody just mentioned (http://lampsvn.epfl.ch/trac/scala/ticket/2252#comment:3) that annotations
should maybe still be copied to the scala getter/setter by default. I disagree, what do others think?
Note that we will most likely chose the same rules for non-classfile (scala-only) annotations.

Lukas

Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Alex Cruise-2
In reply to this post by Lukas Rytz
Lukas Rytz wrote:

> wrapping up the discussions and tickets about placement of field
> annotations. There seems
> to be consensus that by default, field annotations should *not* be
> copied to any synthetic
> methods (getter / setter, beanGetter / -Setter). This would already
> solve some of the problems
> and allow people to do the correct thing manually.
>
> Then there were some proposals to make the placement of annotations
> configurable:
>
>  a) using nested annotations, i.e.
>    @Getter(new Id()) val x
Can we aim this stone at another bird and optionally tie together Scala
setters and JavaBeans setters?

http://lampsvn.epfl.ch/trac/scala/ticket/2215 is kind of annoying
because it makes you manually chain setters/getters between Scala/Bean
methods; I don't know if it should be the default but there should be a
way to indicate that the generated setter must call the written one.  
Maybe {@Getter | @Setter}(beanStyle={CHAIN, NONE, INDEPENDENT}), where
CHAIN makes the generated method call the overridden one if any, NONE
does not generate a bean-style getter, and INDEPENDENT generates one
that does not call the overridden setter.

Relatedly, it would be good to embrace-and-extend Josh's work on
http://github.com/jsuereth/private-setter-scalac-plugin/tree/master , e.g.

    @Setter(access=PROTECTED, accessScope="com.example.pkg" /* package
literals? */)

Finally (for now :), we should talk about the possibility of combining
getter and setter annotations into a single @Property annotation, e.g.:

    @Property(beanStyle, setter=@Setter(access=PRIVATE))

And if we *really* play our cards right, and decide to care, we could
work out the semantics of integrating this with .NET-style properties.
>  c) by specifying the target of annotations on package / class level,
> e.g.
>     @SetterAnnotation(classOf[X]) package object foo
>
>     package foo
>     class C { @X var f = 0 }
>
>     => annotation X goes to the setter for every field in any class
> inside foo
I think this makes sense mainly for relatively granular, cohesive
packages, otherwise the "suppress inherited annotations" annotations
could get quite populous. :)

Thanks!

-0xe1a
Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Lukas Rytz
In reply to this post by Lukas Rytz
[forwarding to list]

On Aug 14, 2009, at 16:04, Lukas Rytz wrote:

somebody just mentioned (http://lampsvn.epfl.ch/trac/scala/ticket/2252#comment:3) that annotations
should maybe still be copied to the scala getter/setter by default. I disagree, what do others think?
Note that we will most likely chose the same rules for non-classfile (scala-only) annotations.

I disagree as well, having actually tried using Scala and some frameworks that use annotations for dependency injection:

Some explicitly check that an annotated method starts with "set" and otherwise throw an error. I think Hibernate was in this category.

JBoss Seam doesn't check that setters start with "set", but when it wants to compute the name of a component, it just chops off the first 3 characters of the method name. I actually got Seam bijection to work by hacking it, but it wasn't pretty at all. Trying to compensate for having annotations appear on both the field and the (multiple) getters and setters is not particularly easy.

Let's also not forget that having this class of annotations ("inject a value here") on both a setter and a getter is just not sensible, no programmer would place them there, so why should Scala do it? Compatibility with *existing* frameworks is the issue.

I would be very happy if the bug just got fixed so that the annotations will only appear on the fields.


Of course, the "luxury" version of the fix is to check whether the annotation is even allowed for fields or methods...

--
Bart Schuller


Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Lukas Rytz
In reply to this post by Josh Suereth
Another proposal came up today during our group meeting. You can annotate your annotation type
with the places where the annotation should go:

   class C {
     @BeanProperty @(Id @BeanGettter) val x = 1
   }

This has the advantage that we don't need to nest annotations. Then, for DRY, you can define type
aliases for your annotated annotation types:

   object ScalaJPA {
     type Id = javax.persistence.Id @BeanGetter
     // many more
   }

Which gives you the desired result (just import the alias instead of the underlying annotation):

   import ScalaJPA.Id
   class C {
     @BeanProperty @Id val x = 1
   }


The BIG advantage here is that we need much (much) less compiler magic; type aliases are
expanded in the compiler anyway, so no lookup is needed when deciding where to copy the
annotations.

The alias definitions can be written once and then be shared.

Everybody happy with that?

Lukas



On Fri, Aug 14, 2009 at 14:21, Josh Suereth <[hidden email]> wrote:
I'd personally prefer (a), especially if I could event have it look like @Getter(@Id), or even @Getter { val a = @Id }.  I think this gives me the most flexibility and bang-for-my buck.

C would also work out for most of my projects.

Is there anyway we could use (a) + (c) in tandem?  I.e. (c) sets the default location for an annotation, but I can override with annotations from (a)?


- Josh


On Fri, Aug 14, 2009 at 8:15 AM, Lukas Rytz <[hidden email]> wrote:
Hi all,

wrapping up the discussions and tickets about placement of field annotations. There seems
to be consensus that by default, field annotations should not be copied to any synthetic
methods (getter / setter, beanGetter / -Setter). This would already solve some of the problems
and allow people to do the correct thing manually.

Then there were some proposals to make the placement of annotations configurable:

 a) using nested annotations, i.e.
   @Getter(new Id()) val x

 b) by having a list of defaults (e.g. properties files) specifying
    annotation X goes to field
    annotation Y goes to getter

 c) by specifying the target of annotations on package / class level, e.g.
    @SetterAnnotation(classOf[X]) package object foo

    package foo
    class C { @X var f = 0 }

    => annotation X goes to the setter for every field in any class inside foo


a) needs to be done at every definition.
b) needs to be done once.
c) needs to be done once per project.


The corresponding tickets are:
 - http://lampsvn.epfl.ch/trac/scala/ticket/1846 (we need a way to control placement of annotations)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2245 (proposes to use sensible defaults)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2252 (annotations should not be copied to any synthetic method by default)


Let me know if I missed or misunderstood something.

For now, I think solution c) looks the most promising. Additionally, we could use the information given
by the annotations java.lang.annotation.Target.

Cheers: Lukas


Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Florian Hars-3
In reply to this post by Lukas Rytz
Lukas Rytz schrieb:
> Some explicitly check that an annotated method starts with "set" and
> otherwise throw an error. I think Hibernate was in this category.

OpenJPA on the other hand will notice that you have both fields and
methods with persistence annotations, complain that this is undefined
by the JPA spec and refuse to do anything with it.

- Florian
Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Tim Pigden
In reply to this post by Lukas Rytz
Just another example of what doesn't work - the hibernate reference implementation of jsr 303 (currently hibernate-validator 4.0.0Beta2) and the JSR specifically only wants the annotations on fields or properties defined as getX or isX. (It's not final, just the final draft but I don't think current scalac behaviour is a reasonable cause for late stage objections)
Tim
Reply | Threaded
Open this post in threaded view
|

Re: [scala] field annotations, getters/setters and BeanProperty

Lukas Rytz
In reply to this post by Lukas Rytz
The proposal below has been implemented in trunk.

The meta-annotations in package 'scala.annotation.target' are
used to control to which of the accessors the annotations on
a field are copied. There are @field, @getter, @setter, @beanGetter
and @beanSetter.

By default, field annotations are only added to the actual field, but
not to any of the accessors. By annotating the annotation type with
one or several of the meta-annotations this behavior can be changed.

In the following example, the annotation @Id will be added only to the
bean getter 'getX()'

  import javax.persistence.Id
  class A {
    @(Id @beanGetter) @BeanProperty val x = 0
  }

The syntax can be improved using a type alias:

  object ScalaJPA {
    type Id = javax.persistence.Id @beanGetter
    // more aliases for that project
  }
  import ScalaJPA.Id
    class A {
    @Id @BeanProperty val x = 0
  }


I'm happy to get feedback! Let me know your thoughts on the design and if the
implementation works as expected (nightly build from September 30 or later, [1]).

Cheers: Lukas


[1] http://www.scala-lang.org/archives/downloads/distrib/files/nightly/distributions/scala-2.8.0.r18830-b20090930020951.tgz



On Tue, Aug 18, 2009 at 17:28, Lukas Rytz <[hidden email]> wrote:
Another proposal came up today during our group meeting. You can annotate your annotation type
with the places where the annotation should go:

   class C {
     @BeanProperty @(Id @BeanGettter) val x = 1
   }

This has the advantage that we don't need to nest annotations. Then, for DRY, you can define type
aliases for your annotated annotation types:

   object ScalaJPA {
     type Id = javax.persistence.Id @BeanGetter
     // many more
   }

Which gives you the desired result (just import the alias instead of the underlying annotation):

   import ScalaJPA.Id
   class C {
     @BeanProperty @Id val x = 1
   }


The BIG advantage here is that we need much (much) less compiler magic; type aliases are
expanded in the compiler anyway, so no lookup is needed when deciding where to copy the
annotations.

The alias definitions can be written once and then be shared.

Everybody happy with that?

Lukas



On Fri, Aug 14, 2009 at 14:21, Josh Suereth <[hidden email]> wrote:
I'd personally prefer (a), especially if I could event have it look like @Getter(@Id), or even @Getter { val a = @Id }.  I think this gives me the most flexibility and bang-for-my buck.

C would also work out for most of my projects.

Is there anyway we could use (a) + (c) in tandem?  I.e. (c) sets the default location for an annotation, but I can override with annotations from (a)?


- Josh


On Fri, Aug 14, 2009 at 8:15 AM, Lukas Rytz <[hidden email]> wrote:
Hi all,

wrapping up the discussions and tickets about placement of field annotations. There seems
to be consensus that by default, field annotations should not be copied to any synthetic
methods (getter / setter, beanGetter / -Setter). This would already solve some of the problems
and allow people to do the correct thing manually.

Then there were some proposals to make the placement of annotations configurable:

 a) using nested annotations, i.e.
   @Getter(new Id()) val x

 b) by having a list of defaults (e.g. properties files) specifying
    annotation X goes to field
    annotation Y goes to getter

 c) by specifying the target of annotations on package / class level, e.g.
    @SetterAnnotation(classOf[X]) package object foo

    package foo
    class C { @X var f = 0 }

    => annotation X goes to the setter for every field in any class inside foo


a) needs to be done at every definition.
b) needs to be done once.
c) needs to be done once per project.


The corresponding tickets are:
 - http://lampsvn.epfl.ch/trac/scala/ticket/1846 (we need a way to control placement of annotations)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2245 (proposes to use sensible defaults)
 - http://lampsvn.epfl.ch/trac/scala/ticket/2252 (annotations should not be copied to any synthetic method by default)


Let me know if I missed or misunderstood something.

For now, I think solution c) looks the most promising. Additionally, we could use the information given
by the annotations java.lang.annotation.Target.

Cheers: Lukas