Feature proposal: make @NotNull annotation useful with fields assigned late

Answered

Hello JetBrains wise men :)

Assume class X has field "foo" initialized in some init() method (not in constructor):

class X {
  Object foo; // left uninitialized in constructor

   void init() { // I'm not a constructor
     foo = calculate_not_null_value();
   }
}

X.foo must be accessed (read) only after init() has been called at least once (init() can be called multiple times, each time assigning foo with a new value).
Accessing X.foo before the first init() is a logical error and should fail ASAP.

For example, this late initialization pattern is used in UI code (re)calculating component properties in updateUI().

Intention: I want IDEA help ensure, in the editor and runtime, that X.foo is not null when it is read-accessed.

Current possibilities:

1. First idea, obvious but wrong: annotate X.foo with @NotNull.

IDEA wants @NotNull fields be initialized and issues a warning otherwise.

The warning can be suppressed, but even worse thing is that IDEA does not seem to insert any runtime checks for @NotNull field read accesses, obviously assuming it cannot be else.

Not only #1 is useless, it also adds "yellow code". Totally bad.

2. Don't annotate X.foo at all or annotate it with @Nullable. On each access, explicitly check for null with assert, Objects.requireNonNull() etc.

This approach works, but adds boilerplate code (explicit checks for null on each access).

3. Access foo via getter that checks for null before return.

Works, but adds boilerplate code (the getter).

4. Use a wrapper to store the value:

class X {
  final Wrapper<Object> fooWrapper = new Wrapper<>();

  void init() {
    fooWrapper.set(calculate_not_null_value());
  }
}

On access, use fooWrapper.get() that will check for null inside.

Comparing with #3, this approach eliminates getters for each field (this example has one foo for simplicity, but in the real life there may be many such fields in a class).

However, each access still adds boilerplate code: it's "foo.get()" instead of "foo".

 

Conclusion: it seems we don't have a direct solution now, but only workarounds more or less elegant.

How I see the perfect solution:

Add a new annotation, let's say @AssignedLater (or whatever you prefer to name it), that should be used together with @NotNull:

@NotNull @AssignedLater Object foo;

(or, alternatively, a single @NotNullAssignedLater instead)


How this the annotation should work:

- Unlike "classic" @NotNull, it's OK that foo is not initialized in the constructor (furthermore, the opposite may deserve a warning).

- Unlike "classic" @NotNull, each time foo is read-accessed, it's checked for null.

- As with "classic" @NotNull, don't allow null be explicitly assigned to foo.

 

Best regards, Anton

4 comments
Comment actions Permalink

Thanks, that's an interesting suggestion (though I'm not very fond of the names). Would a separate @AssignedLater have any meaningful semantics by itself?

BTW you surely know that "NotNull fields must be initialized" check can be turned off in the inspection settings, and that initializing non-final fields later can lead to surprising data races in multithreaded environments?

0
Comment actions Permalink

Hello Peter

Thank you for the fast answer.

Yes, I know that inspections can be disabled, and about all possible troubles with non-final fields. The use case I mentioned comes from AWT/Swing so cannot be eliminated. I know other use cases too.

However, as I wrote, I would like to have runtime checks. This what makes IDEA compiler and corresponding ANT task unprecedentedly cool. As I understand, this is not what is controlled with inspection settings but is instead hard coded in the not-null-instrumenter.

Regarding the proposed annotation itself. No, @AssignedLater does not make sense alone, only in a combination with @NotNull.

Nevertheless, I think that there may be even better suggestion.

What about @NotNullRead (and maybe @NotNullWrite, however I do not see a use case for it at the moment)?

Semantics of @NotNullRead might be: each time the field is read-accessed, check that it's not null.

This sounds simpler to explain than my previous suggestion, but also covers my use case just fine.

@NotNullWrite, if it is needed at all, would mean "don't store null in this field". Again, I cannot say at the moment why this might be helpful.

@NotNullRead and @NotNullWrite put together are identical to @NotNull.

Best regards,

Anton

 

0
Comment actions Permalink

Update: another justification for @NotNullRead and @NotNullWrite:

I'm talking about field annotation, of which field I want to eliminate field accessors.

In this context, @NotNullRead serves the role of @NotNull annotation of the would-be getter's return value, and @NotNullWrite - the role of the @NotNull annotation of would-be setter's parameter:

@NotNullRead Object foo;

corresponds to

@NotNull Object getFoo() { return foo;}

and

@NotNullWrite Object foo;

corresponds to

void setFoo(@NotNull newValue) { foo = newValue;}

0
Comment actions Permalink

So, there seem to be two more or less separate issues:

  1. Add runtime assertions on reading/writing/reading-and-writing not-null fields. Sounds reasonable (and it's not obvious why it hasn't been done from the very beginning), but might lead to increased bytecode size and (maybe) performance degradation. It's also not easy (at least in the current architecture) to instrument field access outside of the field's containing class.
  2. Provide a way of not yellow-highlighting some non-initialized @NotNull fields in the editor. You're not alone in that, see https://youtrack.jetbrains.com/issue/IDEABKL-7280 and related issues.

The upside of your solution is that both problems are solved. The downside is complicating the annotation system, which might confuse people. Some think of @NotNull/@Nullable as part of the type, with additional rules for subtyping checks etc. We're even planning to make our annotations TYPE_USE only. But @NotNullRead isn't about only type anymore, it's about field, so it doesn't seem to fit that formalism.

It's a difficult decision that, I believe, requires more consideration. Let's continue the discussion in YouTrack: https://youtrack.jetbrains.com/issue/IDEABKL-7316

0

Please sign in to leave a comment.