SonarQube - A "NullPointerException" could be thrown;

Question

SonarQube reports this bug to me: A "NullPointerException" could be thrown; "getBody()" can return null.

This is the code:

if (holdingResponseEntity == null || holdingResponseEntity.getBody() == null || holdingResponseEntity.getBody().getError() || holdingResponseEntity.getBody().getResult() == null) { throw new HoldingNotFoundException("Holding whit id=" + idHolding + " not found-"); }

Soudan

Answer 1

You first test if getBody() returns null.

SonarQube sees that and thinks the method can return null.

Then, you call the same method again. SonarQube just knows that the method can return null so it tells you the warning.

In other words, SonarQube thinks that it could return something different than null the first time but return null the second time.

If the method is just a simple getter and the object is not modified concurrently, this is no problem.

In order to remove the warning, you can do one of the following:

• Save the result of getBody() to a variable, check if it is null and continue if it is not
• Add a //NOSONAR comment telling SonarQube that you know what you are doing.

Note that you might want to explain why this is ok in the comment, if you decide for the second case.

Newel

Answer 2

It could theoretically be the case that a repeated call of getBody() might return an other value. The following utilizes hasBody() and calls getBody() only once.

if (holdingResponseEntity == null
        || !holdingResponseEntity.hasBody()) {
    throw new HoldingNotFoundException("Holding with id=" + idHolding + " not found.");
}
T body = holdingResponseEntity.getBody(); // Change T appropriately.
if (body.getError() || body.getResult() == null) {
    throw new HoldingNotFoundException("Holding with id=" + idHolding + " not found.");
}

Normally you should check the source code of the library getBody. My guess: it is not a simple getter, but construes the body once. Because there is a hasBody.

Such things make code checkers valuable.

Baguio

Answer 3

However there are other ways to solve this as well, as answered above but if I want to solve it the way you have written the code with minimum changes and avoid warning from Sonar, this would be as below -

Using && operator will make sure once the condition fails , it would not check for subsequent condition , and that solves the issue .

if(holdingResponseEntity != null &&
                holdingResponseEntity.getBody() != null &&
                holdingResponseEntity.getBody().getError() != null &&
                holdingResponseEntity.getBody().getResult() != null){
           //Everything is good if it comes here

        }else{
            System.out.println("Going to throw ***** ");
            throw new HoldingNotFoundException("Holding whit id=" + idHolding + " not found-");
        }

Tenancy

If i understand correctly, your code still got a warning from SonarQube. Take a look at holdingResponseEntity.getBody() != null and holdingResponseEntity.getBody().getError() !=null. The first one is oke but the second one can throw NPE because the second one call getBody() again, and result can be null in theory. Unless you're checking like this (body=holdingResponseEntity.getBody()) != null && body.getError() != null – Honna

Answer 4

There is multiple way to solve this but in some of the scenario sonar will fail to check the condition so better you can use Optional to check null and sonar issue will be resolved For Example:

Optional.ofNullable(responseEntity.getBody()).isEmpty()

Planometer

Answer 5

Lets assume that holdingResponseEntity.getBody() is returning HoldingResponse;

HoldingResponse holdingResponse = holdingResponseEntity.getBody();
if (null!=holdingResponse && null!=holdingResponse.getErrors()) {
    throw new HoldingNotFoundException("Holding whit id=" + idHolding + " not found-");
}

For more details check this https://community.sonarsource.com/t/false-positive-a-nullpointerexception-could-be-thrown/47025/3

Mceachern