Annotate much of java.time for nullness.

[cpovirk]

No description provided.

[cpovirk]

The docs for get and getUnits are not explicit about whether their TemporalUnit instances can be null.

Both JDK implementation of TemporalAmount reject nulls from get (Period, Duration) -- albeit by throwing UnsupportedTemporalTypeException instead of NullPointerException. This means that we need to omit @Nullable here if we want to be safe.

For getUnits, the TemporalUnit instances are an output, so we could in principle still have @Nullable there. But java.time is consistent in treating a null TemporalUnit as bogus: See, for example, Temporal.isSupported, or see the use of similar TemporalField in TemporalAccessor.get. That probably argues for doing the same here. (Alternatively, it argues that this one exception could have been intentional :) But I think that would be at least a little perverse.)

[mernst]

Thanks for these annotations. They are a useful addition. I have one concern.

Method TemporalQuery.queryFrom is documented as possibly returning null, but its return type is R and R might be instantiated as a non-null type. This seems wrong to me. I have the same concern about TemporalAccessor.query and its overrides. Am I missing something?

[cpovirk]

I found the docs around TemporalQuery and TemporalAccessor to be not very explicit about nullness, so I went through a chain of reasoning for them :(

TemporalQuery is perhaps the simpler case: It's a fully abstract, essentially a Function<TemporalAccessor, R>. Thus, R appears in only one place, the return type of queryFrom. So the question in my mind is: Does it make sense to distinguish between a TemporalQuery that might return null and one that never does? Returning null is the documented behavior of the implementations in the TemporalQueries utility class. But, as the TemporalQuery docs say, most callers will pass method references like LocalDate::from. And LocalDate.from is defined never to return null. It seems useful for callers of TemporalQuery.from to get the most precise return type possible (based on the type of the argument they pass), since null-returning queries exist but are atypical.

And when I say "callers of TemporalQuery.from," I really mean only the people who call it indirectly through TemporalAccessor.query and DateTimeFormatter.parse​. (There appear to be no direct callers of TemporalQuery.from in Google's codebase!) This is where the docs are especially frustrating: I think it makes perfect sense for temporalAccessor.query(temporalQuery) to return null if and only when temporalQuery could return null. But TemporalAccessor.query, while it documents a default implementation that behaves this way, does not come right out and guarantee it for all implementations.

Ah, but as I am writing this, I see the following on TemporalQuery:

There are two equivalent ways of using a TemporalQuery. The first is to invoke the method on this interface directly. The second is to use TemporalAccessor.query(TemporalQuery).

So I believe that we have justification to conclude that TemporalAccessor.query can return null only when R is a @Nullable type.

I've added a @CFComment to each file, attempting to summarize this more briefly.