FindBugs Warning removal

Question

I am getting following error while running FindBugs:

Bug type NP_NULL_ON_SOME_PATH.

The code giving error is below:

List<Request> reviewerList = null;
    
response = restAPI CAll // assume some rest call.
reviewerListDto = response.getBody(); //it never returns null

// this check is added because of find bugs, else it wont allow to access getReviewers() method 
if (reviewerListDto!=null) { 
    reviewerList = reviewerListDto.getReviewers();
}
// HERE is the point I am stuck 
for (Request reviewer : reviewerList) {
    reviewer.setPullrequestId(pullrequest.getId());
    reviewer.setRepositoryId(pullrequest.getRepositoryId());
}

The error is coming because the reviewerList can be empty as per FindBugs in for loop. But I know that it will never be null. One way to remove it to check reviewerList for null before for loop, but is there any other way?

`reviewerList` remains `null` if `reviewerListDto` equals `null` — Jeroen Steenbeeke, May 31 '21 at 14:39
yes but reviewerListDto will never be null. I added that if condition eviewerListDto!=null because of find bug warning — user124, May 31 '21 at 14:58
I believe that by adding `if(reviewerListDto!=null) {` you are instructing Findbugs that `response.getBody` can in fact return `null`. — Jeroen Steenbeeke, May 31 '21 at 15:02
but if I dont put if, then i cannot access reviewerListDto.getReviewers(); becasue it thinks it can be NPE — user124, Jun 01 '21 at 06:05

score 0 · Answer 1 · answered May 31 '21 at 14:45

You can use @SuppressFBWarnings if you add com.google.code.findbugs:annotations to the classpath:

@SuppressFBWarnings(value="NP_NULL_ON_SOME_PATH", justification="This can never be null")

However, it is basically the same as why you put the if-statement, there. When adding an if-statements, you say that it could be that way and the code is only executed if that happens. If the if statement is always executed, there is no reason for an if statement.

So, you can also suppress the if statement and ignore the warning that caused you to add the if statement.

Another way would be to just put the code using reviewerList inside the if statement or to throw an exception (e.g. an AssertionError) if it is null (or use assert right-away).

List<Request> reviewerList = null;
// response = restAPI CAll // assume some rest call.
reviewerListDto = response.getBody(); //it never returns null
assert reviewerListDto!=null;
reviewerList = reviewerListDto.getReviewers();
for (Request reviewer : reviewerList) {
    reviewer.setPullrequestId(pullrequest.getId());
    reviewer.setRepositoryId(pullrequest.getRepositoryId());
}

what is the good practice here? For me, it is to check list for null instead of assert and suppress. — user124, May 31 '21 at 15:03
If you check for null, you should put the code using `reviewerList` in the `if`, too **and** think of what should happen if it is null. — dan1st, May 31 '21 at 15:05

FindBugs Warning removal

1 Answers1