Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 318471 - [refactoring][inline] Inline variable removes comments
Summary: [refactoring][inline] Inline variable removes comments
Status: CLOSED DUPLICATE of bug 295200
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Raksha Vasisht CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-30 09:49 EDT by Johannes Rieken CLA
Modified: 2011-01-24 14:27 EST (History)
4 users (show)

See Also:


Attachments
not only the comment(s) in between, but also like this. (32.61 KB, image/png)
2010-08-06 20:40 EDT, Jerry Mising name CLA
no flags Details
Test case for Inline variable with comments (2.17 KB, patch)
2010-08-09 10:55 EDT, Raksha Vasisht CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Johannes Rieken CLA 2010-06-30 09:49:06 EDT
Build Identifier: 3.6m7

have the following CU


public class A {
	
	public static void main(String[] args) {
		
		String b= args[0];
		
		// moin moin
		
		System.out.println(b);
		
	}
}


Perform inline variable of b. The comment and the blank lines a removed.

Reproducible: Always
Comment 1 Deepak Azad CLA 2010-07-02 16:09:28 EDT
Similar to Bug 305103. I quickly tried the patch from Bug 306524 comment 8 and it fixes the problem here.
Comment 2 Jerry Mising name CLA 2010-08-06 20:40:01 EDT
Created attachment 176077 [details]
not only the comment(s) in between, but also like this.
Comment 3 Deepak Azad CLA 2010-08-08 08:27:42 EDT
*** Bug 100143 has been marked as a duplicate of this bug. ***
Comment 4 Raksha Vasisht CLA 2010-08-09 08:56:14 EDT
(In reply to comment #1)
> Similar to Bug 305103. I quickly tried the patch from Bug 306524 comment 8 and
> it fixes the problem here.

yes, the patch fixes the problem mentioned in comment #0, but does only for inline variables and not constants or for bug 100143. Here's a scenario where it does not work:

class A {
	
    //comment1- do not delete
	
    //comment before- delete with declaration
    static final int TEST= 1;
    //comment after- delete with declaration
    
    //comment2 - do not delete
    
    boolean b= equals(TEST);
}

The first time I apply the refactoring it works as expected, deleting the comments belonging to the extended range of the declaration node, whereas after that it only deletes the comments before the declaration and not the ones after :

class A {
	
    //comment after- delete with declaration
    
    //comment2 - do not delete
    
    boolean b= equals(1);
}

I think we should separate the problems instead of putting it all in one bug. So I suggest :
- File a bug for the above example
- Mark this resolved for inline variables attaching a test case
- Reopening bug 100143 since that is for inline methods and is still not fixed
Comment 5 Raksha Vasisht CLA 2010-08-09 10:55:13 EDT
Created attachment 176160 [details]
Test case for Inline variable with comments

(In reply to comment #4)

> I think we should separate the problems instead of putting it all in one bug.
> So I suggest :
> - File a bug for the above example
Done, see bug 322310
> - Mark this resolved for inline variables attaching a test case
Attached.

> - Reopening bug 100143 since that is for inline methods and is still not fixed
Done.
Comment 6 Raksha Vasisht CLA 2010-08-09 11:02:13 EDT
(In reply to comment #2)
> Created an attachment (id=176077) [details] [diff]
> not only the comment(s) in between, but also like this.

Comer, here the comment is supposed to get deleted since it belongs to the extended source range of the declaration node. So the behaviour is okay.

(In reply to comment #5)
> Created an attachment (id=176160) [details] [diff]
> Test case for Inline variable with comments

Markus, if this is okay Ill commit it to HEAD.
Comment 7 John Arthorne CLA 2011-01-24 13:46:35 EST
Looks like a duplicate of bug 295200?
Comment 8 Markus Keller CLA 2011-01-24 14:27:22 EST
(In reply to comment #5)
> > - File a bug for the above example (comment 4)
> Done, see bug 322310

That's not the right bug number. Furthermore, I wouldn't change this for inline constant, since the situation is quite different from inline local variable.


(In reply to comment #6)
> > Created attachment 176160 [details]
> > Test case for Inline variable with comments
> 
> Markus, if this is okay Ill commit it to HEAD.

Nope, but you can use the 'in' part for bug 295200.

(In reply to comment #7)
> Looks like a duplicate of bug 295200?
Yes.

*** This bug has been marked as a duplicate of bug 295200 ***