Summary: | Incorrect redundant null check warning | ||
---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Deepak Azad <deepakazad> |
Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> |
Status: | VERIFIED INVALID | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | Olivier_Thomann, srikanth_sankaran |
Version: | 3.7 | ||
Target Milestone: | 3.7 M4 | ||
Hardware: | All | ||
OS: | All | ||
Whiteboard: |
Description
Deepak Azad
2010-11-05 09:43:30 EDT
(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 |