From f4ed32d9b293f6fdcf3e217b2812a09d8403e2f6 Mon Sep 17 00:00:00 2001 From: Marc-Andre Laperle Date: Sat, 15 Jun 2013 20:55:30 -0400 Subject: [PATCH] Bug 316083 - Generate getters and setters does not handle nested classes in cpp definition Change-Id: Id2926661984c7f436aee3cd98b5b5922c2474097 Signed-off-by: Marc-Andre Laperle Reviewed-on: https://git.eclipse.org/r/13843 --- .../GenerateGettersAndSettersTest.java | 39 +++++++++++++++- .../gettersandsetters/AccessorDescriptor.java | 33 ++----------- .../gettersandsetters/AccessorFactory.java | 46 +++++++------------ .../GenerateGettersAndSettersRefactoring.java | 16 +++++-- 4 files changed, 70 insertions(+), 64 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java index e2ef1e4aa3f..26dc2736217 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2013 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: * Institute for Software - initial API and implementation * Sergey Prigogin (Google) + * Marc-Andre Laperle (Ericsson) *******************************************************************************/ package org.eclipse.cdt.ui.tests.refactoring.gettersandsetters; @@ -1631,4 +1632,40 @@ public class GenerateGettersAndSettersTest extends RefactoringTestBase { selectedSetters = new String[] { "a", "b" }; assertRefactoringSuccess(); } + + //A.h + //namespace ns { + //class Test { + //class Foo { + // public: + // int /*$*/a/*$$*/; + // }; + // }; + //} + //==================== + //namespace ns { + //class Test { + //class Foo { + // public: + // int a; + // + // int getA() const; + // }; + // }; + //} + + //A.cpp + //#include "A.h" + // + //==================== + //#include "A.h" + // + //int ns::Test::Foo::getA() const { + // return a; + //} + public void testNestedClasses_Bug316083() throws Exception { + definitionSeparate = true; + selectedGetters = new String[] {"a"}; + assertRefactoringSuccess(); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorDescriptor.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorDescriptor.java index 1ae6e2b08d8..b33a75efab5 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorDescriptor.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorDescriptor.java @@ -1,27 +1,21 @@ /******************************************************************************* - * Copyright (c) 2011, 2012 Google, Inc and others. + * Copyright (c) 2011, 2013 Google, Inc and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Sergey Prigogin (Google) - initial API and implementation + * Sergey Prigogin (Google) - initial API and implementation + * Marc-Andre Laperle (Ericsson) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.gettersandsetters; import com.ibm.icu.text.Collator; -import org.eclipse.cdt.core.dom.ast.IASTCompositeTypeSpecifier; 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.IASTNode.CopyStyle; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTName; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; - -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName; public class AccessorDescriptor implements Comparable { public enum AccessorKind { @@ -99,24 +93,7 @@ public class AccessorDescriptor implements Comparable { return accessorDeclaration; } - public IASTFunctionDefinition getAccessorDefinition(boolean qualifedName) { - ICPPASTQualifiedName qname; - if (qualifedName) { - qname = getClassName(); - } else { - qname = null; - } - - return accessorFactory.createDefinition(qname); - } - - private ICPPASTQualifiedName getClassName() { - IASTNode node = fieldName.getParent(); - while (!(node instanceof IASTCompositeTypeSpecifier)) { - node = node.getParent(); - } - IASTCompositeTypeSpecifier comp = (IASTCompositeTypeSpecifier) node; - - return new CPPASTQualifiedName((ICPPASTName) comp.getName().copy(CopyStyle.withLocations)); + public IASTFunctionDefinition getAccessorDefinition(IASTName declaratorName) { + return accessorFactory.createDefinition(declaratorName); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorFactory.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorFactory.java index 11638a05ca7..ca049293794 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorFactory.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/AccessorFactory.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2013 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: * Institute for Software - initial API and implementation * Sergey Prigogin (Google) + * Marc-Andre Laperle (Ericsson) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.gettersandsetters; @@ -27,7 +28,6 @@ import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTArrayDeclarator; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; import org.eclipse.cdt.core.dom.rewrite.TypeHelper; import org.eclipse.cdt.core.parser.Keywords; @@ -88,9 +88,9 @@ public abstract class AccessorFactory { /** * Creates an accessor definition. * - * @param className qualified name of the class containing the accessor + * @param declaratorName name to use for the declarator */ - public abstract IASTFunctionDefinition createDefinition(ICPPASTQualifiedName className); + public abstract IASTFunctionDefinition createDefinition(IASTName declaratorName); protected IASTDeclSpecifier getParamOrReturnDeclSpecifier() { IASTDeclSpecifier declSpec = declSpecifier.copy(CopyStyle.withLocations); @@ -109,16 +109,16 @@ public abstract class AccessorFactory { public IASTSimpleDeclaration createDeclaration() { IASTSimpleDeclaration getter = new CPPASTSimpleDeclaration(); getter.setDeclSpecifier(getParamOrReturnDeclSpecifier()); - getter.addDeclarator(getGetterDeclarator(null)); + getter.addDeclarator(getGetterDeclarator(new CPPASTName(accessorName.toCharArray()))); return getter; } @Override - public IASTFunctionDefinition createDefinition(ICPPASTQualifiedName className) { + public IASTFunctionDefinition createDefinition(IASTName declaratorName) { IASTFunctionDefinition getter = new CPPASTFunctionDefinition(); getter.setDeclSpecifier(getParamOrReturnDeclSpecifier()); - IASTDeclarator getterDeclarator = getGetterDeclarator(className); + IASTDeclarator getterDeclarator = getGetterDeclarator(declaratorName); // IASTFunctionDefinition expects the outermost IASTFunctionDeclarator in declarator hierarchy while (!(getterDeclarator instanceof IASTFunctionDeclarator)) { getterDeclarator = getterDeclarator.getNestedDeclarator(); @@ -140,10 +140,7 @@ public abstract class AccessorFactory { return compound; } - private IASTDeclarator getGetterDeclarator(ICPPASTQualifiedName qualifiedName) { - CPPASTName getterName = new CPPASTName(); - getterName.setName(accessorName.toCharArray()); - + private IASTDeclarator getGetterDeclarator(IASTName declaratorName) { // Copy declarator hierarchy IASTDeclarator topDeclarator = fieldDeclarator.copy(CopyStyle.withLocations); @@ -167,12 +164,8 @@ public abstract class AccessorFactory { // Create a new innermost function declarator based on the field declarator CPPASTFunctionDeclarator functionDeclarator = new CPPASTFunctionDeclarator(); functionDeclarator.setConst(true); - if (qualifiedName != null) { - qualifiedName.addName(getterName); - functionDeclarator.setName(qualifiedName); - } else { - functionDeclarator.setName(getterName); - } + functionDeclarator.setName(declaratorName); + for (IASTPointerOperator pointer : innermost.getPointerOperators()){ functionDeclarator.addPointerOperator(pointer.copy(CopyStyle.withLocations)); } @@ -200,16 +193,16 @@ public abstract class AccessorFactory { @Override public IASTSimpleDeclaration createDeclaration() { IASTSimpleDeclaration setter = new CPPASTSimpleDeclaration(); - setter.setDeclSpecifier(getVoidDeclSpec()); - setter.addDeclarator(getSetterDeclarator(null)); + setter.setDeclSpecifier(getVoidDeclSpec()); + setter.addDeclarator(getSetterDeclarator(new CPPASTName(accessorName.toCharArray()))); return setter; } @Override - public IASTFunctionDefinition createDefinition(ICPPASTQualifiedName className) { + public IASTFunctionDefinition createDefinition(IASTName declaratorName) { IASTFunctionDefinition setter = new CPPASTFunctionDefinition(); setter.setDeclSpecifier(getVoidDeclSpec()); - setter.setDeclarator(getSetterDeclarator(className)); + setter.setDeclarator(getSetterDeclarator(declaratorName)); setter.setBody(getSetterBody()); return setter; } @@ -244,16 +237,9 @@ public abstract class AccessorFactory { return compound; } - private CPPASTFunctionDeclarator getSetterDeclarator(ICPPASTQualifiedName qualifiedName) { - CPPASTName setterName = new CPPASTName(); - setterName.setName(accessorName.toCharArray()); + private CPPASTFunctionDeclarator getSetterDeclarator(IASTName declaratorName) { CPPASTFunctionDeclarator declarator = new CPPASTFunctionDeclarator(); - if (qualifiedName != null) { - qualifiedName.addName(setterName); - declarator.setName(qualifiedName); - } else { - declarator.setName(setterName); - } + declarator.setName(declaratorName); CPPASTParameterDeclaration parameterDeclaration = new CPPASTParameterDeclaration(); IASTDeclarator parameterDeclarator = fieldDeclarator.copy(CopyStyle.withLocations); parameterDeclarator.setName(getSetterParameterName()); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java index e81c6c41394..9fbf697ba59 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2013 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 @@ -43,6 +43,7 @@ import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; import org.eclipse.cdt.core.model.ICElement; import org.eclipse.cdt.core.model.ICProject; +import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ContainerNode; @@ -53,6 +54,7 @@ import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; import org.eclipse.cdt.internal.ui.refactoring.implementmethod.InsertLocation; import org.eclipse.cdt.internal.ui.refactoring.implementmethod.MethodDefinitionInsertLocationFinder; import org.eclipse.cdt.internal.ui.refactoring.utils.Checks; +import org.eclipse.cdt.internal.ui.refactoring.utils.NameHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum; @@ -224,24 +226,28 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring { throws CoreException, OperationCanceledException { List getterAndSetters = new ArrayList(); List definitions = new ArrayList(); + ICPPASTCompositeTypeSpecifier classDefinition = + CPPVisitor.findAncestorWithType(context.existingFields.get(0), ICPPASTCompositeTypeSpecifier.class); for (AccessorDescriptor accessor : context.selectedAccessors) { + IASTName accessorName = new CPPASTName(accessor.toString().toCharArray()); if (context.isDefinitionSeparate()) { getterAndSetters.add(accessor.getAccessorDeclaration()); - IASTFunctionDefinition functionDefinition = accessor.getAccessorDefinition(true); + IASTName declaratorName = NameHelper.createQualifiedNameFor( + accessorName, classDefinition.getTranslationUnit().getOriginatingTranslationUnit(), classDefinition.getFileLocation().getNodeOffset(), + definitionInsertLocation.getTranslationUnit(), definitionInsertLocation.getInsertPosition(), refactoringContext); + IASTFunctionDefinition functionDefinition = accessor.getAccessorDefinition(declaratorName); // Standalone definitions in a header file have to be declared inline. if (definitionInsertLocation.getTranslationUnit().isHeaderUnit()) { functionDefinition.getDeclSpecifier().setInline(true); } definitions.add(functionDefinition); } else { - getterAndSetters.add(accessor.getAccessorDefinition(false)); + getterAndSetters.add(accessor.getAccessorDefinition(accessorName)); } } if (context.isDefinitionSeparate()) { addDefinition(collector, definitions, pm); } - ICPPASTCompositeTypeSpecifier classDefinition = - CPPVisitor.findAncestorWithType(context.existingFields.get(0), ICPPASTCompositeTypeSpecifier.class); ClassMemberInserter.createChange(classDefinition, VisibilityEnum.v_public, getterAndSetters, false, collector);