Bug 464102 - Toggle Function for nested namespaces

Implemented proper handling of nested namespaces for toggle function
refactoring.

Change-Id: I850d3a7c9957dc2e26db4d1ac1aabf9a33bc2223
Signed-off-by: Thomas Corbat <tcorbat@hsr.ch>

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/togglefunction/ToggleRefactoringTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2013 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2008, 2015 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 
@@ -10,6 +10,7 @@
  *     Institute for Software - initial API and implementation
  *     Sergey Prigogin (Google)
  *     Marc-Andre Laperle (Ericsson)
+ *     Thomas Corbat (IFS)
  *******************************************************************************/
 package org.eclipse.cdt.ui.tests.refactoring.togglefunction;
 
@@ -2883,4 +2884,336 @@ public class ToggleRefactoringTest extends RefactoringTestBase {
 	public void testFunctionWithMacroReference_399215() throws Exception {
 		assertRefactoringSuccess();
 	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//void /*$*/foo/*$$*/() {
+	//}
+	//}
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//void foo();
+	//}
+	//}
+
+	//A.cpp
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//void foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	public void testFunctionInNestedNamespaceToSource_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//void /*$*/foo/*$$*/();
+	//}
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//void foo() {
+	//}
+	//}
+	//}
+
+	//A.cpp
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//void foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	//====================
+	//#include "A.h"
+
+	public void testFunctionInNestedNamespaceToHeader_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//void /*$*/foo/*$$*/();
+	//}
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//void foo() {
+	//}
+	//}
+	//}
+
+	//A.cpp
+	//#include "A.h"
+	//namespace outer {
+	//
+	//void bar() {
+	//}
+	//
+	//namespace inner {
+	//
+	//void foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//void bar() {
+	//}
+	//
+	//}
+
+	public void testFunctionInNestedNamespaceToHeaderLeaveNonemptyNamespace_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//struct S {
+	//	void /*$*/foo/*$$*/();
+	//};
+	//}
+	//}
+	//void outer::inner::S::foo() {
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//struct S {
+	//	void foo();
+	//};
+	//}
+	//}
+
+	//A.cpp
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//void S::foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	public void testQualifiedFunctionInNestedNamespaceToSource_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//struct S {
+	//	void /*$*/foo/*$$*/();
+	//};
+	//}
+	//}
+	//void outer::inner::S::foo() {
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//struct S {
+	//	void foo();
+	//};
+	//}
+	//}
+
+	//A.cpp
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//}
+	//
+	//}
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//void S::foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	public void testQualifiedFunctionInNestedNamespaceToSourceWithInnerNamespace_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//namespace outer {
+	//struct S {
+	//	void /*$*/foo/*$$*/();
+	//};
+	//}
+	//}
+	//}
+	//void outer::inner::outer::S::foo() {
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//namespace outer {
+	//struct S {
+	//	void foo();
+	//};
+	//}
+	//}
+	//}
+
+	//A.cpp
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//}
+	//
+	//}
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//namespace outer {
+	//
+	//void S::foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	//
+	//}
+	public void testQualifiedFunctionInRecurringNestedNamespaceToSourceWithInnerNamespace_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//namespace outer {
+	//struct S {
+	//	void /*$*/foo/*$$*/();
+	//};
+	//}
+	//}
+	//}
+	//void outer::inner::outer::S::foo() {
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//namespace outer {
+	//struct S {
+	//	void foo();
+	//};
+	//}
+	//}
+	//}
+
+	//A.cpp
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//namespace outer {
+	//
+	//void S::foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	//
+	//}
+	public void testQualifiedFunctionInRecurringNestedNamespaceToNewSource_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
+
+	//A.h
+	//namespace outer {
+	//namespace inner {
+	//struct S {
+	//	void /*$*/foo/*$$*/();
+	//};
+	//}
+	//}
+	//void outer::inner::S::foo() {
+	//}
+	//====================
+	//namespace outer {
+	//namespace inner {
+	//struct S {
+	//	void foo();
+	//};
+	//}
+	//}
+
+	//A.cpp
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//namespace outer {
+	//}
+	//
+	//}
+	//
+	//}
+	//====================
+	//#include "A.h"
+	//namespace outer {
+	//
+	//namespace inner {
+	//
+	//namespace outer {
+	//}
+	//
+	//void S::foo() {
+	//}
+	//
+	//}
+	//
+	//}
+	public void testQualifiedFunctionInNestedNamespaceToSourceWithRecurringNamespaceName_464102() throws Exception {
+		assertRefactoringSuccess();
+	}
 }

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromImplementationToHeaderOrClassStrategy.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2012 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2011, 2015 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 
@@ -9,9 +9,11 @@
  * Contributors: 
  * 	   Martin Schwab & Thomas Kallenberg - initial API and implementation
  *     Sergey Prigogin (Google)
+ *     Thomas Corbat (IFS)
  ******************************************************************************/
 package org.eclipse.cdt.internal.ui.refactoring.togglefunction;
 
+import java.util.Collections;
 import java.util.List;
 
 import org.eclipse.core.resources.IFile;
@@ -164,15 +166,29 @@ public class ToggleFromImplementationToHeaderOrClassStrategy implements IToggleR
 	}
 
 	private void removeDefinitionFromImplementation(ASTRewrite implast) {
-		ICPPASTNamespaceDefinition ns =
+		ICPPASTNamespaceDefinition ns = findOutermostNonemptyNamspace();
-				CPPVisitor.findAncestorWithType(context.getDefinition(), ICPPASTNamespaceDefinition.class);
+		if (ns != null) {
-		if (ns != null && isSingleElementInNamespace(ns, context.getDefinition())) {
 			implast.remove(ns, infoText);
 		} else {
 			implast.remove(context.getDefinition(), infoText);
 		}
 	}
 
+	private ICPPASTNamespaceDefinition findOutermostNonemptyNamspace() {
+		List<ICPPASTNamespaceDefinition> namespaces = ToggleNodeHelper.findSurroundingNamespaces(context.getDefinition());
+		Collections.reverse(namespaces);
+		IASTFunctionDefinition definition = context.getDefinition();
+		ICPPASTNamespaceDefinition ns = null;
+		for (ICPPASTNamespaceDefinition namespace : namespaces) {
+			if (isSingleElementInNamespace(namespace, definition)) {
+				ns = namespace;
+			} else {
+				break;
+			}
+		}
+		return ns;
+	}
+
 	private boolean isSingleElementInNamespace(ICPPASTNamespaceDefinition ns,
 			IASTFunctionDefinition definition) {
 		return ns.getChildren().length == 2 && (ns.contains(definition));

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromInHeaderToImplementationStrategy.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2013 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2011, 2015 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 
@@ -10,9 +10,11 @@
  *     Martin Schwab & Thomas Kallenberg - initial API and implementation
  *     Sergey Prigogin (Google)
  *     Marc-Andre Laperle (Ericsson)
+ *     Thomas Corbat (IFS)
  ******************************************************************************/
 package org.eclipse.cdt.internal.ui.refactoring.togglefunction;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.core.resources.IFile;
@@ -21,6 +23,7 @@ import org.eclipse.core.runtime.CoreException;
 import org.eclipse.text.edits.TextEditGroup;
 
 import org.eclipse.cdt.core.CCProjectNature;
+import org.eclipse.cdt.core.dom.ast.ASTNodeFactoryFactory;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.IASTArrayModifier;
 import org.eclipse.cdt.core.dom.ast.IASTComment;
@@ -53,18 +56,50 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNameSpecifier;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamespaceDefinition;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateParameter;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPNodeFactory;
 import org.eclipse.cdt.core.dom.rewrite.ASTRewrite;
 import org.eclipse.cdt.core.dom.rewrite.ASTRewrite.CommentPosition;
 
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNamespaceDefinition;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
 import org.eclipse.cdt.internal.core.dom.rewrite.ASTLiteralNode;
 
 import org.eclipse.cdt.internal.ui.refactoring.Container;
 import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector;
 
 public class ToggleFromInHeaderToImplementationStrategy implements IToggleRefactoringStrategy {
+	private class NamespaceFinderVisitor extends ASTVisitor {
+		private final List<ICPPASTNamespaceDefinition> namespaces;
+		private final Container<IASTNode> result;
+		protected int namespaceIndex = -1;
+		protected int deepestMatch = -1;
+
+		private NamespaceFinderVisitor(List<ICPPASTNamespaceDefinition> namespaces, Container<IASTNode> result) {
+			this.namespaces = namespaces;
+			this.result = result;
+			shouldVisitNamespaces = true;
+		}
+
+		@Override
+		public int visit(ICPPASTNamespaceDefinition namespaceDefinition) {
+			namespaceIndex++;
+			String namespaceName = namespaceDefinition.getName().toString();
+			if (namespaces.size() > namespaceIndex
+					&& namespaces.get(namespaceIndex).getName().toString().equals(namespaceName)) {
+				if (namespaceIndex > deepestMatch) {
+					result.setObject(namespaceDefinition);
+					deepestMatch = namespaceIndex;
+				}
+				return PROCESS_CONTINUE;
+			}
+			return PROCESS_SKIP;
+		}
+
+		@Override
+		public int leave(ICPPASTNamespaceDefinition namespaceDefinition) {
+			namespaceIndex--;
+			return super.leave(namespaceDefinition);
+		}
+	}
 	private IASTTranslationUnit implAst;
 	private ToggleRefactoringContext context;
 	private TextEditGroup infoText;
@@ -92,19 +127,22 @@ public class ToggleFromInHeaderToImplementationStrategy implements IToggleRefact
 			implRewrite.insertBefore(implAst, null, includeNode, infoText);
 		}
 		
-		IASTNode insertionParent = null;
+		IASTNode insertionParent = implAst.getTranslationUnit();
-		ICPPASTNamespaceDefinition parent = getParentNamespace();
+		List<ICPPASTNamespaceDefinition> namespaces = getSurroundingNamespaces();
 		
-		if (parent != null) {
+		if (!namespaces.isEmpty()) {
-			adaptQualifiedNameToNamespaceLevel(newDefinition, parent);
+			IASTNode namespaceInImplementation = searchNamespaceInImplementation(namespaces);
-			insertionParent = searchNamespaceInImplementation(parent.getName());
+			if (namespaceInImplementation != null) {
-			if (insertionParent == null) {
+				insertionParent = namespaceInImplementation;
-				insertionParent = createNamespace(parent);
+			}
-				implRewrite = implRewrite.insertBefore(implAst.getTranslationUnit(), 
+			adaptQualifiedNameToNamespaceLevel(newDefinition, namespaces);
-						null, insertionParent, infoText);
+
+			List<ICPPASTNamespaceDefinition> namespacesToAdd = getNamespacesToAdd(namespaces);
+			for (ICPPASTNamespaceDefinition namespace : namespacesToAdd) {
+				ICPPASTNamespaceDefinition newNamespace = createNamespace(namespace);
+				implRewrite = implRewrite.insertBefore(insertionParent, null, newNamespace, infoText);
+				insertionParent = newNamespace;
 			}
-		} else {
-			insertionParent = implAst.getTranslationUnit();
 		}
 		
 		newDefinition.setParent(insertionParent);
@@ -265,12 +303,12 @@ public class ToggleFromInHeaderToImplementationStrategy implements IToggleRefact
 		return true;
 	}
 
-	private ICPPASTNamespaceDefinition getParentNamespace() {
+	private List<ICPPASTNamespaceDefinition> getSurroundingNamespaces() {
 		IASTNode toquery = context.getDeclaration();
 		if (toquery == null) {
 			toquery = context.getDefinition();
 		}
-		return CPPVisitor.findAncestorWithType(toquery, ICPPASTNamespaceDefinition.class);
+		return ToggleNodeHelper.findSurroundingNamespaces(toquery);
 	}
 
 	private IASTNode findInsertionPoint(IASTNode insertionParent, IASTTranslationUnit unit) {
@@ -325,28 +363,26 @@ public class ToggleFromInHeaderToImplementationStrategy implements IToggleRefact
 		return newDefinition;
 	}
 
-	private void adaptQualifiedNameToNamespaceLevel(
+	private void adaptQualifiedNameToNamespaceLevel(IASTFunctionDefinition new_definition,
-			IASTFunctionDefinition new_definition, IASTNode parent) {
+			List<ICPPASTNamespaceDefinition> namespaces) {
-		if (parent instanceof ICPPASTNamespaceDefinition) {
+		if (new_definition.getDeclarator().getName() instanceof ICPPASTQualifiedName && !namespaces.isEmpty()) {
-			ICPPASTNamespaceDefinition ns = (ICPPASTNamespaceDefinition) parent;
+			ICPPNodeFactory nodeFactory = ASTNodeFactoryFactory.getDefaultCPPNodeFactory();
-			if (new_definition.getDeclarator().getName() instanceof ICPPASTQualifiedName) {
+			ICPPASTQualifiedName qname = (ICPPASTQualifiedName) new_definition.getDeclarator().getName();
-				ICPPASTQualifiedName qname =
+			ICPPASTName lastNameCopy = nodeFactory.newName(qname.getLastName().toCharArray());
-						(ICPPASTQualifiedName) new_definition.getDeclarator().getName();
+			ICPPASTQualifiedName qname_new = nodeFactory.newQualifiedName(lastNameCopy);
-				ICPPASTQualifiedName qname_new = new CPPASTQualifiedName();
+			boolean start = false;
-				boolean start = false;
+			ICPPASTNameSpecifier[] qualifiers = qname.getQualifier();
-				for(ICPPASTNameSpecifier partname: qname.getQualifier()) {
+			for (int i = 0; i < qualifiers.length; i++) {
-					if (partname.toString().equals(ns.getName().toString())) {
+				String qualifierName = qualifiers[i].toString();
-						start = true;
+				if (i < namespaces.size() && qualifierName.equals(namespaces.get(i).getName().toString())) {
-						continue;
+					start = true;
-					}
+				} else if (start) {
-					if (start)
+					qname_new.addNameSpecifier(qualifiers[i]);
-						qname_new.addNameSpecifier(partname);
-				}
-				if (start) {
-					qname_new.setLastName((ICPPASTName) qname.getLastName());
-					new_definition.getDeclarator().setName(qname_new);
 				}
 			}
+			if (start) {
+				new_definition.getDeclarator().setName(qname_new);
+			}
 		}
 	}
 
@@ -363,22 +399,30 @@ public class ToggleFromInHeaderToImplementationStrategy implements IToggleRefact
 		header_rewrite.remove(ToggleNodeHelper.getParentRemovePoint(context.getDefinition()), infoText);
 	}
 
-	private IASTNode searchNamespaceInImplementation(final IASTName name) {
+	private IASTNode searchNamespaceInImplementation(final List<ICPPASTNamespaceDefinition> namespaces) {
 		final Container<IASTNode> result = new Container<IASTNode>();
-		this.implAst.accept(new ASTVisitor() {
+		ASTVisitor visitor = new NamespaceFinderVisitor(namespaces, result);
+		this.implAst.accept(visitor);
+		return result.getObject();
+	}
+
+	private List<ICPPASTNamespaceDefinition> getNamespacesToAdd(final List<ICPPASTNamespaceDefinition> namespaces) {
+		final List<ICPPASTNamespaceDefinition> result = new ArrayList<ICPPASTNamespaceDefinition>();
+		this.implAst.accept(new NamespaceFinderVisitor(namespaces, new Container<IASTNode>()) {
 			{
-				shouldVisitNamespaces = true;
+				shouldVisitTranslationUnit = true;
 			}
 
 			@Override
-			public int visit(ICPPASTNamespaceDefinition namespaceDefinition) {
+			public int leave(IASTTranslationUnit tu) {
-				if (name.toString().equals(namespaceDefinition.getName().toString())) {
+				int startIndex = deepestMatch + 1;
-					result.setObject(namespaceDefinition);
+				int namespacesSize = namespaces.size();
-					return PROCESS_ABORT;
+				if (startIndex < namespacesSize) {
+					result.addAll(namespaces.subList(startIndex, namespacesSize));
 				}
-				return super.visit(namespaceDefinition);
+				return PROCESS_CONTINUE;
 			}
 		});
-		return result.getObject();
+		return result;
 	}
 }

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleNodeHelper.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2013 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2011, 2015 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 
@@ -9,6 +9,7 @@
  * Contributors: 
  *     Martin Schwab & Thomas Kallenberg - initial API and implementation
  *     Marc-Andre Laperle (Ericsson)
+ *     Thomas Corbat (IFS)
  ******************************************************************************/
 package org.eclipse.cdt.internal.ui.refactoring.togglefunction;
 
@@ -51,6 +52,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateId;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTypeId;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
 
 import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector;
 import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper;
@@ -395,4 +397,21 @@ public class ToggleNodeHelper extends NodeHelper {
 		}
 		return comments;
 	}
+
+	/**
+	 * Returns all namespace definitions surrounding <code>node</code>, ordered from outer to inner.
+	 * @param node to collect the namespaces for.
+	 * @return List of the surrounding namespaces.
+	 */
+	public static List<ICPPASTNamespaceDefinition> findSurroundingNamespaces(IASTNode node) {
+		ArrayList<ICPPASTNamespaceDefinition> namespaces = new ArrayList<>();
+		ICPPASTNamespaceDefinition currentNamespace = CPPVisitor.findAncestorWithType(node,
+				ICPPASTNamespaceDefinition.class);
+		while (currentNamespace != null) {
+			namespaces.add(0, currentNamespace);
+			currentNamespace = CPPVisitor.findAncestorWithType(currentNamespace.getParent(),
+					ICPPASTNamespaceDefinition.class);
+		}
+		return namespaces;
+	}
 }