Community
Participate
Working Groups
I20101102-0800 void foo() { String s="123"; String nullString= null; if(s.charAt(2)=='1') { nullString="not null"; } System.out.println(nullString.length()); //Potential null pointer access if(nullString!=null) { //Redundant null check System.out.println(nullString); } } 'Potential null pointer access' is correct, but 'Redundant null check' warning looks incorrect to me. If the sysout statement is removed 'Redundant null check' warning goes away. Whether the variable is accessed or not should not make a difference in the analysis about its null state. The only thing that should matter is whether the variable is assigned an object or not.
(In reply to comment #0) > If the sysout statement is removed 'Redundant null check' warning goes away. > Whether the variable is accessed or not should not make a difference in the > analysis about its null state. The only thing that should matter is whether the > variable is assigned an object or not. Maybe the wording can be improved, but I disagree with this. If the variable is used to invoke a method, then if it is null it would fail with a NPE. So in order to reach the if statement, nullString cannot be null. So if you check null, then it is redundant as it cannot be null at this location without a runtime failure with a NPE.
ok, maybe the warning is not incorrect, but seeing the 2 warnings I was a bit confused for a moment - is the variable null or is it not null?
(In reply to comment #1) >[..] > Maybe the wording can be improved, but I disagree with this. If the variable is > used to invoke a method, then if it is null it would fail with a NPE. So in > order to reach the if statement, nullString cannot be null. So if you check > null, then it is redundant as it cannot be null at this location without a > runtime failure with a NPE. Exactly. The warning's wording is "Redundant null check: The variable nullString cannot be null at *this location*". And so, it is better to see the warnings independently of each other. Since the if statement will only be reached when nullString is not null, the null check is redundant at *that location*. In that sense, both warnings are correct. In this case since they occur one after the other, I agree that they can be a little confusing sometimes. But they also tell the user the complete story only when they both are displayed. In the following example, void foo() { String s="123"; String nullString1= null; if(s.charAt(2)=='1') { nullString="not null"; } System.out.println(nullString1.length()); //Potential null pointer access if(nullString1!=null || nullString2!= null) { //Redundant null check on nullString1 System.out.println(nullString); } } this warning helps the user to convert the always true if statement to a conditional one by simply removing the redundant null check on nullString1. There may be other cases where the user can benefit from both warnings.
Closing as invalid, since the warnings are correct.
Verified for 3.7M4
