groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From MG <mg...@arscreat.com>
Subject Re: Making @Immutable a meta-annotation
Date Thu, 25 Jan 2018 22:41:01 GMT
m:-)g

On 24.01.2018 04:39, Paul King wrote:
> Okay, I made those changes. There is now a makeImmutable annotation 
> attribute. Still a bunch of tidying up work to do including 
> documentation refinements but any feedback welcome.
>
> cheers, Paul.
>
> On Sat, Jan 20, 2018 at 10:03 AM, MG <mgbiz@arscreat.com 
> <mailto:mgbiz@arscreat.com>> wrote:
>
>     @MapConstructor(makeImmutable=true)  (or maybe:
>     @Constructor(defensiveCopying=true, cloneNonImmutableObjects =
>     false,  /*etc*/) ?) for me would be the way to go.
>
>     (No implementation detail smell imho, just fine granularity for
>     developers*, and not reusing the same annotation for (slightly)
>     different things.)
>
>     Cheers,
>     mg
>
>     *Which is always good in my book, plus most people will use
>     @Immutable meta-annotation anyway, plus everyone can create their
>     own meta-annotations from these fine granular annotations (to
>     avoid code clutter)
>
>
>     On 17.01.2018 01:48, Paul King wrote:
>>
>>     Response inline.
>>
>>     On Wed, Jan 17, 2018 at 9:54 AM, MG <mgbiz@arscreat.com
>>     <mailto:mgbiz@arscreat.com>> wrote:
>>
>>         Hmmm.... If the argument for naming the marker annotation
>>         @KnownImmutable was that the existing parameters have similar
>>         names (and cannot be changed) then it seems to me the
>>         "KnownImmutable" name choice was pretty immutable to begin
>>         with...
>>
>>         Apart from that, there is still the inconsistency what
>>         @KnownImmutable  really expresses:
>>
>>           * Class that carries @KnownImmutable is "fully immutable":
>>             When a developer puts the annotation on a class
>>           * Class that carries @KnownImmutable is "base immutable"
>>             (i.e. no defense-copying ctors etc): When being put by
>>             the Groovy compiler on a class after having applied
>>             @ImmutableBase transformations to it.
>>
>>     This second bullet is the wrong way to look at what is going on.
>>     Just using @ImmutableBase (or whatever name) on a class doesn't
>>     add the @KnownImmutable annotation. The @Immutable annotation
>>     collector adds @KnownImmutable knowing that @ImmutableBase and
>>     the various constructor annotations are going to be processed. So
>>     in fact it's just the first case but with the compiler indicating
>>     that it is "fully immutable". So I don't see a conflict as far as
>>     the @Immutable annotation goes. What you could argue is that
>>     users of the constructor transformations might want the defensive
>>     copying etc. but might not want to make the class fully immutable
>>     in which case having an annotation attribute like
>>     @MapConstructor(makeImmutable=true) would make more sense. This
>>     would provide maximum flexibility and remove the slight dual
>>     usage of the annotation. But the other way to look at this is
>>     that having a "makeImmutable" annotation attribute smells of
>>     implementation detail and just using @KnownImmutable is the more
>>     declarative way to express what we want to achieve with less noise.
>>
>>         The way it looks to me you are trying to express two
>>         different things through the same annotation - but to have a
>>         clean design you would need two seperate annotations. Maybe
>>         that is also why you do not like any of my alternatives,
>>         because you are looking for one name that expresses both use
>>         cases - and that does not exist, because the use cases differ (?)
>>
>>         I am still convinced that while knownUmmutable semi-works as
>>         a parameter name inside of @Immutable (I would have picked
>>         guaranteed here also), that does not mean it is a good choice
>>         for the annotation name. But as I said, if you are convinced
>>         that one requires the other, this discussion is mute anyway...
>>
>>
>>         On 16.01.2018 01:56, Paul King wrote:
>>>         Explanations below.
>>>
>>>         On Tue, Jan 16, 2018 at 12:56 AM, MG <mgbiz@arscreat.com
>>>         <mailto:mgbiz@arscreat.com>> wrote:
>>>
>>>             Hi Paul,
>>>
>>>             1) for me, if you have to explain a name better, then it
>>>             is already a bad name. Intuitively suggesting the
>>>             correct interpretation to another developer, without
>>>             requiring him to thoroughly read through the
>>>             documentation, is the art of picking good names (which
>>>             imho Groovy overall does a good job at).
>>>             With regards to @KnownImmutable, "someone (the compiler
>>>             or the developer) knows" is even more confusing. If it
>>>             is in fact irrelevant who knows it, why is there a
>>>             "Known" in the name in the first place ? And why is
>>>             therefore e.g. @IsImmutable not a better name (it could
>>>             also carry a parameter which can be true or false, with
>>>             false allowing a developer to express that a class is
>>>             definitely not immutable (even if it might look that way
>>>             on first glance; e.g. effectively blocking or issuing a
>>>             warning in certain parallel execution scenarios)).
>>>
>>>
>>>         We have since the introduction of @Immutable used the
>>>         knownImmutable and knownImmutableClasses annotation
>>>         attributes and people seem to grok what they mean. This is a
>>>         very similar use case. I think it would be hard to justify
>>>         renaming @KnownImmutable without renaming the annotation
>>>         attributes as well.
>>>
>>>             2) There seems to be a contradiction in your statements:
>>>             You say that "Once @ImmutableBase (or whatever name)
>>>             processing has finished its checks, it can then vouch
>>>             for the class and puts the marker interface
>>>             [@KnownImmutable] "rubber stamp" on it", but further
>>>             down you say that "These changes [that @ImmutableBase
>>>             applies] alone don't guarantee immutability.". Is it a
>>>             "known immutable" after @ImmutableBase has done its
>>>             thing, or not ?
>>>
>>>
>>>         Only after all transformations have completed it is
>>>         guaranteed (see below).
>>>
>>>             3) If I did not miss something the new @Immutable meta
>>>             annotation is made up of the following annotations:
>>>             @ImmutableBase
>>>             @KnownImmutable
>>>             @ToString
>>>             @EqualsAndHashCode
>>>             @MapConstructor
>>>             @TupleConstructor
>>>
>>>             How is any of the last four necessary for a class to be
>>>             immutable ? Immutability to me means, that the state of
>>>             the class cannot be changed after it has been created.
>>>             How are @ToString, @EqualsAndHashCode, @MapConstructor,
>>>             and @TupleConstructor helping with this ?
>>>             At least one ctor to initialize the class fields is
>>>             basically necessary to make this a practically usable
>>>             immutable class, yes, but @IsImmutable it must be after
>>>             @ImmutableBase does its job, or it will not be immutable
>>>             in the end. All the other annotations are just icing on
>>>             the cake (see "@Immutable should be named
>>>             @ImmutableCanonical").
>>>
>>>
>>>         @MapConstructor and @TupleConstructor do different things if
>>>         they find the @KnownImmutable marker interface on the class
>>>         they are processing (defensive copy in/clone/wrap etc.)
>>>         which is needed for immutable classes. We could have used an
>>>         additional annotation attribute (makeImmutable = true) or
>>>         something but the marker interface is useful in its own
>>>         right and it seems sensible to not duplicate the information
>>>         it conveys. Besides we'd have to choose a name for
>>>         "makeImmutable" and again since it's only part of the
>>>         immutable story good naming would be hard.
>>>
>>>             If you keep @ImmutableBase, at least consider replacing
>>>             @KnownImmutable with @GuaranteedImmutableTag or
>>>             @GuaranteedImmutableMarker ? The "Tag" or "Marker"
>>>             postfix at least expresses that this annotation just
>>>             tags the class as having certain properties, and that
>>>             this is a general fact, and not only known to developers
>>>             or compilers in the know...
>>>
>>>
>>>         Marker interfaces are commonplace within the Java world and
>>>         we don't name them as such. It's not CloneableTag or
>>>         SerializableMarker. I think adding such a suffix would be
>>>         confusing.
>>>
>>>             I hope I do not completely miss your point, but this is
>>>             how it looks to me from what I read :-),
>>>             Cheers,
>>>             mg
>>>
>>>
>>>
>>>             On 15.01.2018 14:08, Paul King wrote:
>>>>
>>>>             Response below.
>>>>
>>>>             On Sun, Jan 14, 2018 at 6:11 AM, MG <mgbiz@arscreat.com
>>>>             <mailto:mgbiz@arscreat.com>> wrote:
>>>>
>>>>                 Hi Paul,
>>>>
>>>>                 now I get where you are coming from with
>>>>                 @KnownImmutable. I agree with splitting the two
>>>>                 concepts: Flexible & elegant :-)
>>>>
>>>>                 Transferring the parameter name knownImmutables
>>>>                 (which exists inside the @Immutable context) to the
>>>>                 annotation name KnownImmutable (which has no such
>>>>                 context) still does not work for me, though.
>>>>                 In addition having @Immutable = @KnownImmutable +
>>>>                 @ImmutableBase violates the definition you give for
>>>>                 @KnownImmutable, because either the class is "known
>>>>                 to be immutable" = "immutable by implementation by
>>>>                 the developer", or it becomes immutable through
>>>>                 @ImmutableBase & Groovy...
>>>>
>>>>
>>>>             Well that is perhaps an indication that it needs to be
>>>>             explained better rather than necessarily a bad name.
>>>>             I'll try again. It just means that someone (the
>>>>             compiler or the developer) knows that it is immutable.
>>>>             If that marker interface is on the class there is no
>>>>             need to look further inside the class, you can assume
>>>>             it is vouched for as immutable. Once @ImmutableBase (or
>>>>             whatever name) processing has finished its checks, it
>>>>             can then vouch for the class and puts the marker
>>>>             interface "rubber stamp" on it.
>>>>
>>>>                 What do you think about:
>>>>                 @IsImmutable
>>>>                 @ImmutableContract
>>>>                 @GuaranteedImmutable
>>>>                 instead
>>>>                 ?
>>>>
>>>>                 Thinking about this some more, still don't like
>>>>                 @ImmutableBase. Sounds too much like a base class
>>>>                 to me - and what would be the "base" functionality
>>>>                 of being immutable ? Something either is immutable,
>>>>                 or not (@ImmutableCore also fails in this regard ;-) ).
>>>>                 So still would prefer @ImmutableOnly o.s. ..
>>>>
>>>>
>>>>             @ImmutableOnly indicates that it is somehow immutable
>>>>             at that point - it isn't really a finished immutable
>>>>             class until all the other related transforms have done
>>>>             their thing. Perhaps it is useful to reiterate what it
>>>>             does. It does a whole pile of validation (you can't
>>>>             have public fields, you can't have certain annotation
>>>>             attributes on some of the other annotations that
>>>>             wouldn't make sense for an immutable object, you can't
>>>>             have your own constructors, it can't be applied on
>>>>             interfaces, it checks spelling of property names
>>>>             referenced in annotation attributes) plus some
>>>>             preliminary changes (makes class final, ensures
>>>>             properties have a final private backing field and a
>>>>             getter but no setter, makes a copyWith constructor if
>>>>             needed). These changes alone don't guarantee
>>>>             immutability. Would you prefer @ImmutablePrelim?
>>>>
>>>>                 Cheers,
>>>>                 mg
>>>>
>>>>
>>>>                 -------- Ursprüngliche Nachricht --------
>>>>                 Von: Paul King <paulk@asert.com.au>
>>>>                 <mailto:paulk@asert.com.au>
>>>>                 Datum: 13.01.18 13:17 (GMT+01:00)
>>>>                 An: MG <mgbiz@arscreat.com>
>>>>                 <mailto:mgbiz@arscreat.com>
>>>>                 Betreff: Re: Making @Immutable a meta-annotation
>>>>
>>>>                 I should have explained the @KnownImmutable idea a
>>>>                 bit more. I guess I was thinking about several
>>>>                 possibilities for that in parallel. What I really
>>>>                 think is the way to go though is to split out the
>>>>                 two different aspects that I was trying to capture.
>>>>                 One is triggering the AST transformation, the other
>>>>                 is a runtime marker of immutability. With that in
>>>>                 mind I'd suggest the following:
>>>>
>>>>                 @KnownImmutable will be a marker interface and
>>>>                 nothing more. Any class having that annotation will
>>>>                 be deemed immutable.
>>>>                 E.g. if I write my own Address class and I know
>>>>                 it's immutable I can mark it as such:
>>>>
>>>>                 @KnownImmutable
>>>>                 class Address {
>>>>                 Address(String value) { this.value = value }
>>>>                   final String value
>>>>                 }
>>>>
>>>>                 Now if I have:
>>>>
>>>>                 @Immutable
>>>>                 class Person {
>>>>                   String name
>>>>                   Address address
>>>>                 }
>>>>
>>>>                 Then the processing associated with @Immutable
>>>>                 won't complain about a potentially mutable
>>>>                 "Address" field.
>>>>
>>>>                 Then we can just leave @ImmutableBase (or similar)
>>>>                 as the AST transform to kick off the initial
>>>>                 processing needed for immutable classes.
>>>>                 The @Immutable annotation collector would be
>>>>                 replaced by the constructor annotations, ToString,
>>>>                 EqualsAndHashcode and both ImmutableBase and
>>>>                 KnownImmutable.
>>>>                 The name KnownImmutable matches existing
>>>>                 functionality. Two alternatives to annotating
>>>>                 Address with KnownImmutable that already exist
>>>>                 would be using the following annotation attributes
>>>>                 on @Immutable:
>>>>                 @Immutable(knownImmutableClasses=[Address]) or
>>>>                 @Immutable(knownImmutables=[address]).
>>>>
>>>>                 Cheers, Paul.
>>>>
>>>>
>>>>
>>>>                 On Sat, Jan 13, 2018 at 1:43 PM, MG
>>>>                 <mgbiz@arscreat.com <mailto:mgbiz@arscreat.com>>
wrote:
>>>>
>>>>                     Hi Paul,
>>>>
>>>>                     I think the core of the problem is, that
>>>>                     @Immutable as a meta-annotation woud be better
>>>>                     off being called something along the line of
>>>>                     @ImmutableCanonical (see: If you do no need the
>>>>                     immutability, use @Canonical), since it does
>>>>                     not solely supply immutability support - then
>>>>                     it would be natural to call the actual core
>>>>                     immutability annotation just "Immutable".
>>>>
>>>>                     That is probably off the table, since it would
>>>>                     be a breaking change - so we are stuck with the
>>>>                     problem of naming the immutability annotation
>>>>                     part something else.
>>>>
>>>>                     @ImmutableClass would imply to me that the
>>>>                     "Class" part carries some meaning, which I feel
>>>>                     it does not, since
>>>>                     a) "Class" could be postfixed to any annotation
>>>>                     name that applies to classes
>>>>                     b) The meta-annotation should accordingly also
>>>>                     be called "ImmutableClass"
>>>>                     Because of that I find postfixing "Immutable"
>>>>                     with "Class" just confusing. It also is not
>>>>                     intuitive to me, which annotation does only
>>>>                     supply the core, and which supplies the
>>>>                     extended (canonical) functionality...
>>>>
>>>>                     I do not understand where you are going with
>>>>                     @KnownImmutable (known to whom ?-) To me this
>>>>                     seems less intuitive/more confusing than
>>>>                     @ImmutableClass...).
>>>>
>>>>                     @ImmutableCore is similar to @ImmutableBase
>>>>                     (because I intentionally based it on it :-) ),
>>>>                     but different in the sense that it imho
>>>>                     expresses the semantics of the annotation:
>>>>                     Making the object purely immutable-only,
>>>>                     without any constructors, toString
>>>>                     functionality, etc.
>>>>
>>>>                     How about:
>>>>                     @ImmutableOnly
>>>>                     @PureImmutable
>>>>                     @ModificationProtected
>>>>
>>>>                     @Locked
>>>>                     @Frozen
>>>>
>>>>                     @Unchangeable
>>>>                     @Changeless
>>>>
>>>>                     @InitOnly
>>>>                     @InitializeOnly
>>>>
>>>>                     @Constant
>>>>                     @Const
>>>>
>>>>                     @NonModifieable
>>>>                     @NonChangeable
>>>>
>>>>                     ?
>>>>                     mg
>>>>
>>>>
>>>>
>>>>                     On 12.01.2018 08:01, Paul King wrote:
>>>>>                     @ImmutableCore is similar to @ImmutableBase -
>>>>>                     probably okay but I don't think ideal. Another
>>>>>                     alternative would be @ImmutableInfo or have an
>>>>>                     explicit marker interface with a different
>>>>>                     package, e.g.
>>>>>                     groovy.transform.marker.Immutable but that
>>>>>                     might cause IDE completion headaches. Perhaps
>>>>>                     @KnownImmutable as a straight marker interface
>>>>>                     might be the way to go - then it could be used
>>>>>                     explicitly on manually created immutable
>>>>>                     classes and avoid the need to use the
>>>>>                     knownImmutableClasses/knownImmutables
>>>>>                     annotation attributes for that case.
>>>>>
>>>>>                     Cheers, Paul.
>>>>>
>>>>>                     On Thu, Jan 11, 2018 at 9:34 PM, mg
>>>>>                     <mgbiz@arscreat.com
>>>>>                     <mailto:mgbiz@arscreat.com>> wrote:
>>>>>
>>>>>                         Hi Paul,
>>>>>
>>>>>                         great to make @Immutable more fine
>>>>>                         granular / flexible :-)
>>>>>
>>>>>                         what about
>>>>>                         @ImmutabilityChecked
>>>>>                         or
>>>>>                         @ImmutableCore
>>>>>                         instead of @ImmutableClass ?
>>>>>
>>>>>                         Cheers
>>>>>                         mg
>>>>>
>>>>>                         -------- Ursprüngliche Nachricht --------
>>>>>                         Von: Paul King <paulk@asert.com.au
>>>>>                         <mailto:paulk@asert.com.au>>
>>>>>                         Datum: 11.01.18 08:07 (GMT+01:00)
>>>>>                         An: dev@groovy.apache.org
>>>>>                         <mailto:dev@groovy.apache.org>
>>>>>                         Betreff: Making @Immutable a meta-annotation
>>>>>
>>>>>
>>>>>                         There has been discussion on and off about
>>>>>                         making @Immutable a meta-annotation
>>>>>                         (annotation collector) in much the same
>>>>>                         way as @Canonical was re-vamped. (This is
>>>>>                         for 2.5+).
>>>>>
>>>>>                         I have a preliminary PR which does this:
>>>>>                         https://github.com/apache/groovy/pull/653
>>>>>                         <https://github.com/apache/groovy/pull/653>
>>>>>
>>>>>                         Preliminary because it still needs a bit
>>>>>                         of refactoring to reduce some duplication
>>>>>                         of code that exists between the normal and
>>>>>                         immutable map and tuple constructors. I
>>>>>                         still need to do this but that can happen
>>>>>                         transparently behind the scenes as an
>>>>>                         implementation detail if we don't finish
>>>>>                         it straight away. As well as reducing
>>>>>                         duplication, the pending refactoring will
>>>>>                         enable things like the pre and post
>>>>>                         options for MapConstructor and
>>>>>                         TupleConstructor which aren't currently
>>>>>                         working.
>>>>>
>>>>>                         I am keen on any feedback at this point.
>>>>>                         In particular, while most of the
>>>>>                         functionality is pushed off into the
>>>>>                         collected annotations/transforms, I ended
>>>>>                         up with some left over checks which I kept
>>>>>                         in an annotation currently called
>>>>>                         @ImmutableClass. I tried various names for
>>>>>                         this class, e.g. @ImmutableBase and
>>>>>                         @ImmutableCheck but finally settled on
>>>>>                         @ImmutableClass since the annotation
>>>>>                         causes the preliminary checks to be
>>>>>                         performed but also acts as a marker
>>>>>                         interface for the MapConstructor and
>>>>>                         TupleConstructor transforms to do the
>>>>>                         alternate code needed for immutability and
>>>>>                         to indicate that a class is immutable when
>>>>>                         it might itself be a property of another
>>>>>                         immutable class. Let me know if you can
>>>>>                         think of a better name or have any other
>>>>>                         feedback.
>>>>>
>>>>>                         Cheers, Paul.
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>


Mime
View raw message