7

I commonly come across the following IntelliJ inspection 

private boolean bar() {    
    return foo().contains("foo"); // Method invocation 'contains' may produce 'java.lang.NullPointerException'
}

private String foo() {
    return null;
}

The inspections seem fine to me, but one of (or often times the only) suggested fix from IntelliJ is this:

private boolean bar() {
    return Objects.requireNonNull(foo()).contains("foo");
}

The warning is then gone. But I don't understand how this helps at all? requireNonNull will just throw the same NullPointerException that would have been thrown anyhow when .contains was invoked on null.

Usually, IntelliJ makes meaningful suggestions, and this is a common one, so am I missing the point here?

A1m
  • 2,897
  • 2
  • 24
  • 39
  • I agree the suggested solution is no better than the problem itself. – vavasthi Mar 21 '19 at 03:31
  • I guess the suggestion is meaningless here because you get an object and immediately use it. It's more meaningful if you get an object in one place and dereference in a different one. This way the exception will be thrown earlier, which is better. It still doesn't sound reasonable. –  Mar 21 '19 at 03:37
  • I agree. The solution is going to give you NPE anyway:) – Rakesh Mar 21 '19 at 14:27

1 Answers1

7

The reasoning behind using Objects#requireNonNull is similar to the one behind returning Optional: you do it to manage expectations.

  • When you return Optional<Something> instead of just Something you're saying: "Hey, I know this Something value might be missing, so instead of returning null and letting you guess, I'm returning an Optional<Something> instead to make it clear that you should expect it to maybe not be there, and that I expect you to check if it's there before using it."
  • When you call Objects.requireNonNull(something) in your code before using something, you're saying: "Hey, if you're reading this, I just wanted you to know that I expect this something parameter to not be null, and that I expect you to make sure it isn't before calling this code; so instead of just trusting you and outright calling something.contains(...), I'm invoking requireNonNull right here and now to make it clear (in case any of us missed the @NotNull annotation in the method declaration)."

Does it solve the problem? No, it doesn't. IntelliJ cannot magically forbid a parameter from being null. But it can force anybody reading that code to be aware that the parameter is not supposed to be null, and to reason about scenarios where that might happen.

walen
  • 7,103
  • 2
  • 37
  • 58
  • 4
    As IntelliJ IDEA developer responsible for this subsystem I confirm that this answer is correct. In general the idea is that `foo()` may probably return null elsewhere, but in this particular context I manifest that this should not happen. It's a better way to avoid warning than suppression. – Tagir Valeev Mar 22 '19 at 03:07