CODEWITHSUNDEEP
java
rickygrimes
rickygrimes

Reputation: 2716

Sonar critical violation - Nullcheck of value previously dereferenced

For the below piece of code I have in one of my test classes, Sonar throws me a critical violation - Correctness - Nullcheck of value previously dereferenced

 if (testLst != null && !testLst.isEmpty()) {
        for (Test test : testLst) {
            if (test.getName().equalsIgnoreCase("TEST")) {
            // do blah
            }

Can someone throw some light on this on what am I doing wrong here?

EDIT: One of the answers here suggested this is because I could have accessed the variable before, and so the null check is redundant. That's not true though. Here is the line of code before my null check.

 testLst = myTest.getValues(); //I am basically populating the array by doing a get, but I am not accessing the list itself by doing a get on it directly - like testLst.get()
 if (testLst != null && !testLst.isEmpty()) {
            for (Test test : testLst) {
                if (test.getName().equalsIgnoreCase("TEST")) {
                // do blah
                }

Upvotes: 8

Views: 28554

Answers (3)

Chry007
Chry007

Reputation: 541

I know I am too late for OP but maybe someone else might find this helpful:

Sonarqube throws this error at the line, that would cause the initial NPE, not the line that includes the redundant null check (the error message indicates otherwise)

For OP:

testLst = myTest.getValues();

I am guessing getValues() never returns null therefore the testLst cannot be null but only empty at the point of the null check.

Upvotes: 2

Mickaël Guerin
Mickaël Guerin

Reputation: 1

testLst = myTest.getValues(); //I am basically populating the array by doing a get, but I am not accessing the list itself by doing a get on it directly - like testLst.get()
if (testLst != null && !testLst.isEmpty()) {
    for (Test test : testLst) {
        if (test.getName().equalsIgnoreCase("TEST")) {
        // do blah
        }

Did you checked myTest != null ? Otherwise sonar will throw the critical violation I guess.

Upvotes: -1

M A
M A

Reputation: 72874

This message is shown when you're checking if a variable's value is null (in this case testLst) whereas you already accessed the variable before. The null check is not needed since if the value was null, a NullPointerException would have been thrown.

Example:

testLst.remove(something);
if (testLst != null && !testLst.isEmpty()) {
    for (Test test : testLst) {
       if (test.getName().equalsIgnoreCase("TEST")) {
        // do blah
        }

The check testLst != null is redundant since at the time the program reaches the if statement, testLst cannot be null, otherwise the previous statement testLst.remove(something) would have thrown a NullPointerException. In this case, you should place the null check before accessing testLst, in a place where it can be null:

if(testLst != null) {
   testLst.remove(something);
   if (!testLst.isEmpty()) {
       for (Test test : testLst) {
          if (test.getName().equalsIgnoreCase("TEST")) {
           // do blah
          }

Upvotes: 14

Related Questions