Bug 143026 - [ast rewrite] Clean up parantheses are not recognizing comment //
Summary: [ast rewrite] Clean up parantheses are not recognizing comment //
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 minor (vote)
Target Milestone: 3.3 RC1   Edit
Assignee: Martin Aeschlimann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 128422 161862 184703 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-05-22 13:48 EDT by Nils Hammar CLA
Modified: 2007-05-15 05:25 EDT (History)
6 users (show)

See Also:
david_audel: review+
jerome_lanneluc: review+

Attachments
patch (2.81 KB, patch)
2007-05-11 10:47 EDT, Martin Aeschlimann CLA 		no flags Details | Diff
additional test (3.26 KB, patch)
2007-05-11 11:28 EDT, Martin Aeschlimann CLA 		no flags Details | Diff
View All 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 Nils Hammar CLA 2006-05-22 13:48:24 EDT 
If you have a // comment inside an if statement and do a Source/Clean up... and select Always for "Use parentheses around conditions" you will get a slight code corruption:

See example
Before:
------------
        if (a == 1 //
            || b == 2)
        {
            System.out.println("Hello World!");
        }
------------

After:
------------
        if ((a == 1 //)
            || (b == 2))
        {
            System.out.println("Hello World!");
        }
------------

As you can see the parenthesis is located to the right of the comment mark instead of to the left of the comment mark.

An annoying bug, but not very bad.
Comment 1 Olivier Thomann CLA 2006-05-22 21:13:06 EDT 
Move to JDT/UI
Comment 2 Martin Aeschlimann CLA 2006-05-23 03:53:21 EDT 
what build are you using? This should be fixed
Comment 3 Martin Aeschlimann CLA 2006-05-23 03:53:43 EDT 
Sorry, just reproduced in RC4.
Comment 4 Nils Hammar CLA 2006-05-23 12:11:48 EDT 
(In reply to comment #3)
> Sorry, just reproduced in RC4.
> 

And I'm using RC5 for the moment.
Comment 5 Benno Baumgartner CLA 2007-05-02 04:21:59 EDT 
*** Bug 161862 has been marked as a duplicate of this bug. ***
Comment 6 Benno Baumgartner CLA 2007-05-02 04:22:39 EDT 
*** Bug 184703 has been marked as a duplicate of this bug. ***
Comment 7 Benno Baumgartner CLA 2007-05-02 04:24:05 EDT 
Martin, if you want me to fix this I'll do it, but I would need some help from you where to start...
Comment 8 Carl-Eric Menzel CLA 2007-05-02 05:51:25 EDT 
Is there something I can do to help? I haven't yet tried to do anything with Eclipse sources, so I'm not sure.

Just out of curiosity, why is this not considered major or critical? It breaks code, something a code formatter or cleaner IMO should never do. It's a rare occurrence, but I was more than just a bit surprised when my code suddenly turned red.

Again, if there's something I can do to help, please let me know.
Comment 9 Martin Aeschlimann CLA 2007-05-02 08:05:43 EDT 
I'll have a look at it
Comment 10 Martin Aeschlimann CLA 2007-05-11 10:44:38 EDT 
*** Bug 128422 has been marked as a duplicate of this bug. ***
Comment 11 Martin Aeschlimann CLA 2007-05-11 10:47:17 EDT 
Created attachment 66862 [details]
patch
Comment 12 Martin Aeschlimann CLA 2007-05-11 10:50:19 EDT 
Jerome and David, do you approve?
Comment 13 Martin Aeschlimann CLA 2007-05-11 10:55:19 EDT 
The fix avoids the compilation error by adding an line delimiter after the line comment. That means for the given example, the closing parenthesis is added on the new line. This is not elegant, but if we want to change that, we would have to tune the comment mapping heuristic, which is too sensitive at this time.
Currently the comment is mapped to the preceding symbol ('1') what needs to be respected by the copy, remove or insert operations.
Comment 14 Olivier Thomann CLA 2007-05-11 11:01:46 EDT 
(In reply to comment #13)
> The fix avoids the compilation error by adding an line delimiter after the line
> comment. That means for the given example, the closing parenthesis is added on
> the new line. This is not elegant
Maybe. But line comments inside binary expression are ugly anyway. As long as the code compiles, any fix is fine for me.
The cleanup should be to remove these comments :-).
Comment 15 Martin Aeschlimann CLA 2007-05-11 11:28:51 EDT 
Created attachment 66877 [details]
additional test

additional test that shows why 
 b= offset < formatted.length() && 
      !IndentManipulation.isLineDelimiterChar(formatted.charAt(offset))
is necessary
Comment 16 David Audel CLA 2007-05-11 11:38:05 EDT 
The patch do exactly what described in comment 13.
The patch is good for me.
Comment 17 Martin Aeschlimann CLA 2007-05-14 06:46:23 EDT 
patch released > 20070514
Comment 18 Eric Jodet CLA 2007-05-15 05:09:45 EDT 
Verified for 3.3 RC1 using I20070515-0010