Explanations below.
On Tue, Jan 16, 2018 at 12:56 AM, MG <mg...@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 <mg...@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 <pa...@asert.com.au> <pa...@asert.com.au>
>> Datum: 13.01.18 13:17 (GMT+01:00)
>> An: MG <mg...@arscreat.com> <mg...@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 <mg...@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 <mg...@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 <pa...@asert.com.au>
>>>> Datum: 11.01.18 08:07 (GMT+01:00)
>>>> An: 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
>>>>
>>>> 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.
>>>>
>>>>
>>>
>>>
>>
>
>
