Bug 518273 - Toggle function should remove the override keyword when moving out of class

Change-Id: I7ad80262c7f9b98d3ae9b23ae2b45bf0c94ccf99
Signed-off-by: Marc-Andre Laperle <malaperle@gmail.com>

4 files changed, 92 insertions, 7 deletions

diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFunctionDeclarator.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFunctionDeclarator.java
index f167de18372..b7d34d8ee44 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFunctionDeclarator.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFunctionDeclarator.java
@@ -213,6 +213,12 @@ public interface ICPPASTFunctionDeclarator extends IASTStandardFunctionDeclarato
 	public void addVirtSpecifier(ICPPASTVirtSpecifier virtSpecifier);
 
 	/**
+	 * Set virt-specifiers of this function.
+	 * @since 6.6
+	 */
+	public void setVirtSpecifiers(ICPPASTVirtSpecifier[] newVirtSpecifiers);
+
+	/**
 	 * @deprecated Not used.
 	 * @noreference This field is not intended to be referenced by clients.
 	 */
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFunctionDeclarator.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFunctionDeclarator.java
index 0aa2b78e943..7c4fd31fa22 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFunctionDeclarator.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFunctionDeclarator.java
@@ -362,12 +362,26 @@ public class CPPASTFunctionDeclarator extends CPPASTDeclarator implements ICPPAS
 
 	@Override
 	public void addVirtSpecifier(ICPPASTVirtSpecifier virtSpecifier) {
-        assertNotFrozen();
-    	if (virtSpecifier != null) {
-    		assert virtSpecifiers != null;
-    		virtSpecifiers = ArrayUtil.append(virtSpecifiers, virtSpecifier);
-    		virtSpecifier.setParent(this);
-			virtSpecifier.setPropertyInParent(EXCEPTION_TYPEID);
-    	}
+		assertNotFrozen();
+		if (virtSpecifier != null) {
+			assert virtSpecifiers != null;
+			virtSpecifiers = ArrayUtil.append(virtSpecifiers, virtSpecifier);
+			virtSpecifier.setParent(this);
+			virtSpecifier.setPropertyInParent(VIRT_SPECIFIER);
+		}
+	}
+
+	@Override
+	public void setVirtSpecifiers(ICPPASTVirtSpecifier[] newVirtSpecifiers) {
+		assertNotFrozen();
+		if (newVirtSpecifiers == null) {
+			virtSpecifiers = NO_VIRT_SPECIFIERS;
+		} else {
+			virtSpecifiers = newVirtSpecifiers;
+			for (ICPPASTVirtSpecifier virtSpecifier : newVirtSpecifiers) {
+				virtSpecifier.setParent(this);
+				virtSpecifier.setPropertyInParent(VIRT_SPECIFIER);
+			}
+		}
 	}
 }
diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/togglefunction/ToggleRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/togglefunction/ToggleRefactoringTest.java
index f16f3c9e9d6..4f6774f9617 100644
--- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/togglefunction/ToggleRefactoringTest.java
+++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/togglefunction/ToggleRefactoringTest.java
@@ -3568,4 +3568,63 @@ public class ToggleRefactoringTest extends RefactoringTestBase {
 	public void testFreeFunctionFromHeaderToImplInC_531701() throws Exception {
 		assertRefactoringSuccess();
 	}
+
+	//Test.h
+	//class Base {
+	//	virtual void foo() {
+	//	}
+	//};
+	//
+	//class Foo : public Base {
+	//	void /*$*/foo/*$$*/() override final
+	//	{
+	//	}
+	//};
+	//====================
+	//class Base {
+	//	virtual void foo() {
+	//	}
+	//};
+	//
+	//class Foo : public Base {
+	//	void foo() override final;
+	//};
+	//
+	//inline void Foo::foo() {
+	//}
+	public void testToggleWithVirtSpecifiers_518273() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//Test.h
+	//class Base {
+	//	virtual void foo() {
+	//	}
+	//};
+	//
+	//class Foo : public Base {
+	//	void foo() override final;
+	//};
+	//====================
+	//class Base {
+	//	virtual void foo() {
+	//	}
+	//};
+	//
+	//class Foo : public Base {
+	//	void foo() override final
+	//	{
+	//	}
+	//};
+
+	//Test.cpp
+	//#include "Test.h"
+	//
+	//void Foo::/*$*/foo/*$$*/() {
+	//}
+	//====================
+	//#include "Test.h"
+	public void testToggleWithVirtSpecifiersImplementationToClass_518273() throws Exception {
+		assertRefactoringSuccess();
+	}
 }
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleNodeHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleNodeHelper.java
index a3d3855a1e4..afbbb9bc27f 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleNodeHelper.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleNodeHelper.java
@@ -40,6 +40,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCatchHandler;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTConstructorChainInitializer;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTDeclarator;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionWithTryBlock;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTName;
@@ -144,6 +145,11 @@ public class ToggleNodeHelper extends NodeHelper {
 
 		ICPPASTFunctionDefinition newFunction = 
 				createFunctionSignatureWithEmptyBody(newDeclSpecifier, newDeclarator, oldDefinition);
+		// Virt-specifiers are only valid in the class declaration.
+		if (newFunction.getDeclarator() instanceof ICPPASTFunctionDeclarator) {
+			ICPPASTFunctionDeclarator functionDeclarator = (ICPPASTFunctionDeclarator) newFunction.getDeclarator();
+			functionDeclarator.setVirtSpecifiers(null);
+		}
 
 		return newFunction;
 	}