There's cake!

[Alex Buckley]

Hi Reinier,

Obviously, a world without container annotations is much simpler, but we 
are stuck with them. In terms of how your proposal handles them, I note 
that:

- It still requires the containing annotation type to be marked 
(@ContainerFor(A.class)) for the benefit of legacy clients.

- It still requires a sensible relationship between the containing and 
repeatable annotation types. (For example, it would be silly to allow 
getAnnotation(FooContainer.class) to succeed auto-magically if 
FooContainer.class truly exists and has @Retention(SOURCE).)

- It still requires "look through" by the core reflection and language 
model APIs when presented with legacy class files that physically have 
container annotations.

- It has the same compatibility impact at runtime for the common case 
(Example 1.2-1), namely the exposure of multiple annotations of the same 
type.

(Frankly, I expect the last point to cause far more teeth-gnashing than 
the container/containee relationship, since only the API owner sets that 
relationship whereas untold numbers of clients are affected by 
behavioral incompatibility. We considered having the SE 7 reflection 
methods behave identically in SE 8 to SE 7, but decided instead to make 
the methods "do the right thing" in SE 8 and beyond - the language 
allows multiple annotations of the same type, so reflection should too.)

All of the above makes me think there is very little simplification in 
your proposal. You've just changed the "opt in" flag from 
@ContainedBy(...) to @Repeatable. And there are two downsides:

- Since the Foo type is just "repeatable", there is no control over 
_where_ @Foo may be repeated. It is an important aspect of the Oracle 
proposal that the annotation type owner controls the kinds of 
declarations (classes, fields, etc) on which their annotation may occur 
more than once (via @Target on the containing annotation type).

- Your reflection behavior is to synthesize a container annotation only 
if the client asks for one and there's at least one base annotation 
present. A legacy client may legitimately expect to find @FooContainer 
with an _empty_ value(), but no such annotation will be synthesized. I 
also suspect the rules for container synthesis are more complex than 
presently thought.

The current design has been publicly specified for over three months and 
the implementation is pretty much complete, so it's a bit late to 
redesign things.

An orthogonal issue is reflection over a mix of base and container 
annotations. There is a deliberate policy laid out in section 1.2 that 
@Foo(1) on the subclass hides @Foo(0) (whether contained or not) on the 
superclass. Under a different policy, where no Foo hiding occurs, we 
would still "look through" @FooContainer and return @Foo(0) from the 
superclass + @Foo(1) from the subclass. I am surprised you think that 
getAnnotations() in SE 8 should ever return a container. Again, Oracle 
is happy to propose that SE 7 methods like getAnnotations() change 
behavior if it's "the right thing" going forward.

Alex

On 11/26/2012 1:22 PM, Reinier Zwitserloot wrote:
> I must echo Paul's sentiments here: Are we really about burden the future
> of java _forever_ just to cater to existing containers?
>
> Eh, no worries: There's a way to simplify this proposal considerably and
> yet continue to support these. Yep, we can have our cake, and eat it too.
>
> Before I continue with a proposal to (vastly!) improve the spec, I'd like
> to state my assumptions here:
>
> * According to the Java EE team, there is significant use of container
> annotations out there, and we should not just rely on these libraries to
> use deprecation mechanisms to migrate away from them. In short:
> .get(Declared)Annotation(SomeExplicitlyDefinedContainerAnnotation.class)
> needs to continue to work even if some element is annotated with `@Foo(1)
> @Foo(2)` instead of `@FooContainer({@Foo(1), @Foo(2)})`.
>
> * Burdening the future of java with (other than preserving backwards
> compatibility) container annotations that have to be explicitly defined and
> which are public API is a extremely high cost to pay, and something we
> should strive to avoid at all costs.
>
>
> It is clear to me that we should try to find a way to have our cake and eat
> it to: Ditch the concept of container annotations entirely, and yet, allow
> already existing containers to continue to work.
>
> This seems easy to do. The proposal to make this work is as follows:
>
> * Allow multiple annotations to be supported in the most natural way
> possible: `@Foo(1) @Foo(2)` is compiled to simply having two @Foo
> annotation blocks in the class file; absolutely no need for @ContainedBy,
> nor for any explicit or implicit container annotation. The reflection API
> implementations become much simpler as a result as the 'weird' scenarios
> where there is a mix of the annotation itself and its container annotation
> all disappear. To allow user control of cardinality, a '@Repeatable' meta
> annotation can be created to signal that some annotation can be repeated on
> its target elements.
>
> * To continue to support explicit container annotations, we add the
> following feature to the reflective APIs: _IF_ any annotation is annotated
> with `@ContainerFor(A.class)`, _AND_ there is a request for this specific
> annotation, e.g. .getAnnotation(FooContainer.class), _AND_ the result
> _would_ be null (i.e. there is no @FooContainer on the element at all),
> _AND_ at least one annotation of type A is present, then then the
> reflection API will provide an instance of FooContainer anyway, filling the
> value() array with each instance of A on the chosen element. The same rules
> apply here (such as: The return type of the value() method must be A[]) as
> in the current version of the proposal.
>
>
> The above proposal has every advantage of the current proposal and almost
> none of its considerable disadvantages. to wit:
>
> * The library author must update the container annotation explicitly. This
> is true both in the current proposal (@ContainerFor must be added), as it
> is in this new proposal (@ContainerFor must also be added).
>
> * In the new proposal, there is absolutely no need for a container of any
> sort for any new API which is built for java8+. In the current proposal,
> when containers should have been a distant memory, we'll be stuck with them
> forever.
>
> * Unavoidably surprising behavior occurs when you mix the container and the
> base annotation. In the current proposal, the answer given by the
> reflection API sometimes changes between JDK7 and JDK8 and sometimes it
> doesn't. In this proposal, backwards compatibility actually improves:
> .getAnnotation(SomeContainer.class) will give the exact same answer, by
> definition, as JDK7's answer, _UNLESS_ that answer would be 'null'. For
> example, in the current proposal, @Inherit can change the rules, and result
> in a different answer:
>
> @FooContainer(@Foo(0)) public class A {}
> @Foo(1) public class B extends A {}
>
> Asking for .getAnnotations() will return just @Foo(1) and nothing else; in
> JDK7, you'd get both @FooContainer and @Foo. In my proposal, you get the
> same answer as JDK7.
>
> Again, the one and only backwards incompatible change is that _IF_ the
> answer _WOULD_ be null, then instead you get a generated answer.
>
> The practical upshot is very simple:
>
> * Developers of new APIs, or anybody who has an aggressive deprecation
> policy forgets entirely about the idea of containers or removes them from
> their API, and gets the benefit of a much simpler reflection API forever.
> They do not have the burden of having to maintain a public container
> annotation.
>
> * Developers of existing APIs stick a '@ContainerFor' on their container,
> @Repeatable on the base annotation, and continue to publish this container
> forever. Their code uses .getAnnotation(FooContainer.class) and reads that
> annotation's value() method instead of ever using
> .getAnnotations(Foo.class). This actually means they get to keep using
> their current code instead of changing to a new API!.
>
> It's win, win.
>
>   --Reinier Zwitserloot
>