From 953c613eb9191e9a6384be9061336e3dab812e56 Mon Sep 17 00:00:00 2001 From: Emanuel Graf Date: Wed, 9 Mar 2011 07:46:05 +0000 Subject: [PATCH] Bug 333939: When a statement is replaced or removed using the ASTRewrite class any comment attached to the statement is lost https://bugs.eclipse.org/bugs/show_bug.cgi?id=333939 --- .../changegenerator/ChangeGeneratorTest.java | 17 ++- .../cdt/core/dom/rewrite/ASTRewrite.java | 137 ++++++++++++++---- .../core/dom/rewrite/ASTRewriteAnalyzer.java | 7 +- .../core/dom/rewrite/astwriter/ASTWriter.java | 6 +- .../rewrite/astwriter/ASTWriterVisitor.java | 20 ++- .../dom/rewrite/astwriter/NodeWriter.java | 34 ++++- .../ASTModificationHelper.java | 33 ++++- .../changegenerator/ChangeGenerator.java | 5 +- .../ChangeGeneratorWriterVisitor.java | 2 +- .../ModifiedASTDeclSpecWriter.java | 4 +- .../ModifiedASTDeclarationWriter.java | 6 +- .../ModifiedASTDeclaratorWriter.java | 17 ++- .../ModifiedASTExpressionWriter.java | 4 +- .../ModifiedASTStatementWriter.java | 5 +- .../resources/refactoring/ExtractMethod.rts | 10 +- .../refactoring/ExtractMethodHistory.rts | 4 +- 16 files changed, 240 insertions(+), 71 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ChangeGeneratorTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ChangeGeneratorTest.java index 29b85433663..3e38fdb3f41 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ChangeGeneratorTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ChangeGeneratorTest.java @@ -20,6 +20,7 @@ import org.eclipse.cdt.core.parser.tests.rewrite.TestHelper; import org.eclipse.cdt.core.tests.BaseTestFramework; import org.eclipse.cdt.internal.core.dom.rewrite.ASTModificationStore; import org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator; +import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.ASTCommenter; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IncrementalProjectBuilder; import org.eclipse.core.resources.ResourcesPlugin; @@ -46,24 +47,26 @@ public abstract class ChangeGeneratorTest extends BaseTestFramework { @Override public void runTest() throws Exception{ final ASTModificationStore modStore = new ASTModificationStore(); - final ChangeGenerator changegenartor = new ChangeGenerator(modStore); - IFile testFile = importFile("source.h", source); //$NON-NLS-1$ - + IFile testFile = importFile("source.h", source); //$NON-NLS-1$ + ASTVisitor visitor = createModificator(modStore); - + CCorePlugin.getIndexManager().reindex(cproject); ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor()); boolean joined = CCorePlugin.getIndexManager().joinIndexer(20000, new NullProgressMonitor()); assertTrue("The indexing operation of the test CProject has not finished jet. This should not happen...", joined); - + IASTTranslationUnit unit = CoreModelUtil.findTranslationUnit(testFile).getAST(); + final ChangeGenerator changegenartor = new ChangeGenerator(modStore, + ASTCommenter.getCommentedNodeMap(unit)); unit.accept(visitor); - + changegenartor.generateChange(unit); Document doc = new Document(source); - for (Change curChange : ((CompositeChange)changegenartor.getChange()).getChildren()){ + for (Change curChange : ((CompositeChange) changegenartor.getChange()) + .getChildren()) { if (curChange instanceof TextFileChange) { TextFileChange textChange = (TextFileChange) curChange; textChange.getEdit().apply(doc); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/rewrite/ASTRewrite.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/rewrite/ASTRewrite.java index 7e20633a449..d4e6affabbf 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/rewrite/ASTRewrite.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/rewrite/ASTRewrite.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2007, 2009 Wind River Systems, Inc. and others. + * Copyright (c) 2007, 2011 Wind River Systems, Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -10,6 +10,8 @@ *******************************************************************************/ package org.eclipse.cdt.core.dom.rewrite; +import java.util.List; + import org.eclipse.cdt.core.dom.ast.IASTComment; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTPreprocessorStatement; @@ -17,49 +19,80 @@ import org.eclipse.cdt.core.dom.ast.IASTProblem; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.internal.core.dom.rewrite.ASTLiteralNode; import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification; +import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification.ModificationKind; import org.eclipse.cdt.internal.core.dom.rewrite.ASTModificationStore; import org.eclipse.cdt.internal.core.dom.rewrite.ASTRewriteAnalyzer; -import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification.ModificationKind; +import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.ASTCommenter; +import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.text.edits.TextEditGroup; /** - * Infrastructure for modifying code by describing changes to AST nodes. - * The AST rewriter collects descriptions of modifications to nodes and - * translates these descriptions into text edits that can then be applied to - * the original source. This is all done without actually modifying the - * original AST. The rewrite infrastructure tries to generate minimal - * text changes, preserve existing comments and indentation, and follow code - * formatter settings. + * Infrastructure for modifying code by describing changes to AST nodes. The AST rewriter collects + * descriptions of modifications to nodes and translates these descriptions into text edits that can then be + * applied to the original source. This is all done without actually modifying the original AST. The rewrite + * infrastructure tries to generate minimal text changes, preserve existing comments and indentation, and + * follow code formatter settings. A {@link IASTComment} can be removed from or added to a node. *

