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.
