JDK-8033909 : Objects.requireNonNull(T, Supplier) doesn't specify what happens if the passed supplier is null itself
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util
  • Affected Version: 9
  • Priority: P3
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2014-02-07
  • Updated: 2017-02-16
  • Resolved: 2017-02-16
Related Reports
Relates :  
JDK-8174950 - Gracefully handle null Supplier in Objects.requireNonNull
Relates :  
JDK-8011800 - Add java.util.Objects.requireNonNull(T, Supplier)
Description
The following JavaSE8's method :

http://download.java.net/jdk8/docs/api/java/util/Objects.html#requireNonNull-T-java.util.function.Supplier-

doesn't specify what will happen if the passed instance of the message supplier is itself null - would there be an NPE thrown and what message would it contain.

Not having this specified prevents from developing conformance tests for this scenario.
Comments
In my estimation, the current specification is sufficiently precise as-is. Closing as will not fix.
16-02-2017
As Alan states in the review thread (http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-February/046364.html) this bug is about the specification of Objects.requireNonNull(T, Supplier). I've therefore reassigned it back to you Joe as Alan proposed and opened a new issue (JDK-8174950) for fixing the implementation. Please feel free to close this bug as WNF if you think that's appropriate.
14-02-2017
Volker's patch is an implementation change and I think should be tracked as a separate issue. I think JDK-8033909 should be re-assigned back to Joe to decide on whether he agrees with a spec change, if not then the issue should be closed as WNF.
14-02-2017
I favour some variant of documentation change rather than an implementation change.
05-03-2014
Review thread: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024989.html
05-03-2014
Volker, feel free to make yourself the assignee of this issue and propose a patch to core-libs. Thanks.
08-02-2014
Sorry, but I really can't understand why we make such fuss about this topic. From a user perspective, I think requireNonNull(T obj, Supplier<String> messageSupplier) should behave exactly like requireNonNull(T obj, String message). After all, the latter was introduced as specialization (and optimization under certain circumstances) of the first one. If requireNonNull with a String message argument is called with a null obj and a null message it just constructs a NPE with a null message. The same method with a Supplier argument just do exactly the same thing and not relying on the VM to 'do the right thing'. It should as well simply throw a newly created NPE with an explicit null message. And all that's as trivial as the before mentioned one line change: throw new NullPointerException(messageSupplier == null ? null : messageSupplier.get()); No API doc and no TCK changes required! So what's the problem? Should I file a new bug for this and provide a webrev for review?
07-02-2014
As an API design point, I prefer when the body of the text of the API doesn't get bogged down explaining in detail what happens when multiple preconditions of the method are violated. The JCK team is not the only consumer of the specification and I don't think expounding on such details helps normal users very much. When I work on APIs, I try to include a general "unless otherwise specified, if you pass in a null argument you will indeed get a NPE" once per file so that every single method's javadoc is not littered with distracting "you know, if you pass *this* argument as null, an NPE will result."
07-02-2014
Dmitry sent mail acknowledging that the conformance test shouldn't be testing unspecified behavior and he is working to remove or exclude the test.
07-02-2014
OK, but then there should be no TCK tests which checks that such an implicitly, VM-generated NPE has a null message. api/java_util/Objects/index.html#RequireNonNullMessageSupplier did exactly this, but Dimitry already promised that it will be removed from the Java 8 TCK.
07-02-2014
I agree with the change in priority of this bug from p2 -> p3. Even if it is relevant, lack of explicit "you get an NPE when you pass in null" hardly justifies a p2 setting. The specification of this method states an NPE is thrown if the first argument is null; that is an accurate description if its behavior. The treatment of the null-ness of the argument in intentionally non-symmetric. If the first argument is non-null the second argument should not be examined at all. If both argument are null, an NPE should be thrown and is thrown by the RI.
07-02-2014
I read Joe's comment but he relays on unspecified VM behaviour (i.e. a VM generated NPE has a null message) which is not specified anywhere (see our implementation below). So why not simply change requireNonNull() to something like: public static <T> T requireNonNull(T obj, Supplier<String> messageSupplier) { if (obj == null) throw new NullPointerException(messageSupplier == null ? null : messageSupplier.get()); return obj; } and the JavaDoc to: * If the supplier is {@code null} then a {@code NullPointerException}, with * a {@code null} message, is thrown. This doesn't seem to be too much effort (like the solution he had in mind with another call to requireNonNull()) and it would also save the current JCK test (api/java_util/Objects/index.html#RequireNonNullMessageSupplier) which was the initial cause for this report. I just ask, because our SAP JVM will currently print something like: java.lang.NullPointerException: while trying to invoke the method java.util.function.Supplier.get() of a null object loaded from the second parameter of the method at java.util.Objects.requireNonNull(Objects.java:290) which may be strange for the user which expects to get a plain NPE from requireNonNull(). In this case the extra information which is a nice feature otherwise may be a little confusing.
07-02-2014
Suggested tweak to the JavaDoc: /** * Checks that the specified object reference is not {@code null} and * throws a customized {@link NullPointerException} if it is. * * <p>Unlike the method {@link #requireNonNull(Object, String)}, * this method allows creation of the message to be deferred until * after the null check is made. While this may confer a * performance advantage in the non-null case, when deciding to * call this method care should be taken that the costs of * creating the message supplier are less than the cost of just * creating the string message directly. * * @param obj the object reference to check for nullity * @param messageSupplier supplier of the detail message to be * used in the event that a {@code NullPointerException} is thrown. * If the supplier is {@code null} then a {@code NullPointerException}, with * an unspecified string message, is thrown. * * @param <T> the type of the reference * @return {@code obj} if not {@code null} * @throws NullPointerException if {@code obj} is {@code null} or the * {@code messageSupplier is null} * @since 1.8 */ This is arguably backwards compatible since there is really only one viable implementation when the messageSupplier is null: an NPE with an unspecified (or impl specific) message.
07-02-2014
See also the review thread where Joe noted that he deliberately did not specify the supplier == null case. http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/015876.html
07-02-2014