Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 333939

Summary:

When a statement is replaced or removed using the ASTRewrite class any comment attached to the statement is lost

Product:

[Tools] CDT

Reporter:

Fredrik Berg Kjolstad <fredrikbk>

Component:

cdt-refactoring

Assignee:

Emanuel Graf <emanuel>

Status:

RESOLVED FIXED

QA Contact:

Emanuel Graf <emanuel>

Severity:

normal

Priority:

CC:

cdtdoug, malaperle

Version:

8.0

Target Milestone:

8.0

Hardware:

Macintosh

OS:

Mac OS X - Carbon (unsup.)

Whiteboard:

Bug Depends on:

Bug Blocks:

296192

Attachments:

Description	Flags
Bugfix that fixes the issue where comments attached to a replaced statement were lost	emanuel: iplog+
API for modifying Comments (contains Fredrik's patch 186460)	emanuel: iplog-
API for modifying Comments refined patch (contains Fredrik's patch 186460)	emanuel: iplog-

Description Fredrik Berg Kjolstad

2011-01-10 23:02:57 EST

Build Identifier: M20100909-0800

When a statement is replaced or removed using ASTRewrite class in the refactoring infrastructure any comment attached to the statement is lost.  This include leading, trailing, and freestanding comments.

This is problematic if one for instance have a refactoring that, for example, replaces function calls with calls to another function - maybe a safer or faster version of the same function. The user would not want comments that happen, for arbitrary reasons, to be attached to the replaced statement to be lost.

At the very least it should be possible to control this through the refactoring infrastructure. The Extract Method refactoring in JDT, for instance, only removes the comment before statements to be extracted from the original source code if these are selected when invoking the refactoring.

Reproducible: Always

Steps to Reproduce:
1. Create a statement with a comment above it
2. Invoke ASTRewrite.replace to replace the statement with another statement
3. Observe that the comment was also removed

Comment 1 Fredrik Berg Kjolstad

2011-01-10 23:35:57 EST

Created attachment 186460 [details]
Bugfix that fixes the issue where comments attached to a replaced statement were lost

The patch fixes the issue where comments attached to a statement (leading,trailing,freestanding) are lost when the statement is replaced.

Comment 2 Fredrik Berg Kjolstad

2011-01-10 23:37:19 EST

Comment by Emanuel Graf from original aggregate bug regarding attachment 186460 [details]:

Bugfix patch that fixes a bug where comments attached to a replaced statement are lost

This patch duplicates the comment in some cases. eg extract function

void foo(){
//comment
int i = 0;
}
becomes:
int bar(){
//comment
int i = 0;
return i;
}

void foo(){
//comment
int i = bar();
}

Duplication is better than losing it but still requires the user to change the code manually after each refactoring.

Comment 3 Emanuel Graf

2011-03-08 04:28:14 EST

Created attachment 190635 [details]
API for modifying Comments (contains Fredrik's patch 186460)

Fredrik's patch duplicates some comments. There was no way to add or remove comments using the ASTRewrite infrastructure. I added API to add and remove comments.

Comment 4 Emanuel Graf

2011-03-09 02:09:38 EST

Created attachment 190729 [details]
API for modifying Comments refined patch (contains Fredrik's patch 186460)

Added a missing null check. And adapted some refactoring tests.

Comment 5 Emanuel Graf

2011-03-09 02:46:38 EST

Fixed in HEAD > 20110309

Comment 6 CDT Genie

2011-03-09 03:23:32 EST

*** cdt cvs genie on behalf of egraf ***
Bug 333939: When a statement is replaced or removed using the ASTRewrite class any comment attached to the statement is lost
<a  href=https://bugs.eclipse.org/bugs/show_bug.cgi?id=333939>https://bugs.eclipse.org/bugs/show_bug.cgi?id=333939</a>

[*] ASTWriterVisitor.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriterVisitor.java?root=Tools_Project&r1=1.9&r2=1.10
[*] NodeWriter.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/NodeWriter.java?root=Tools_Project&r1=1.4&r2=1.5
[*] ASTWriter.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriter.java?root=Tools_Project&r1=1.7&r2=1.8

[*] ASTRewrite.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/rewrite/ASTRewrite.java?root=Tools_Project&r1=1.3&r2=1.4

[*] ASTRewriteAnalyzer.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java?root=Tools_Project&r1=1.3&r2=1.4

[*] ModifiedASTStatementWriter.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTStatementWriter.java?root=Tools_Project&r1=1.5&r2=1.6
[*] ModifiedASTDeclSpecWriter.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclSpecWriter.java?root=Tools_Project&r1=1.4&r2=1.5
[*] ModifiedASTDeclarationWriter.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclarationWriter.java?root=Tools_Project&r1=1.4&r2=1.5
[*] ModifiedASTExpressionWriter.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTExpressionWriter.java?root=Tools_Project&r1=1.8&r2=1.9
[*] ModifiedASTDeclaratorWriter.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclaratorWriter.java?root=Tools_Project&r1=1.6&r2=1.7
[*] ASTModificationHelper.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java?root=Tools_Project&r1=1.6&r2=1.7
[*] ChangeGenerator.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGenerator.java?root=Tools_Project&r1=1.19&r2=1.20
[*] ChangeGeneratorWriterVisitor.java 1.11 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java?root=Tools_Project&r1=1.10&r2=1.11

[*] ExtractMethod.rts 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts?root=Tools_Project&r1=1.17&r2=1.18
[*] ExtractMethodHistory.rts 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodHistory.rts?root=Tools_Project&r1=1.3&r2=1.4

[*] ChangeGeneratorTest.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ChangeGeneratorTest.java?root=Tools_Project&r1=1.5&r2=1.6