From c0402d5d06e9840f1fe5ac2cfa36b21d3e4d4072 Mon Sep 17 00:00:00 2001 From: Marco Stornelli Date: Sat, 25 Jan 2020 11:42:36 +0100 Subject: [PATCH] Bug 434677 - Fix implement method insert position with namespaces If the correct namespace existed before the refactoring in the translation unit, refactoring process just ignored it. Change-Id: I9d6bd301807bb2d3f83f74ef772395d3470cf8bd Signed-off-by: Marco Stornelli --- .../ImplementMethodRefactoringTest.java | 22 ++++++++ .../implementmethod/InsertLocation.java | 2 + .../MethodDefinitionInsertLocationFinder.java | 52 ++++++++++++++++++- 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java index 3b917123ce1..019ba9eff69 100755 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java @@ -897,4 +897,26 @@ public class ImplementMethodRefactoringTest extends RefactoringTestBase { public void testWithInnerEnum_Bug452809() throws Exception { assertRefactoringSuccess(); } + + //A.h + // + //namespace N { + //struct A { + // /*$*/void waldo();/*$$*/ + //}; + //} + + //A.cpp + //#include "A.h" + //namespace N {} + //==================== + //#include "A.h" + //namespace N { + //void A::waldo() { + //} + // + //} + public void testNamespaceAlreadyInDefinition_Bug434677() throws Exception { + assertRefactoringSuccess(); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java index 71afa47bccc..74cbfd98745 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java @@ -63,6 +63,8 @@ public class InsertLocation { } else if (nodeToInsertAfter != null) { IASTFileLocation fileLocation = nodeToInsertAfter.getFileLocation(); return fileLocation.getNodeOffset() + fileLocation.getNodeLength(); + } else if (parentNode != null) { + return parentNode.getFileLocation().getNodeOffset(); } return 0; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java index 88ad130e390..4cefeae76e8 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java @@ -22,6 +22,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.eclipse.cdt.core.dom.ast.ASTVisitor; +import org.eclipse.cdt.core.dom.ast.DOMException; import org.eclipse.cdt.core.dom.ast.IASTDeclaration; import org.eclipse.cdt.core.dom.ast.IASTFileLocation; import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator; @@ -29,11 +31,18 @@ import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition; import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTName; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamespaceDefinition; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPNamespace; import org.eclipse.cdt.core.model.ITranslationUnit; +import org.eclipse.cdt.core.parser.util.CharArrayUtils; import org.eclipse.cdt.internal.ui.editor.SourceHeaderPartnerFinder; import org.eclipse.cdt.internal.ui.refactoring.CRefactoringContext; import org.eclipse.cdt.internal.ui.refactoring.utils.DefinitionFinder; +import org.eclipse.cdt.internal.ui.refactoring.utils.NamespaceHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; @@ -111,7 +120,48 @@ public class MethodDefinitionInsertLocationFinder { ITranslationUnit partner = SourceHeaderPartnerFinder.getPartnerTranslationUnit(declarationTu, refactoringContext); if (partner != null) { - insertLocation.setParentNode(refactoringContext.getAST(partner, null), partner); + if (methodDeclarationLocation == null) { + insertLocation.setParentNode(refactoringContext.getAST(partner, null), partner); + return insertLocation; + } + final ICPPASTName[] names = NamespaceHelper.getSurroundingNamespace(declarationTu, + methodDeclarationLocation.getNodeOffset(), refactoringContext); + IASTTranslationUnit ast = refactoringContext.getAST(partner, null); + IASTNode[] target = new IASTNode[1]; + if (ast != null) { + ast.accept(new ASTVisitor() { + { + shouldVisitNamespaces = true; + } + + @Override + public int visit(ICPPASTNamespaceDefinition namespaceDefinition) { + IASTName name = namespaceDefinition.getName(); + IBinding binding = name.resolveBinding(); + if (!(binding instanceof ICPPNamespace)) + return PROCESS_CONTINUE; + try { + char[][] qualNames = ((ICPPNamespace) binding).getQualifiedNameCharArray(); + if (qualNames.length != names.length - 1) + return PROCESS_CONTINUE; + for (int i = 0; i < names.length - 1; ++i) { + if (!CharArrayUtils.equals(qualNames[i], names[i].getSimpleID())) + return PROCESS_CONTINUE; + } + } catch (DOMException e) { + e.printStackTrace(); + return PROCESS_CONTINUE; + } + target[0] = namespaceDefinition; + return PROCESS_ABORT; + } + }); + } + if (target[0] != null) { + insertLocation.setParentNode(target[0], partner); + } else { + insertLocation.setParentNode(ast, partner); + } } } else { insertLocation.setParentNode(parent.getTranslationUnit(), declarationTu);