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 230832 - Convert to switch over Strings does not preserve all the original comments
Summary: Convert to switch over Strings does not preserve all the original comments
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: 7.3
Hardware: PC Windows 7
: P3 normal (vote)
Assignee: Svata Dedic
URL:
Keywords:
Depends on: 239487
Blocks:
  Show dependency tree
 
Reported: 2013-06-06 08:53 UTC by Jachym_Vojtek
Modified: 2014-07-10 14:28 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:

Attachments
Simple example (37.77 KB, image/png)
2013-07-15 17:08 UTC, markiewb 		Details
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 Jachym_Vojtek 2013-06-06 08:53:39 UTC 
When I apply convert to switch on code with comments, not all the comments are preserved in resulting switch.
Original:
public void foo(String someName) {
  if /* 0 */ ( /* 1 */ /* 2 */someName /* 3 */ /* 4 */ == /* 5 */ /* 6 */ "AB" /* 7 */ /* 8 */) /* 9 */ {   // 10
    System.out.println("This is AB");
  } /* 11 */ else /* 12 */ /* 13 */ if /* 14 */ ( /* 15 */"BB"/* 16 */. /* 17 */equals(/* 18 */someName /* 19 */) /* 20 */) /* 21 */ {
    System.out.println("This is BB");
  } /* 22 */ else /* 23 */ {
    System.out.println("Not AB, BB");
  }
}

Converted result:
    public void foo(String someName) {
        switch (someName /* 3 */ /* 4 */) {
            case "AB" /* 7 */ /* 8 */:
                // 10
                System.out.println("This is AB");
                break;
            case "BB"/* 16 */:
                System.out.println("This is BB");
                break;
            default:
                System.out.println("Not AB, BB");
                break;
       }
    }

I admit that this is really ugly code I enclose. But I would appreciate at least preserved comments: 0, 11, 14, 22.
Comment 1 markiewb 2013-07-15 17:08:22 UTC 
Created attachment 137141 [details]
Simple example

Issue still valid in 7.4. Comments get lost. See screenshot
Comment 2 Svata Dedic 2013-11-25 15:53:08 UTC 
Folks, I would prefer not to fix the issue for the patch. Either the if-to-string implementation would become complicated a lot, correcting issues from the lower layers, or I would need to alter code that assigns comments to Tree instances. 

The later is actually preferred, as it may help preserving comments in other cases where a chained if-else-if is being refactored, but I fear that the amount of necessary testing would be higher than we can afford for the patch release.

For example comments 12, 13, 14 from Jachym's great example are assigned to the 1st if while they semantically belong to the else statement. 

Agreed ?
Comment 3 Svata Dedic 2014-07-10 13:51:06 UTC 
As issue #239487 is going to be fixed, this issue will be ALMOST fixed as well. The main (?) noticeable defect remaining from this issue's testcase is duplication of comment #3 after the refactoring. I do not consider that as harmful - no data loss, and the comment is only duplicated because (in this specific case) it is not attached to the correct Tree node. Regular cases with comments outside expressions are OK.
Comment 4 Svata Dedic 2014-07-10 14:28:13 UTC 
Fixed by general support implemented as part of issue #239487 and isssue #245348