This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 178441 - Comparing Strings Fix should not treat literals the same as variables
Summary: Comparing Strings Fix should not treat literals the same as variables
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: 6.x
Hardware: All All
: P4 normal with 1 vote (vote)
Assignee: Max Sauer
URL:
Keywords:
Depends on: 158042
Blocks:
  Show dependency tree
 
Reported: 2009-12-10 12:03 UTC by malfunction84
Modified: 2011-06-22 05:27 UTC (History)
2 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:

Attachments
patch (25.67 KB, patch)
2010-03-02 19:21 UTC, malfunction84 		Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description malfunction84 2009-12-10 12:03:04 UTC 
The Fix introduced in bug 158042 treats String literals and String variables the same way.  This can generate some unnecessary code:

// Original code
if (str == "") {
}

// With current ternary Fix applied
if (str == null ? "" == null : str.equals("")) {
}

// With current non-ternary Fix applied
if ((str == null && "" == null) || (str != null && str.equals(""))) {
}

Comparing a String literal to null will always return false, so some of the generated code above is unnecessary.  The following is what should be generated:

// With proposed Fix 1 applied
if (str != null && str.equals("")) {
}

To accommodate different coding styles, an option could be introduced into the Fix to allow it to generate the following code instead:

// With proposed Fix 2 applied
if ("".equals(str)) {
}

I am willing to implement these changes, add new test cases, and attach a patch here.
Comment 1 malfunction84 2009-12-27 12:08:33 UTC 
I have begun work on a patch which I will attach here.
Comment 2 malfunction84 2010-03-02 19:21:49 UTC 
Created attachment 94702 [details]
patch

I've attached a patch which implements the suggested behavior.  The patch includes test cases.
Comment 3 malfunction84 2010-03-02 19:26:14 UTC 
Reassigning to component default assignee.

Please review the attached patch.
Comment 4 malfunction84 2010-07-12 21:02:16 UTC 
Reassigning to original default assignee.

Please review the attached patch.
Comment 5 malfunction84 2010-10-20 19:44:32 UTC 
Can someone please review the attached patch?  It's been sitting around for almost 8 months now.
Comment 6 Jan Lahoda 2010-10-21 11:45:03 UTC 
Sorry, it slipped off my radar. I have applied the patch in my local repository, will be public on my next push (c4318e5f6fe6). Thanks for the patch.
Comment 7 malfunction84 2010-10-21 13:41:31 UTC 
(In reply to comment #6)
> Sorry, it slipped off my radar. I have applied the patch in my local
> repository, will be public on my next push (c4318e5f6fe6). Thanks for the
> patch.

No worries.  I wasn't upset; I just wanted to make a blip on someone's radar.  :)  Thanks, and you're welcome!
Comment 8 malfunction84 2011-06-22 05:27:54 UTC 
This is definitely resolved in 7.0.