From f44c51e7d3da255ac9628890d10a749600e14413 Mon Sep 17 00:00:00 2001 From: Markus Schorn Date: Fri, 23 May 2008 14:26:46 +0000 Subject: [PATCH] Fixes comment handling with preprocessor nodes by Emanuel Graf, bug 233438. --- .../comenthandler/CommentHandlingTest.java | 24 +++--- .../rewrite/CommentHandlingTestSource.rts | 44 ++++++++++ .../changegenerator/ChangeGenerator.java | 55 +++++++++++-- .../rewrite/commenthandler/ASTCommenter.java | 10 ++- .../commenthandler/NodeCommentMap.java | 15 ++++ .../resources/refactoring/ExtractConstant.rts | 81 +++++++++++++++++++ .../resources/refactoring/ExtractMethod.rts | 3 +- 7 files changed, 208 insertions(+), 24 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/comenthandler/CommentHandlingTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/comenthandler/CommentHandlingTest.java index b84f8c6e587..77c49872189 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/comenthandler/CommentHandlingTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/comenthandler/CommentHandlingTest.java @@ -87,19 +87,19 @@ public class CommentHandlingTest extends RewriteBaseTest { @Override protected void runTest() throws Throwable { - if (fileMap.size() > 1) { - fail("To many files for CommentHandlingTest"); //$NON-NLS-1$ - } else if (fileMap.size() == 0) { + if (fileMap.size() == 0) { fail("No file for testing"); //$NON-NLS-1$ } - - TestSourceFile file = fileMap.values().iterator().next(); - NodeCommentMap nodeMap = ASTCommenter.getCommentedNodeMap(getUnit()); - StringBuilder expectedResultBuilder = buildExpectedResult(file); - StringBuilder actualResultBuilder = buildActualResult(nodeMap); - - assertEquals(expectedResultBuilder.toString(), actualResultBuilder.toString()); + for(String fileName : fileMap.keySet()) { + TestSourceFile file = fileMap.get(fileName); + NodeCommentMap nodeMap = ASTCommenter.getCommentedNodeMap(getUnit(fileName)); + + StringBuilder expectedResultBuilder = buildExpectedResult(file); + StringBuilder actualResultBuilder = buildActualResult(nodeMap); + + assertEquals(expectedResultBuilder.toString(), actualResultBuilder.toString()); + } } private StringBuilder buildExpectedResult(TestSourceFile file) { @@ -160,8 +160,8 @@ public class CommentHandlingTest extends RewriteBaseTest { return LEADING_COMMENT_SEPARATOR + ANY_CHAR_REGEXP + TRAILING_COMMENT_SEPARATOR + ANY_CHAR_REGEXP + FREESTANDING_COMMENT_SEPARATOR + ANY_CHAR_REGEXP; } - private IASTTranslationUnit getUnit() throws CoreException { - ITranslationUnit tu = (ITranslationUnit) CCorePlugin.getDefault().getCoreModel().create(project.getFile(fileMap.keySet().iterator().next())); + private IASTTranslationUnit getUnit(String fileName) throws CoreException { + ITranslationUnit tu = (ITranslationUnit) CCorePlugin.getDefault().getCoreModel().create(project.getFile(fileName)); return tu.getAST(); } diff --git a/core/org.eclipse.cdt.core.tests/resources/rewrite/CommentHandlingTestSource.rts b/core/org.eclipse.cdt.core.tests/resources/rewrite/CommentHandlingTestSource.rts index 443348952a6..98fc157d935 100644 --- a/core/org.eclipse.cdt.core.tests/resources/rewrite/CommentHandlingTestSource.rts +++ b/core/org.eclipse.cdt.core.tests/resources/rewrite/CommentHandlingTestSource.rts @@ -3054,3 +3054,47 @@ private: =>trailing =>freestanding +//!CommentRecognition Bug 233438 +//#org.eclipse.cdt.core.parser.tests.rewrite.comenthandler.CommentHandlingTest +//@test.h +#ifndef HIDEMETHOD_H_ +#define HIDEMETHOD_H_ + +class HideMethod { +public: + HideMethod(); + virtual ~HideMethod(); + int methode2(); +private: + int i; +}; + +#endif /* HIDEMETHOD_H_ */ + +//= +=>leading +=>trailing +=>freestanding + +//@test.cpp +#include "test.h" + +HideMethod::HideMethod() +{ +} + +HideMethod::~HideMethod() +{ +} + +int HideMethod::methode2(){ + i++; + //comment + return i; +} + +//= +=>leading +return i; = //comment +=>trailing +=>freestanding 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 67a53836e3e..8901a868856 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 @@ -11,10 +11,12 @@ *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.rewrite.changegenerator; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import org.eclipse.cdt.core.dom.ast.ASTVisitor; +import org.eclipse.cdt.core.dom.ast.IASTComment; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTDeclaration; import org.eclipse.cdt.core.dom.ast.IASTDeclarator; @@ -241,7 +243,7 @@ public class ChangeGenerator extends CPPASTVisitor { targetLocation.getNodeLength(), newNodeCode)); break; case INSERT_BEFORE: - edit.addChild(new InsertEdit(targetLocation.getNodeOffset(), + edit.addChild(new InsertEdit(getOffsetIncludingComments(modification.getTargetNode()), newNodeCode)); break; case APPEND_CHILD: @@ -303,17 +305,56 @@ public class ChangeGenerator extends CPPASTVisitor { return formattedCode.toString(); } - public static String originalCodeOfNode(IASTNode node) { + public String originalCodeOfNode(IASTNode node) { if (node.getFileLocation() != null) { IFile sourceFile = FileHelper.getIFilefromIASTNode(node); - int nodeOffset = node.getFileLocation().getNodeOffset(); - int nodeLength = node.getFileLocation().getNodeLength(); - return FileContentHelper.getContent(sourceFile, nodeOffset, - nodeLength); + int nodeOffset = getOffsetIncludingComments(node); + int nodeLength = getNodeLengthIncludingComments(node); + + return FileContentHelper.getContent(sourceFile, nodeOffset, nodeLength); } return null; } + private int getNodeLengthIncludingComments(IASTNode node) { + int nodeOffset = node.getFileLocation().getNodeOffset(); + int nodeLength = node.getFileLocation().getNodeLength(); + + ArrayList comments = commentMap.getAllCommentsForNode(node); + if(!comments.isEmpty()) { + int startOffset = nodeOffset; + int endOffset = nodeOffset + nodeLength; + for(IASTComment comment : comments) { + IASTFileLocation commentLocation = comment.getFileLocation(); + if(commentLocation.getNodeOffset() < startOffset) { + startOffset = commentLocation.getNodeOffset(); + } + if(commentLocation.getNodeOffset() + commentLocation.getNodeLength() >= endOffset) { + endOffset = commentLocation.getNodeOffset() + commentLocation.getNodeLength(); + } + } + nodeLength = endOffset - startOffset; + } + return nodeLength; + } + + private int getOffsetIncludingComments(IASTNode node) { + int nodeOffset = node.getFileLocation().getNodeOffset(); + + ArrayList comments = commentMap.getAllCommentsForNode(node); + if(!comments.isEmpty()) { + int startOffset = nodeOffset; + for(IASTComment comment : comments) { + IASTFileLocation commentLocation = comment.getFileLocation(); + if(commentLocation.getNodeOffset() < startOffset) { + startOffset = commentLocation.getNodeOffset(); + } + } + nodeOffset = startOffset; + } + return nodeOffset; + } + private String getIndent(IASTNode nextNode) { IASTFileLocation fileLocation = nextNode.getFileLocation(); int length = fileLocation.getNodeOffset() @@ -564,7 +605,7 @@ public class ChangeGenerator extends CPPASTVisitor { } protected void createChange(MultiTextEdit edit, IASTNode changedNode) { - int changeOffset = changedNode.getFileLocation().getNodeOffset(); + int changeOffset = getOffsetIncludingComments(changedNode); TextEditGroup editGroup = new TextEditGroup(Messages.ChangeGenerator_group); for (ASTModification currentModification : modificationParent diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/ASTCommenter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/ASTCommenter.java index 53aa8250704..12505df3796 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/ASTCommenter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/ASTCommenter.java @@ -83,12 +83,12 @@ public class ASTCommenter { private static ArrayList removeAllPreprocessorComments(IASTTranslationUnit tu, ArrayList comments) { IASTPreprocessorStatement[] preprocessorStatements = tu.getAllPreprocessorStatements(); - TreeMap treeOfPreProcessorLines = new TreeMap(); + TreeMap treeOfPreProcessorLines = new TreeMap(); ArrayList listOfPreProcessorOffset = new ArrayList(); for (IASTPreprocessorStatement statement : preprocessorStatements) { if (isInWorkspace(statement)) { - treeOfPreProcessorLines.put(OffsetHelper.getStartingLineNumber(statement),null); + treeOfPreProcessorLines.put(OffsetHelper.getStartingLineNumber(statement), statement.getFileLocation().getFileName()); listOfPreProcessorOffset.add(statement.getFileLocation().getNodeOffset()); } } @@ -96,8 +96,10 @@ public class ASTCommenter { ArrayList commentsInCode = new ArrayList(); for (IASTComment comment : comments) { int comStartLineNumber = OffsetHelper.getStartingLineNumber(comment); - if (treeOfPreProcessorLines.containsKey(comStartLineNumber)) { - continue; + if (treeOfPreProcessorLines.containsKey(comStartLineNumber) + && treeOfPreProcessorLines.get(comStartLineNumber).equals(comment.getFileLocation().getFileName() + )) { + continue; } if(commentIsAtTheBeginningBeforePreprocessorStatements(comment, listOfPreProcessorOffset)) { continue; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/NodeCommentMap.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/NodeCommentMap.java index a711bbb1efd..4f65d336710 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/NodeCommentMap.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/commenthandler/NodeCommentMap.java @@ -135,4 +135,19 @@ public class NodeCommentMap { public HashMap> getFreestandingMap() { return freestandingMap; } + + /** + * Returns an ArrayList for the given node. This ArrayList contains all the comments + * which are assigned to this specific node. If no comments are available an empty + * ArrayList is returned. + * @param node The key to fetch the associated comments. + * @return ArrayList + */ + public ArrayList getAllCommentsForNode(IASTNode node) { + ArrayList comment = new ArrayList(); + comment.addAll(getFreestandingCommentsForNode(node)); + comment.addAll(getLeadingCommentsForNode(node)); + comment.addAll(getTrailingCommentsForNode(node)); + return comment; + } } diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts index 54829d9960b..6aac51914f6 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts @@ -77,6 +77,87 @@ void A::bar() int b = theAnswer; } +//!ExtractConstantInt 2 +//#org.eclipse.cdt.ui.tests.refactoring.extractconstant.ExtractConstantRefactoringTest +//@A.h +#ifndef A_H_ +#define A_H_ + +class A +{ +public: + A(); + virtual ~A(); + int foo(); + void bar(); +}; + +#endif /*A_H_*/ + + +//= +#ifndef A_H_ +#define A_H_ + +class A +{ +public: + A(); + virtual ~A(); + int foo(); + void bar(); + static const int theAnswer = 42; +}; + +#endif /*A_H_*/ + + +//@A.cpp +#include "A.h" + +A::A() +{ +} + +A::~A() +{ +} + +int A::foo() +{ + //Hallo + return //$42$//; +} + +void A::bar() +{ + int a = 42; + int b = 42; +} + +//= +#include "A.h" + +A::A() +{ +} + +A::~A() +{ +} + +int A::foo() +{ + //Hallo + return theAnswer; +} + +void A::bar() +{ + int a = theAnswer; + int b = theAnswer; +} + //!ExtractConstantFloat //#org.eclipse.cdt.ui.tests.refactoring.extractconstant.ExtractConstantRefactoringTest //@A.h 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 25ebbb0943b..e48a8023303 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts @@ -855,7 +855,8 @@ int A::foo() { int* i = new int(2); exp(i); - return *i; + //A end-comment + return *i; } int A::help()