Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 505828

Summary:	[null] The "Inherit null annotations" feature should be stronger than @NonNullByDefault
Product:	[Eclipse Project] JDT	Reporter:	Michael Vorburger <mike>
Component:	Core	Assignee:	JDT-Core-Inbox <jdt-core-inbox>
Status:	CLOSED WONTFIX	QA Contact:
Severity:	critical
Priority:	P3	CC:	stephan.herrmann
Version:	4.6
Target Milestone:	---
Hardware:	PC
OS:	Linux
Whiteboard:	stalebug

Description Michael Vorburger

2016-10-12 16:47:27 EDT

The "Inherit null annotations" option intro. in bug 388281 IMHO doesn't inter-op "correctly" (i.e. not the way an end-user would typically expect) with @NonNullByDefault.  Example:

interface SomeInterface {

    void foo(@Nullable Object bar);
}

@NonNullByDefault
public class SomeClass implements SomeInterface {

    @Override
    public void foo(Object bar) {
    }
}

This causes a "The default '@NonNull' conflicts with the inherited '@Nullable' annotation in the overridden method from SomeInterface" error.  -- I do understand why it's saying that, and repeating foo(@Nullable Object bar) in SomeClass "fixes" the error of course  - but it's not what you would expect - after all, what's the point of having a feature to inherit null annotations if they're not... inherited, and you have to repeat them anyway, in this case? 

It would thus seem more "natural" if the inherited @Nullable annotations would automatically be "stronger" than @NonNullByDefault.

PS: I had also raised this in https://www.eclipse.org/forums/index.php/m/1741422/ 2 months ago already, but the analysis shown above is more succinct, and can hopefully contribute to a resolution of this.

Comment 1 Stephan Herrmann

2016-10-13 06:11:58 EDT

The current design of null annotation inheritance is a compromise. The background can be studied in bug 388281.

Summary:

Coming from bug 385440 there was a common understanding that annotation inheritance would be particularly important for migration scenarii: if super class gets annotated, this shouldn't force all "legacy" subclasses to react immediately. By means of annotation inheritance those subclasses could still be legal without changes.

Discussion regarding "conflicts" between inheritance and @NNBD starts in bug 388281 comment 7, were I argued that things could get quite confusing and "I don't want you to write this kind of code".

The discussion also touches on showing effective null annotations in hovers, which at that point was not planned, but meanwhile I've implemented that feature. So regarding visualization we are in a better position by now (but others made a strong point that code needs to be understandable also without the tool).

In bug 388281 comment 10 Sebastian proposed "the strongest annotation wins", which I answered by proposing "inheritance beats default".

Till today I am not 100% convinced which of the two approaches would be more useful / match more people's intuition.

Today's solution was introduced in bug 388281 comment 12.
Since then we treat inheritance and @NNBD equally as mechanisms for "implicit" null annotations.

In bug 388281 comment 20 Markus criticized that "this 'inheritance' is not well-defined and easily understood, and it carries UX problems with it..."

Agreement was achieved in bug 388281 comment 24 f.

Comment 2 Eclipse Genie

2019-07-04 07:35:38 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.