From 442bcd62e11f4e5a26ffc9347e4b342be807e19d Mon Sep 17 00:00:00 2001 From: Thomas Corbat Date: Wed, 8 Apr 2015 15:44:02 +0200 Subject: [PATCH] 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 --- .../togglefunction/ToggleRefactoringTest.java | 335 +++++++++++++++++- ...ImplementationToHeaderOrClassStrategy.java | 24 +- ...eFromInHeaderToImplementationStrategy.java | 134 ++++--- .../togglefunction/ToggleNodeHelper.java | 21 +- 4 files changed, 463 insertions(+), 51 deletions(-) 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 37598c4125e..1445b7fbdca 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 @@ -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(); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromImplementationToHeaderOrClassStrategy.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromImplementationToHeaderOrClassStrategy.java index b7ab8fa087b..d15489297b4 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromImplementationToHeaderOrClassStrategy.java +++ b/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 = - CPPVisitor.findAncestorWithType(context.getDefinition(), ICPPASTNamespaceDefinition.class); - if (ns != null && isSingleElementInNamespace(ns, context.getDefinition())) { + ICPPASTNamespaceDefinition ns = findOutermostNonemptyNamspace(); + if (ns != null) { implast.remove(ns, infoText); } else { implast.remove(context.getDefinition(), infoText); } } + private ICPPASTNamespaceDefinition findOutermostNonemptyNamspace() { + List 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)); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromInHeaderToImplementationStrategy.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromInHeaderToImplementationStrategy.java index 93558f74087..c207617295c 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/togglefunction/ToggleFromInHeaderToImplementationStrategy.java +++ b/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 namespaces; + private final Container result; + protected int namespaceIndex = -1; + protected int deepestMatch = -1; + + private NamespaceFinderVisitor(List namespaces, Container 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; - ICPPASTNamespaceDefinition parent = getParentNamespace(); + IASTNode insertionParent = implAst.getTranslationUnit(); + List namespaces = getSurroundingNamespaces(); - if (parent != null) { - adaptQualifiedNameToNamespaceLevel(newDefinition, parent); - insertionParent = searchNamespaceInImplementation(parent.getName()); - if (insertionParent == null) { - insertionParent = createNamespace(parent); - implRewrite = implRewrite.insertBefore(implAst.getTranslationUnit(), - null, insertionParent, infoText); + if (!namespaces.isEmpty()) { + IASTNode namespaceInImplementation = searchNamespaceInImplementation(namespaces); + if (namespaceInImplementation != null) { + insertionParent = namespaceInImplementation; + } + adaptQualifiedNameToNamespaceLevel(newDefinition, namespaces); + + List 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 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( - IASTFunctionDefinition new_definition, IASTNode parent) { - if (parent instanceof ICPPASTNamespaceDefinition) { - ICPPASTNamespaceDefinition ns = (ICPPASTNamespaceDefinition) parent; - if (new_definition.getDeclarator().getName() instanceof ICPPASTQualifiedName) { - ICPPASTQualifiedName qname = - (ICPPASTQualifiedName) new_definition.getDeclarator().getName(); - ICPPASTQualifiedName qname_new = new CPPASTQualifiedName(); - boolean start = false; - for(ICPPASTNameSpecifier partname: qname.getQualifier()) { - if (partname.toString().equals(ns.getName().toString())) { - start = true; - continue; - } - if (start) - qname_new.addNameSpecifier(partname); - } - if (start) { - qname_new.setLastName((ICPPASTName) qname.getLastName()); - new_definition.getDeclarator().setName(qname_new); + private void adaptQualifiedNameToNamespaceLevel(IASTFunctionDefinition new_definition, + List namespaces) { + if (new_definition.getDeclarator().getName() instanceof ICPPASTQualifiedName && !namespaces.isEmpty()) { + ICPPNodeFactory nodeFactory = ASTNodeFactoryFactory.getDefaultCPPNodeFactory(); + ICPPASTQualifiedName qname = (ICPPASTQualifiedName) new_definition.getDeclarator().getName(); + ICPPASTName lastNameCopy = nodeFactory.newName(qname.getLastName().toCharArray()); + ICPPASTQualifiedName qname_new = nodeFactory.newQualifiedName(lastNameCopy); + boolean start = false; + ICPPASTNameSpecifier[] qualifiers = qname.getQualifier(); + for (int i = 0; i < qualifiers.length; i++) { + String qualifierName = qualifiers[i].toString(); + if (i < namespaces.size() && qualifierName.equals(namespaces.get(i).getName().toString())) { + start = true; + } else if (start) { + qname_new.addNameSpecifier(qualifiers[i]); } } + 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 namespaces) { final Container result = new Container(); - this.implAst.accept(new ASTVisitor() { + ASTVisitor visitor = new NamespaceFinderVisitor(namespaces, result); + this.implAst.accept(visitor); + return result.getObject(); + } + + private List getNamespacesToAdd(final List namespaces) { + final List result = new ArrayList(); + this.implAst.accept(new NamespaceFinderVisitor(namespaces, new Container()) { { - shouldVisitNamespaces = true; + shouldVisitTranslationUnit = true; } @Override - public int visit(ICPPASTNamespaceDefinition namespaceDefinition) { - if (name.toString().equals(namespaceDefinition.getName().toString())) { - result.setObject(namespaceDefinition); - return PROCESS_ABORT; + public int leave(IASTTranslationUnit tu) { + int startIndex = deepestMatch + 1; + int namespacesSize = namespaces.size(); + if (startIndex < namespacesSize) { + result.addAll(namespaces.subList(startIndex, namespacesSize)); } - return super.visit(namespaceDefinition); + return PROCESS_CONTINUE; } }); - return result.getObject(); + return result; } } 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 0c5d8137046..f54213cf479 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 @@ -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 node, ordered from outer to inner. + * @param node to collect the namespaces for. + * @return List of the surrounding namespaces. + */ + public static List findSurroundingNamespaces(IASTNode node) { + ArrayList 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; + } }