RFR(XS): 8033909: Objects.requireNonNull(T, Supplier) doesn't specify what happens if the passed supplier is null itself

Volker Simonis volker.simonis at gmail.com
Tue Feb 14 07:36:20 UTC 2017

On Tue, Feb 14, 2017 at 12:56 AM, David Holmes <david.holmes at oracle.com> wrote:
> On 14/02/2017 2:57 AM, Alan Bateman wrote:
>> On 09/02/2017 16:18, Volker Simonis wrote:
>>> :
>>> I really think that the simplest and most natural fix for this problem
>>> is to simply check for a null Supplier argument and create the NPE
>>> with an explicit null message in that case (as outlined in the
>>> webrev). This:
>>>   - makes the two requireNonNul() overloads for String and Supplier
>>> behave the same (which I think was the initial intention).
>>>   - doesn't require documentation changes
>>>   - makes it possible to write a simple and conforming TCK test
>>> If we can't agree on this quickly, I suggest to close the issue as
>>> "will-not-fix" and leave it untested by the TCK.
>> If I read JDK-8033909 correctly then it's a request for the javadoc to
>> specify the expected behavior when called with a messageSupplier of
>> null. I think Joe said at one point that he deliberately didn't specify
>> this. If that remains the position then the issue can be closed as WNF
>> so that we don't waste any more discussion on it.
> I think the fact this method is intended to throw a NPE if the target object
> is null, complicates the "normal" position of "if any arg is null we throw
> NPE", and so requires explicitly being addressed. I would address this by
> making the supplier be optional, so that it is only used if non-null and so
> can itself never be the source of the generated NPE. That would then lead
> naturally to the implementation Volker proposes as being the only correct
> implementation.

Thanks David!

Finally somebody who understands me :)
As this seems a very controversial topic, I'll wait for a second
positive review before I'll push this.


> David
> -----
>> If I read your mail and patch correctly then it's more about what the
>> NPE message is. I agree the exception you pasted in from the SAP build
>> is difficult to read, in which case your patch makes it easier to read.
>> That make sense and maybe just push that with a separate issue.
>> -Alan

More information about the core-libs-dev mailing list