Allow a @Nullable value for the second parameter of requireNonNullElse.

[cpovirk]

The existing approach follows the Checker Framework philosophy, under
which passing null for a @Nullable parameter should never lead to a
NullPointerException. (Compare Method.invoke, whose "receiver" and
"arguments" parameters are declared as non-nullable by the Checker
Framework, even though some callers can pass null for the "receiver"
and some must pass null for some of the arguments.)

The JSpecify thinking has moved in the direction of saying that, if
null should ever be permitted for a parameter, then the parameter
should be declared with a type that includes null. And we know that
many callers pass null for the second parameter. Here's an imaginary
such caller:

for (Key key : union(map1.keySet(), map2.keySet()) {
  Value value = requireNonNullElse(map1.get(key), map2.get(key));
}

(We could consider making a similar change to requireNonNullElseGet.
But it's rarely used, and perhaps no one wants to pass a Supplier that
may return null, anyway. So I'm happy to leave it alone for now or to
change it if that's more palatable.)

[wmdietl]

I'm confused by the example in the PR description, because it uses firstNonNull and not requireNonNullElse.
The method and the parameter javadoc for requireNonNullElse both explicitly say that the second parameter has to be non-null, so I'm not sure allowing null here makes sense.
Can you elaborate?

[cpovirk]

Sorry about the PR description: I had reused it from google/guava@4aaebca without editing it enough. I've updated it to use requireNonNullElse.

And wow, that doc is not especially clear. I had been focused on the following part, which makes it sounds like a null second argument is fine (at least in situations like the one in my PR description):

jdk/src/java.base/share/classes/java/util/Objects.java

Lines 318 to 319 in 234d208

	* @throws NullPointerException if both {@code obj} is null and
	* {@code defaultObj} is {@code null}

There's some further text to that effect, too:

jdk/src/java.base/share/classes/java/util/Objects.java

Lines 316 to 317 in 234d208

	* @return the first argument if it is non-{@code null} and
	* otherwise the second argument if it is non-{@code null}

But now I see what you're looking at:

jdk/src/java.base/share/classes/java/util/Objects.java

Lines 313 to 314 in 234d208

	* @param defaultObj a non-{@code null} object to return if the first argument
	* is {@code null}

And arguably also this, but less so:

jdk/src/java.base/share/classes/java/util/Objects.java

Line 310 in 234d208

* otherwise returns the non-{@code null} second argument.

I think part of the question is how much the @param clause's "if" is supposed to apply to. Is it:

if the first argument null, [then defaultObj must be] a non-null object to return

Or:

[defaultObj must be] a non-null object [which the method will] return if the first argument [is] null

I'm annotating the method to match the former; you're advocating for the latter.

I have no real argument here, just "Obviously :) they meant to permit null": Doing so is useful in examples like mine, and rejecting null entirely would have been so easy. But the doc does everything it can to muddy the waters—really, everything it can to suggest that I'm wrong without being 100% clear about it.

[wmdietl]

Thanks for your updated example and explanation! This makes much more sense now.

If we just focus on values in legal executions, there are calls like requireNonNullElse(nonNull, null), where the second argument is null and nothing bad happens. So by our definition, I would agree we should make the parameter @Nullable.

If I were to rewrite your example to make it more "obvious", I would end-up inlining the body of requireNonNullElse. So again, it would make sense to allow abstracting out that logic into a helper method.

With a more conservative POV, I would continue to argue that only non-null values make sense for the second argument, to help avoid all NPEs at compile time. But then I would also want the first parameter of requireNonNull to be @NonNull, which we have discussed more than enough.

I think this is another example where we want additional preconditions on the method, to precisely express that either of the arguments (or both) needs to be non-null. The current @RequiresNonNull annotation is not strong enough to express such a contract.

So overall, I agree with making this change.