From d36ed7cfd5cd23a7b219812bd0b9b0a085f03f77 Mon Sep 17 00:00:00 2001 From: Marco Stornelli Date: Sun, 19 Apr 2020 15:30:27 +0200 Subject: [PATCH] Bug 562292 - Fix method definition position for override method The position of namespaces didn't take into account because the declaration doesn't exist yet when we use this kind of refactoring, so the find method of MethodDefinitionInsertLocationFinder didn't look for namespaces. Change-Id: I839194879c41f86653c837ca83a306ea1840c1d0 --- .../OverrideMethodsRefactoringTest.java | 62 +++++++++++++++++++ .../ImplementMethodRefactoring.java | 10 ++- .../MethodDefinitionInsertLocationFinder.java | 12 +++- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/overridemethods/OverrideMethodsRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/overridemethods/OverrideMethodsRefactoringTest.java index 9147c8b2a52..5fcde1e927b 100755 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/overridemethods/OverrideMethodsRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/overridemethods/OverrideMethodsRefactoringTest.java @@ -672,4 +672,66 @@ public class OverrideMethodsRefactoringTest extends RefactoringTestBase { selectedMethods = new String[] { "baseFunc()const" }; assertRefactoringSuccess(); } + + //A.h + //namespace Bar { + //namespace Foo { + //namespace Baz { + //class Base { + //public: + // virtual ~Base(); + // virtual void baseFunc() const = 0; + //}; + //class X: public Base { + //public: + // X(); + // /*$*//*$$*/ + //}; + //} + //} + //} + //==================== + //namespace Bar { + //namespace Foo { + //namespace Baz { + //class Base { + //public: + // virtual ~Base(); + // virtual void baseFunc() const = 0; + //}; + //class X: public Base { + //public: + // X(); + // virtual void baseFunc() const; + //}; + //} + //} + //} + + //A.cpp + //#include "A.h" + // + //namespace Bar { + //namespace Foo { + //namespace Baz { + //} + //} + //} + //==================== + //#include "A.h" + // + //namespace Bar { + //namespace Foo { + //namespace Baz { + // + //void X::baseFunc() const { + //} + // + //} + //} + //} + public void testWithHeaderAndSource_Bug562292() throws Exception { + selectedMethods = new String[] { "baseFunc()const" }; + assertRefactoringSuccess(); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java index ace1e899f5d..f6bdc336959 100755 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java @@ -229,7 +229,7 @@ public class ImplementMethodRefactoring extends CRefactoring { throw new OperationCanceledException(); } IASTSimpleDeclaration decl = config.getDeclaration(); - InsertLocation insertLocation = findInsertLocation(decl, subMonitor); + InsertLocation insertLocation = findInsertLocation(decl, subMonitor, functionOffset); if (subMonitor.isCanceled()) { throw new OperationCanceledException(); } @@ -277,11 +277,17 @@ public class ImplementMethodRefactoring extends CRefactoring { private InsertLocation findInsertLocation(IASTSimpleDeclaration methodDeclaration, IProgressMonitor subMonitor) throws CoreException { + return findInsertLocation(methodDeclaration, subMonitor, -1); + } + + private InsertLocation findInsertLocation(IASTSimpleDeclaration methodDeclaration, IProgressMonitor subMonitor, + int functionOffset) throws CoreException { if (insertLocations.containsKey(methodDeclaration)) { return insertLocations.get(methodDeclaration); } InsertLocation insertLocation = methodDefinitionInsertLocationFinder.find(tu, - methodDeclaration.getFileLocation(), methodDeclaration.getParent(), refactoringContext, subMonitor); + methodDeclaration.getFileLocation(), methodDeclaration.getParent(), refactoringContext, subMonitor, + functionOffset); if (insertLocation.getTranslationUnit() == null || NodeHelper.isContainedInTemplateDeclaration(methodDeclaration)) { 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 4cefeae76e8..9a45998a18b 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 @@ -63,6 +63,12 @@ public class MethodDefinitionInsertLocationFinder { public InsertLocation find(ITranslationUnit declarationTu, IASTFileLocation methodDeclarationLocation, IASTNode parent, CRefactoringContext refactoringContext, IProgressMonitor pm) throws CoreException { + return find(declarationTu, methodDeclarationLocation, parent, refactoringContext, pm, -1); + } + + public InsertLocation find(ITranslationUnit declarationTu, IASTFileLocation methodDeclarationLocation, + IASTNode parent, CRefactoringContext refactoringContext, IProgressMonitor pm, int functionOffset) + throws CoreException { IASTDeclaration[] declarations = NodeHelper.getDeclarations(parent); InsertLocation insertLocation = new InsertLocation(); @@ -120,12 +126,14 @@ public class MethodDefinitionInsertLocationFinder { ITranslationUnit partner = SourceHeaderPartnerFinder.getPartnerTranslationUnit(declarationTu, refactoringContext); if (partner != null) { - if (methodDeclarationLocation == null) { + if (methodDeclarationLocation == null && functionOffset < 0) { insertLocation.setParentNode(refactoringContext.getAST(partner, null), partner); return insertLocation; } final ICPPASTName[] names = NamespaceHelper.getSurroundingNamespace(declarationTu, - methodDeclarationLocation.getNodeOffset(), refactoringContext); + methodDeclarationLocation != null ? methodDeclarationLocation.getNodeOffset() + : functionOffset, + refactoringContext); IASTTranslationUnit ast = refactoringContext.getAST(partner, null); IASTNode[] target = new IASTNode[1]; if (ast != null) {