- * The initial implementation does not support nodes that implement - * {@link IASTPreprocessorStatement}, {@link IASTComment} or {@link IASTProblem}. + * The initial implementation does not support nodes that implement {@link IASTPreprocessorStatement} or + * {@link IASTProblem}. *

- * EXPERIMENTAL. This class or interface has been added as - * part of a work in progress. There is no guarantee that this API will - * work or that it will remain the same. Please do not use this API without + * EXPERIMENTAL. This class or interface has been added as part of a work in progress. There + * is no guarantee that this API will work or that it will remain the same. Please do not use this API without * consulting with the CDT team. *

+ * * @since 5.0 * @noinstantiate This class is not intended to be instantiated by clients. */ public final class ASTRewrite { + /** + * Defines the positions of the comment. + * + * @since 5.3 + */ + public enum CommentPosition{ + + /** + * Comments before a statement, declaration, or definition + */ + leading, + /** + * Comments right after the AST node on the same line + */ + trailing, + /** + * Comments before a closing brace such as they occur in namespace-, class- and method-definitions or + * at the end of a file + */ + freestanding + } + /** * Creates a rewriter for a translation unit. */ public static ASTRewrite create(IASTTranslationUnit node) { - return new ASTRewrite(node, new ASTModificationStore(), null); + NodeCommentMap commentMap = ASTCommenter.getCommentedNodeMap(node); + return new ASTRewrite(node, new ASTModificationStore(), null, commentMap); } private final IASTNode fRoot; private final ASTModificationStore fModificationStore; private final ASTModification fParentMod; + private final NodeCommentMap fCommentMap; + + private enum Operation{ + insertBefore, + replace, + remove + } - private ASTRewrite(IASTNode root, ASTModificationStore modStore, ASTModification parentMod) { + private ASTRewrite(IASTNode root, ASTModificationStore modStore, ASTModification parentMod, NodeCommentMap commentMap) { fRoot= root; fModificationStore= modStore; fParentMod= parentMod; + fCommentMap = commentMap; } /** @@ -88,7 +121,7 @@ public final class ASTRewrite { */ public final void remove(IASTNode node, TextEditGroup editGroup) { checkBelongsToAST(node); - checkSupportedNode(node); + checkSupportedNode(node, Operation.remove); ASTModification mod= new ASTModification(ModificationKind.REPLACE, node, null, editGroup); fModificationStore.storeModification(fParentMod, mod); } @@ -112,11 +145,11 @@ public final class ASTRewrite { throw new IllegalArgumentException(); } checkBelongsToAST(node); - checkSupportedNode(node); - checkSupportedNode(replacement); + checkSupportedNode(node, Operation.replace); + checkSupportedNode(replacement, Operation.replace); ASTModification mod= new ASTModification(ModificationKind.REPLACE, node, replacement, editGroup); fModificationStore.storeModification(fParentMod, mod); - return new ASTRewrite(replacement, fModificationStore, mod); + return new ASTRewrite(replacement, fModificationStore, mod, fCommentMap); } /** @@ -140,9 +173,9 @@ public final class ASTRewrite { if (newNode == null) { throw new IllegalArgumentException(); } - checkSupportedNode(parent); - checkSupportedNode(insertionPoint); - checkSupportedNode(newNode); + checkSupportedNode(parent, Operation.insertBefore); + checkSupportedNode(insertionPoint, Operation.insertBefore); + checkSupportedNode(newNode, Operation.insertBefore); ASTModification mod; if (insertionPoint == null) { @@ -155,7 +188,7 @@ public final class ASTRewrite { mod= new ASTModification(ModificationKind.INSERT_BEFORE, insertionPoint, newNode, editGroup); } fModificationStore.storeModification(fParentMod, mod); - return new ASTRewrite(newNode, fModificationStore, mod); + return new ASTRewrite(newNode, fModificationStore, mod, fCommentMap); } /** @@ -175,7 +208,7 @@ public final class ASTRewrite { if (!(fRoot instanceof IASTTranslationUnit)) { throw new IllegalArgumentException("This API can only be used for the root rewrite object."); //$NON-NLS-1$ } - return ASTRewriteAnalyzer.rewriteAST((IASTTranslationUnit) fRoot, fModificationStore); + return ASTRewriteAnalyzer.rewriteAST((IASTTranslationUnit) fRoot, fModificationStore, fCommentMap); } private void checkBelongsToAST(IASTNode node) { @@ -188,15 +221,63 @@ public final class ASTRewrite { throw new IllegalArgumentException(); } - private void checkSupportedNode(IASTNode node) { + private void checkSupportedNode(IASTNode node, Operation op) { if (node instanceof IASTComment) { - throw new IllegalArgumentException("Rewriting comments is not yet supported"); //$NON-NLS-1$ + if(op != Operation.remove) { + throw new IllegalArgumentException("Rewriting comments is not yet supported"); //$NON-NLS-1$ + } } if (node instanceof IASTPreprocessorStatement) { throw new IllegalArgumentException("Rewriting preprocessor statements is not yet supported"); //$NON-NLS-1$ } if (node instanceof IASTProblem) { - throw new IllegalArgumentException("Rewriting problem nodes is supported"); //$NON-NLS-1$ + throw new IllegalArgumentException("Rewriting problem nodes is not supported"); //$NON-NLS-1$ } } + + + /** + * Assigns the comment to the node. + * + * @param node + * @param comment + * @param pos + * @since 5.3 + */ + public void addComment(IASTNode node, IASTComment comment, CommentPosition pos) { + switch (pos) { + case leading: + fCommentMap.addLeadingCommentToNode(node, comment); + break; + case trailing: + fCommentMap.addTrailingCommentToNode(node, comment); + break; + case freestanding: + fCommentMap.addFreestandingCommentToNode(node, comment); + break; + } + } + + + /** + * + * @param node + * the node + * @param pos + * the position + * @return All comments assigned to the node at this position + * @since 5.3 + */ + public List getComments(IASTNode node, CommentPosition pos) { + switch (pos) { + case leading: + return fCommentMap.getLeadingCommentsForNode(node); + case trailing: + return fCommentMap.getTrailingCommentsForNode(node); + case freestanding: + return fCommentMap.getFreestandingCommentsForNode(node); + + } + return fCommentMap.getLeadingCommentsForNode(node); + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java index 2b273d457ee..106d7934251 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Wind River Systems, Inc. and others. + * Copyright (c) 2008, 2011 Wind River Systems, Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -12,6 +12,7 @@ package org.eclipse.cdt.internal.core.dom.rewrite; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator; +import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap; import org.eclipse.core.resources.IFile; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.TextFileChange; @@ -19,8 +20,8 @@ import org.eclipse.ltk.core.refactoring.TextFileChange; public class ASTRewriteAnalyzer { private static ICTextFileChangeFactory sFileChangeFactory; - public static Change rewriteAST(IASTTranslationUnit root, ASTModificationStore modificationStore) { - ChangeGenerator rewriter = new ChangeGenerator(modificationStore); + public static Change rewriteAST(IASTTranslationUnit root, ASTModificationStore modificationStore, NodeCommentMap commentMap) { + ChangeGenerator rewriter = new ChangeGenerator(modificationStore, commentMap); rewriter.generateChange(root); return rewriter.getChange(); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriter.java index f88757b8dd9..b77385bf8a8 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -64,6 +64,10 @@ public class ASTWriter { return write(rootNode, null, new NodeCommentMap()); } + public String write(IASTNode rootNode, NodeCommentMap commentMap) { + return write(rootNode, null, commentMap); + } + /** * * Generates the source code representing this node including comments. diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriterVisitor.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriterVisitor.java index afea96e7751..7cd9bf10821 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriterVisitor.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/ASTWriterVisitor.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -12,10 +12,13 @@ *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.rewrite.astwriter; +import java.util.ArrayList; + import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.IASTArrayModifier; import org.eclipse.cdt.core.dom.ast.IASTComment; import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; +import org.eclipse.cdt.core.dom.ast.IASTCopyLocation; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTDeclaration; import org.eclipse.cdt.core.dom.ast.IASTDeclarator; @@ -23,6 +26,7 @@ import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTInitializer; import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTNodeLocation; import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration; import org.eclipse.cdt.core.dom.ast.IASTStatement; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; @@ -110,11 +114,23 @@ public class ASTWriterVisitor extends ASTVisitor { } private void writeLeadingComments(IASTNode node) { - for(IASTComment comment : commentMap.getLeadingCommentsForNode(node)) { + for(IASTComment comment : getLeadingComments(node)) { scribe.print(comment.getComment()); scribe.newLine(); } } + + + + private ArrayList getLeadingComments(IASTNode node) { + ArrayList leadingComments = commentMap.getLeadingCommentsForNode(node); + IASTNodeLocation[] locs = node.getNodeLocations(); + if (locs != null && locs.length > 0 && locs[0] instanceof IASTCopyLocation) { + IASTCopyLocation copyLoc = (IASTCopyLocation) locs[0]; + leadingComments.addAll(commentMap.getLeadingCommentsForNode(copyLoc.getOriginalNode())); + } + return leadingComments; + } public void visit(ASTLiteralNode lit) { scribe.print(lit.getRawSignature()); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/NodeWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/NodeWriter.java index 700165b1bd9..61a7f241430 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/NodeWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/NodeWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2010 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -11,9 +11,13 @@ *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.rewrite.astwriter; +import java.util.ArrayList; + import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.IASTComment; +import org.eclipse.cdt.core.dom.ast.IASTCopyLocation; import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTNodeLocation; import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap; /** @@ -89,14 +93,24 @@ public class NodeWriter { } protected boolean hasTrailingComments(IASTNode node){ - if(commentMap.getTrailingCommentsForNode(node).size()>0) { + if(getTrailingComments(node).size()>0) { return true; } return false; } + + private ArrayList getTrailingComments(IASTNode node) { + ArrayList trailingComments = commentMap.getTrailingCommentsForNode(node); + IASTNodeLocation[] locs = node.getNodeLocations(); + if (locs != null && locs.length > 0 && locs[0] instanceof IASTCopyLocation) { + IASTCopyLocation loc = (IASTCopyLocation) locs[0]; + trailingComments.addAll(commentMap.getTrailingCommentsForNode(loc.getOriginalNode())); + } + return trailingComments; + } protected void writeTrailingComments(IASTNode node, boolean newLine) { - for(IASTComment comment : commentMap.getTrailingCommentsForNode(node)) { + for(IASTComment comment : getTrailingComments(node)) { scribe.printSpace(); scribe.print(comment.getComment()); if(newLine) { @@ -106,14 +120,24 @@ public class NodeWriter { } protected boolean hasFreestandingComments(IASTNode node){ - if(commentMap.getFreestandingCommentsForNode(node).size()>0) { + if(getFreestandingComments(node).size()>0) { return true; } return false; } + private ArrayList getFreestandingComments(IASTNode node) { + ArrayList freestandingComments = commentMap.getFreestandingCommentsForNode(node); + IASTNodeLocation[] locs = node.getNodeLocations(); + if (locs != null && locs.length > 0 && locs[0] instanceof IASTCopyLocation) { + IASTCopyLocation loc = (IASTCopyLocation) locs[0]; + freestandingComments.addAll(commentMap.getFreestandingCommentsForNode(loc.getOriginalNode())); + } + return freestandingComments; + } + protected void writeFreeStandingComments(IASTNode node) { - for(IASTComment comment : commentMap.getFreestandingCommentsForNode(node)) { + for(IASTComment comment : getFreestandingComments(node)) { scribe.print(comment.getComment()); scribe.newLine(); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java index 8879b1442e4..186f399fb41 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java @@ -15,14 +15,17 @@ import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import org.eclipse.cdt.core.dom.ast.IASTComment; import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTInitializer; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification; import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification.ModificationKind; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ContainerNode; +import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap; public class ASTModificationHelper { @@ -33,7 +36,7 @@ public class ASTModificationHelper { } - public T[] createModifiedChildArray(IASTNode parent, T[] unmodifiedChildren, Class clazz){ + public T[] createModifiedChildArray(IASTNode parent, T[] unmodifiedChildren, Class clazz, NodeCommentMap commentMap){ ArrayList modifiedChildren = new ArrayList(Arrays.asList(unmodifiedChildren)); for(ASTModification parentModification : modificationsForNode(parent)){ @@ -78,6 +81,7 @@ public class ASTModificationHelper { switch (childModification.getKind()) { case REPLACE: if (newNode != null) { + copyComments(newNode, currentChild, commentMap); modifiedChildren.add( modifiedChildren.indexOf(childModification.getTargetNode()), newNode); @@ -96,6 +100,33 @@ public class ASTModificationHelper { return modifiedChildren.toArray(newArrayInstance(clazz, modifiedChildren.size())); } + private void copyComments(IASTNode newNode, IASTNode oldNode, NodeCommentMap commentMap) { + // Attach all the comments that is attached to oldNode to newNode + ArrayList leadingComments = commentMap.getLeadingCommentsForNode(oldNode); + for (IASTComment comment : leadingComments) { + commentMap.addLeadingCommentToNode(newNode, comment); + } + + ArrayList trailingComments = commentMap.getTrailingCommentsForNode(oldNode); + for (IASTComment comment : trailingComments) { + commentMap.addTrailingCommentToNode(newNode, comment); + } + + ArrayList freestandingComments = commentMap.getFreestandingCommentsForNode(oldNode); + for (IASTComment comment : freestandingComments) { + commentMap.addFreestandingCommentToNode(newNode, comment); + } + + // Detach comments from oldNode (to avoid memory leak) + HashMap> leadingMap = commentMap.getLeadingMap(); + leadingMap.remove(oldNode); + + HashMap> trailingMap = commentMap.getTrailingMap(); + trailingMap.remove(oldNode); + + HashMap> freestandingMap = commentMap.getFreestandingMap(); + freestandingMap.remove(oldNode); + } private void insertNode(Class clazz, ArrayList modifiedChildren, ASTModification parentModification, IASTNode newNode) { diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGenerator.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGenerator.java index 0a63a9d14c3..ece73b87906 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGenerator.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGenerator.java @@ -37,7 +37,6 @@ import org.eclipse.cdt.internal.core.dom.rewrite.ASTModificationStore; import org.eclipse.cdt.internal.core.dom.rewrite.ASTRewriteAnalyzer; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriter; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ProblemRuntimeException; -import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.ASTCommenter; import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap; import org.eclipse.cdt.internal.core.dom.rewrite.util.FileContentHelper; import org.eclipse.cdt.internal.core.dom.rewrite.util.FileHelper; @@ -95,8 +94,9 @@ public class ChangeGenerator extends ASTVisitor { } - public ChangeGenerator(ASTModificationStore modificationStore) { + public ChangeGenerator(ASTModificationStore modificationStore, NodeCommentMap commentMap) { this.modificationStore = modificationStore; + this.commentMap = commentMap; } @@ -108,7 +108,6 @@ public class ChangeGenerator extends ASTVisitor { throws ProblemRuntimeException { change = new CompositeChange(Messages.ChangeGenerator_compositeChange); initParentModList(); - commentMap = ASTCommenter.getCommentedNodeMap(rootNode.getTranslationUnit()); rootNode.accept(pathProvider); for (IFile currentFile : changes.keySet()) { diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java index b291dbf3475..fbd76ceaa0f 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java @@ -332,7 +332,7 @@ public class ChangeGeneratorWriterVisitor extends ASTWriterVisitor { ASTModificationHelper helper = new ASTModificationHelper( stack); - IASTDeclaration[] declarations = helper.createModifiedChildArray(tu, tu.getDeclarations(), IASTDeclaration.class); + IASTDeclaration[] declarations = helper.createModifiedChildArray(tu, tu.getDeclarations(), IASTDeclaration.class, commentMap); for (IASTDeclaration currentDeclaration : declarations) { currentDeclaration.accept(this); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclSpecWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclSpecWriter.java index c5ad63b0352..e6de9cb3271 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclSpecWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclSpecWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -30,6 +30,6 @@ public class ModifiedASTDeclSpecWriter extends DeclSpecWriter { @Override protected IASTDeclaration[] getMembers(IASTCompositeTypeSpecifier compDeclSpec) { return modificationHelper.createModifiedChildArray(compDeclSpec, compDeclSpec.getMembers(), - IASTDeclaration.class); + IASTDeclaration.class, commentMap); } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclarationWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclarationWriter.java index 5c8b9306c1f..59e3953fad8 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclarationWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclarationWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -31,14 +31,14 @@ public class ModifiedASTDeclarationWriter extends DeclarationWriter { @Override protected void writeDeclarationsInNamespace(ICPPASTNamespaceDefinition namespaceDefinition, IASTDeclaration[] declarations) { - IASTDeclaration[] modifiedDeclarations = modificationHelper.createModifiedChildArray(namespaceDefinition, declarations, IASTDeclaration.class); + IASTDeclaration[] modifiedDeclarations = modificationHelper.createModifiedChildArray(namespaceDefinition, declarations, IASTDeclaration.class, commentMap); super.writeDeclarationsInNamespace(namespaceDefinition, modifiedDeclarations); } @Override protected void writeCtorChainInitializer(ICPPASTFunctionDefinition funcDec, ICPPASTConstructorChainInitializer[] ctorInitChain) { - ICPPASTConstructorChainInitializer[] modifiedChainInitializer = modificationHelper.createModifiedChildArray(funcDec, ctorInitChain, ICPPASTConstructorChainInitializer.class); + ICPPASTConstructorChainInitializer[] modifiedChainInitializer = modificationHelper.createModifiedChildArray(funcDec, ctorInitChain, ICPPASTConstructorChainInitializer.class, commentMap); super.writeCtorChainInitializer(funcDec, modifiedChainInitializer); } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclaratorWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclaratorWriter.java index 5914fd1359d..cb764ab7edb 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclaratorWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTDeclaratorWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -40,26 +40,29 @@ public class ModifiedASTDeclaratorWriter extends DeclaratorWriter { @Override protected void writeParameterDeclarations(IASTStandardFunctionDeclarator funcDec, IASTParameterDeclaration[] paraDecls) { - IASTParameterDeclaration[] modifiedParameters = modificationHelper.createModifiedChildArray(funcDec, paraDecls, IASTParameterDeclaration.class); + IASTParameterDeclaration[] modifiedParameters = modificationHelper + .createModifiedChildArray(funcDec, paraDecls, IASTParameterDeclaration.class, + commentMap); super.writeParameterDeclarations(funcDec, modifiedParameters); } @Override protected void writePointerOperators(IASTDeclarator declarator,IASTPointerOperator[] unmodifiedPointerOperations) { - IASTPointerOperator[] modifiedPointer = modificationHelper.createModifiedChildArray(declarator, unmodifiedPointerOperations, IASTPointerOperator.class); + IASTPointerOperator[] modifiedPointer = modificationHelper.createModifiedChildArray( + declarator, unmodifiedPointerOperations, IASTPointerOperator.class, commentMap); super.writePointerOperators(declarator, modifiedPointer); } @Override protected void writeArrayModifiers(IASTArrayDeclarator arrDecl, IASTArrayModifier[] arrMods) { - IASTArrayModifier[] modifiedModifiers = modificationHelper.createModifiedChildArray(arrDecl, arrMods, IASTArrayModifier.class); + IASTArrayModifier[] modifiedModifiers = modificationHelper.createModifiedChildArray(arrDecl, arrMods, IASTArrayModifier.class, commentMap); super.writeArrayModifiers(arrDecl, modifiedModifiers); } @Override protected void writeExceptionSpecification(ICPPASTFunctionDeclarator funcDec, IASTTypeId[] exceptions ) { - IASTTypeId[] modifiedExceptions = modificationHelper.createModifiedChildArray(funcDec, exceptions, IASTTypeId.class); + IASTTypeId[] modifiedExceptions = modificationHelper.createModifiedChildArray(funcDec, exceptions, IASTTypeId.class, commentMap); // it makes a difference whether the exception array is identical to // ICPPASTFunctionDeclarator.NO_EXCEPTION_SPECIFICATION if (modifiedExceptions.length == 0 && @@ -74,7 +77,7 @@ public class ModifiedASTDeclaratorWriter extends DeclaratorWriter { protected void writeKnRParameterDeclarations( ICASTKnRFunctionDeclarator knrFunct, IASTDeclaration[] knrDeclarations) { - IASTDeclaration[] modifiedDeclarations = modificationHelper.createModifiedChildArray(knrFunct, knrDeclarations, IASTDeclaration.class); + IASTDeclaration[] modifiedDeclarations = modificationHelper.createModifiedChildArray(knrFunct, knrDeclarations, IASTDeclaration.class, commentMap); super.writeKnRParameterDeclarations(knrFunct, modifiedDeclarations); } @@ -82,7 +85,7 @@ public class ModifiedASTDeclaratorWriter extends DeclaratorWriter { @Override protected void writeKnRParameterNames( ICASTKnRFunctionDeclarator knrFunct, IASTName[] parameterNames) { - IASTName[] modifiedNames = modificationHelper.createModifiedChildArray(knrFunct, parameterNames, IASTName.class); + IASTName[] modifiedNames = modificationHelper.createModifiedChildArray(knrFunct, parameterNames, IASTName.class, commentMap); super.writeKnRParameterNames(knrFunct, modifiedNames); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTExpressionWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTExpressionWriter.java index 7d33c7592fb..08262e57d24 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTExpressionWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTExpressionWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2010 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -37,7 +37,7 @@ public class ModifiedASTExpressionWriter extends ExpressionWriter { @Override protected void writeExpressions(IASTExpressionList expList, IASTExpression[] expressions) { IASTExpression[] modifiedExpressions = modificationHelper.createModifiedChildArray(expList, - expressions, IASTExpression.class); + expressions, IASTExpression.class, commentMap); super.writeExpressions(expList, modifiedExpressions); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTStatementWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTStatementWriter.java index f90fe0bb590..0f5f0491941 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTStatementWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModifiedASTStatementWriter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik * Rapperswil, University of applied sciences and others * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -41,6 +41,7 @@ public class ModifiedASTStatementWriter extends StatementWriter { @Override protected IASTStatement[] getNestedStatements(IASTCompoundStatement compoundStatement) { - return modificationHelper.createModifiedChildArray(compoundStatement, compoundStatement.getStatements(), IASTStatement.class); + return modificationHelper.createModifiedChildArray(compoundStatement, + compoundStatement.getStatements(), IASTStatement.class, commentMap); } } diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts index b0d7a551ca7..8f91ea478c7 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts @@ -87,7 +87,7 @@ int A::help() return 42; } -//!ExtractFunctionRefactoringTest +//!ExtractFunctionRefactoringTest with comment //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -168,6 +168,7 @@ void A::exp(int & i) int A::foo() { int i = 2; + //comment exp(i); return i; } @@ -205,6 +206,7 @@ void exp(int & i) int main(){ int i; + // Comment exp(i); return i; } @@ -235,7 +237,7 @@ void exp(int & i) int main(){ int i; - exp(i); + exp(i); // Comment return i; } @@ -2703,7 +2705,8 @@ int Test::exp() } int Test::calculateStuff() { - int result = exp(); + // refactor these lines to a new method: + int result = exp(); return result; } @@ -2860,6 +2863,7 @@ void A::exp(int & i) int A::foo() { int i = 2; + //comment exp(i); return i; } diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodHistory.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodHistory.rts index 0077bd9134d..3196f88be81 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodHistory.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodHistory.rts @@ -178,6 +178,7 @@ void A::exp(int & i) int A::foo() { int i = 2; + //comment exp(i); return i; } @@ -217,6 +218,7 @@ void exp(int & i) int main(){ int i; + // Comment exp(i); return i; } @@ -249,7 +251,7 @@ void exp(int & i) int main(){ int i; - exp(i); + exp(i); // Comment return i; }