Addressing cases like the AtomicReference zero-param constructor

[cpovirk]

@wmdietlGC and I were talking about AtomicReference. We see ~3 ways to annotate it:

1. All APIs operate on @Nullable T. (Then, for the declaration of T, we employ whatever pattern we prefer for parameters that want nothing to do with nullness.)

2. All APIs operate on plain T. T is declared to be definitely nullable. "Definitely nullable" is a feature that JSpecify may not support, at least initially, so this option would require additional support from tools.

3. All APIs operate on plain T. T is declared to permit both nullable and non-nullable type arguments.

The problem with (3) is the no-arg AtomicReference constructor. It creates an instance whose value is null, even if T is a non-nullable type:

AtomicReference<String> ref = new AtomicReference<>();
ref.get().hashCode(); // NPE

So we might end up going with (1) for now. But (1) is annoying because it provides no way for JSpecify to know that get() returns a non-nullable value -- even if the user passed a non-null value to the constructor and never set the value to null.

So we might like (3). That could take the form of:

• banning that constructor entirely (requiring users to write new AtomicReference<>(null) if that's what they want)
• some way to restrict instantiations of a constructor (Restrict the instantiation type on a constructor typetools/checker-framework#1330) -- and perhaps of calls to a method, too (currently available in CF but not in our immediate plans for JSpecify)

(This problem does not exist with static factory methods, since we already provide a way to set bounds there.)

A "@DoNotCall" annotation is something that we could do someday (and something we've found useful in Error Prone), so maybe this is just our first issue to request that. Or maybe there's something nullness-specific here that we'd want to do.

[cpovirk]

This could hypothetically be nice for Guava's UncheckedExecutionException: It would be nice if users could assume that getCause() returns a non-null value. But to guarantee that, we'd need to not only annotate the constructors' cause parameters as non-null but also ban usages of the constructors that don't accept a cause.

[edit: a little more on this]

[kevinb9n]

I'm gonna treat this issue as being about The Interesting Case Of AtomicReference, rather than the general topic, if that's okay.

# 3 seems worth trying for. The fact it at least stops you from actually setting a nullable reference is useful: then if an unexpected null does happen, it's easier to conclude that the setting code probably never ran, instead of wondering whether maybe that expression was itself unexpectedly null.

If our hypothetical API owner agreed with that, then ideally they could proceed to make a design choice between options like:

1. Null is the default value, so the read methods will all project to @Nullable T and all users will just have to deal with that case.
2. Null is the default value, but all those projections will be a pain for users so let's just lie instead. <-- I think we all agree that we should discourage this in strong terms.
3. There should be no default value, so what the no-arg constructor really produces is an uninitialized instance. We can't deprecate that, so this class becomes another example of an API with a sequencing contract ("must use at least one write operation (including 1-arg constructor) before any read operation"). It should communicate this contract clearly to its users (whether it has a way to do this using annotations or not).
4. Ban that constructor in any case, as originally suggested. I'm not sure this would be justified yet.

# 3 leads to an interesting question: suppose the user A breaks the sequencing contract (for example because they don't have tools that know how to enforce it!), and user B uses the class correctly. The read methods can be annotated sensibly for user A (nullable) or user B (non-nullable), but we have to choose.

EDIT: Call those options 3A and 3B. The difference between 3B and 2 is that 2 is just a lie, but 3B informs the user of what bounds they need to stay within, and tells the truth about what happens as they long as they do.

One viewpoint toward 3A vs. 3B would be that 3B can only be justified if the sequencing contract is itself expressible through JSpecify. That maintains certain guarantees that tools would all be able to make.

Personally, I favor 3B in either case. I think there are always a long tail of bad actions you can take that will break an object's state such that it doesn't conform to its contract. You can change a Guava ImmutableList through reflection, etc.

[kevin1e100]

FWIW another example for this is ThreadLocal I believe

[kevinb9n]

3B seems to kind of put AtomicReference itself into the same category as "fields" and "array components". All 3 have this initialization problem.

[kevinb9n]

So my proposal is that we useAtomicReference<T extends @Nullable Object> and we just add this class to the list of "things we have initialization concerns about". Does anyone feel we need to keep this open?

[kevinb9n]

Came across this again; wrote up https://github.com/jspecify/jspecify/wiki/nullness-borderline-cases#the-atomicreference-case