Bug 339250

Summary: [null] Incorrect redundant null check warning on a String
Product: [Eclipse Project] JDT Reporter: Ayushman Jain <amj87.iitr>
Component: CoreAssignee: Ayushman Jain <amj87.iitr>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: Olivier_Thomann, satyam.kandula, srikanth_sankaran, stephan.herrmann, sualeh
Version: 3.7Flags: Olivier_Thomann: review+
Target Milestone: 3.7 M6   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
possible fix
none
proposed fix v1.0 + tests none

Description Ayushman Jain CLA 2011-03-08 12:02:00 EST
HEAD

public class Snippet {
	public static void main(String[] args) {
	String test = null;
	test += "... some text";
	if (test != null) {
	System.out.println("It worked!");
	}
	}
}

Above case gives a bogus "null comparison always yields false" warning.
Comment 1 Olivier Thomann CLA 2011-03-08 12:07:29 EST
Note that:
test = test + "... some text";
removes the dead code warning properly.

So there is a missing reset of null info for the org.eclipse.jdt.internal.compiler.ast.CompoundAssignment.
Comment 2 Sualeh Fatehi CLA 2011-03-08 17:36:55 EST
See also: bug #339276
Comment 3 Ayushman Jain CLA 2011-03-09 03:03:36 EST
Created attachment 190734 [details]
possible fix

I observed that this kind of behaviour can only occur in the case of a String. This is because anything can be added to a null String, even the null literal itself. So,
test = null;         // test is null
test += null;        // test has a nonnull value i.e. "nullnull"
test += "abc";       // test has a value "nullnullabc"
For base types, such as int, float, etc. we dont need the null info, and for Integer,Float, etc, the null info will never change in a compound assignment operator.

Hence, this fix takes care of compound assign. for a String variable, and makes it definitely non null.
Comment 4 Ayushman Jain CLA 2011-03-09 04:43:12 EST
Created attachment 190741 [details]
proposed fix v1.0 + tests

Same fix with added tests
Olivier, please review. Thanks!
Comment 5 Olivier Thomann CLA 2011-03-09 08:49:20 EST
Looks ok.
Since it works fine, I would be tempted to release this as part of M6.
Ayushman, what do you think ?
Comment 6 Olivier Thomann CLA 2011-03-09 08:51:40 EST
*** Bug 339276 has been marked as a duplicate of this bug. ***
Comment 7 Olivier Thomann CLA 2011-03-09 09:00:56 EST
Released for 3.7M6 on the behalf of Ayushman.
Comment 8 Olivier Thomann CLA 2011-03-10 09:22:52 EST
Verified for 3.7M6.