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