Community
Participate
Working Groups
While working on null annotations in method signatures (bug 186342) I came to wish that completion in a case like this: class A { public @NonNull Object foo(@Nullable Object input) { return this; } } class B extends A { | // Ctrl-space here } when selecting proposal "override foo" should create s.t. like: @Override public @NonNull Object foo(@Nullable Object input) { // TODO Auto-generated method stub return super.foo(input); } All these annotations should be seen as part of the method specification and not copying them will probably render the override incompatible with the inherited method.
I also played with implementing this and found: The change could be made either in OverrideCompletionProposal or generally in StubUtility2.createImplementationStub(..) Should this feature actually also work for other clients of createImplementationStub? Implementing this for marker annotations is straight forward, I haven't tried to include any member value pairs. Currently parameter annotations may get lost on the way because they are not converted by {Source,Binary}TypeConverter, but this can be added trivially.
Copy all annotations is not always desired, e.g. I would not want @SuppressWarnings("all") to be copied.
(In reply to comment #2) > Copy all annotations is not always desired, e.g. I would not want > @SuppressWarnings("all") to be copied. Mh, saying this should be done for all possible annotation types is indeed an over-generalization. For your example I'd probably say manually removing the annotation is easier than adding all annotations where copying is the right thing. The distinction I see: some annotations are part of the method specification (like null annotations) whereas other annotations affect (also) the implementation (like @SuppressWarnings). In trying to find a way to distinguish SPEC annotations from IMPL annotations I wonder if @Retention(SOURCE) would be a good criterion to recognize IMPL annotations? Would it make more sense to keep a (configurable) positive list of annotations we want to copy? What do you think?
> Would it make more sense to keep a (configurable) positive list of > annotations we want to copy? That's the conclusion I also reached in bug 350396 comment 6. But @Retention(SOURCE) sounds even better. I think we should try that first (without any configurable options) and only resort to the white or black list if we have a real need.
> That's the conclusion I also reached in bug 350396 comment 6. > > But @Retention(SOURCE) sounds even better. I think we should try that first > (without any configurable options) and only resort to the white or black list > if we have a real need. +1.
Raksha, could you please have a look? We should use the same implementation also for Extract Interface (bug 350396) and probably more places where we produce overriding methods (Pull Up / Push Down).
To clarify (my initial statement may have been a bit dubious), do we agree that @Retention(SOURCE) describes the weaker kind of annotations that should *not* be copied, whereas all others *will* be copied?
(In reply to comment #7) > To clarify (my initial statement may have been a bit dubious), do we agree that > @Retention(SOURCE) describes the weaker kind of annotations that should *not* > be copied, whereas all others *will* be copied? Yes. With the justification that SOURCE annotations are usually only interesting for the compiler, but CLASS and RUNTIME annotations form a contract (e.g. modify the type of an element).
Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=ccad5c0389ff05c7e0ff5b98b2f9ba7115404f1c
Verified in I20120430-2000.
The automatic copying has been reverted for most cases, see bug 386410.
