error-prone icon indicating copy to clipboard operation
error-prone copied to clipboard

FieldMissingNullable doesn't complain when variable is not initialized

Open Marinell0 opened this issue 4 months ago • 5 comments

Current behavior

With FieldMissingNullable turned on and the following code:

import java.time.OffsetDateTime;

public class Clazz {
	private OffsetDateTime closeTimestamp = null;

	public Clazz() {
		// Do nothing
	}

	public OffsetDateTime getCloseTimestamp() {
		return closeTimestamp;
	}

	public void setCloseTimestamp(OffsetDateTime closeTimestamp) {
		this.closeTimestamp = closeTimestamp;
	}
}

We get the following warnings:

[FieldMissingNullable] Field is assigned (or compared against) a definitely null value but is not annotated @Nullable
    (see https://errorprone.info/bugpattern/FieldMissingNullable)
  Did you mean 'private @Nullable OffsetDateTime closeTimestamp = null;'?

But when we remove the explicit null assignment on class startup, the warning goes away, even tho the default value for a non initialized Object variable is null

Expected behavior

When:

  • Variable is an object (not primitive)
  • Variable is not initialized when declared
  • No assignment is made in constructors

We should also get a warning:

[FieldMissingNullable] Field is assigned (or compared against) a definitely null value but is not annotated @Nullable
    (see https://errorprone.info/bugpattern/FieldMissingNullable)
  Did you mean 'private @Nullable OffsetDateTime closeTimestamp;'?

Marinell0 avatar Aug 25 '25 15:08 Marinell0

Thanks, we should support that, at least optionally. I might have just overlooked it when I revamped the check, or I might have been worried by how some people prefer not to use @Nullable if a field is initialized lazily after object creation. (I'm mostly not a big fan of that approach, but I see how there are some cases in which it's pragmatic.) None of that prevents us from at least offering an option for this.

Our nullness work is mostly on the back burner at the moment, and I've already told at least one other person that I'm hoping to publish a nullness-related Error Prone checker, so it might be a while before this is at the top of my list. It's probably fairly straightforward to implement if you're in position to do that, so that may be an option if you want to speed things along. (Maybe I should have just spent time to see how long it would take to implement instead of writing this... :))

cpovirk avatar Aug 30 '25 17:08 cpovirk

or I might have been worried by how some people prefer not to use @Nullable if a field is initialized lazily after object creation. (I'm mostly not a big fan of that approach, but I see how there are some cases in which it's pragmatic.)

I think this is why NullAway has the concept of initializers (e.g. servlets' init method) though possibly not the same use case as what you're thinking about with "lazily" (but still would be caught by such a change to the check)

I'm hoping to publish a nullness-related Error Prone checker

How would it compare to NullAway?

tbroyer avatar Aug 30 '25 18:08 tbroyer

Thanks for the quick response! I'll have a look at the code and see if I can do something about it =p

As @tbroyer said, NullAway takes care of that, but I think it would be wise to ask for a nullable tag as, even with lazy initialization, if the programmer did something wrong, it could be null

Marinell0 avatar Aug 30 '25 18:08 Marinell0

AFAICT, NullAway does that analysis and should prevent any way to leave the field uninitialized (thus null)

tbroyer avatar Aug 30 '25 18:08 tbroyer

Yes, sorry, the other thing I promised is not a "checker" in the sense that NullAway is. It's just implemented as an Error Prone "checker." I believe the one I promised is for migrating from one nullness annotation to another, as in an existing OpenRewrite recipe.

cpovirk avatar Aug 30 '25 19:08 cpovirk