Bug 339250 - [null] Incorrect redundant null check warning on a String
Summary: [null] Incorrect redundant null check warning on a String
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 M6   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 339276 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-03-08 12:02 EST by Ayushman Jain CLA
Modified: 2011-03-10 09:22 EST (History)
5 users (show)

See Also:
Olivier_Thomann: review+


Attachments
possible fix (1.87 KB, patch)
2011-03-09 03:03 EST, Ayushman Jain CLA
no flags Details | Diff
proposed fix v1.0 + tests (5.90 KB, patch)
2011-03-09 04:43 EST, Ayushman Jain CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.