Bug 316083 - Generate getters and setters does not handle nested classes

in cpp definition

Change-Id: Id2926661984c7f436aee3cd98b5b5922c2474097
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/13843

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();
+	}
 }

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<AccessorDescriptor> {
 	public enum AccessorKind {
@@ -99,24 +93,7 @@ public class AccessorDescriptor implements Comparable<AccessorDescriptor> {
 		return accessorDeclaration;
 	}
 
-	public IASTFunctionDefinition getAccessorDefinition(boolean qualifedName) {
+	public IASTFunctionDefinition getAccessorDefinition(IASTName declaratorName) {
-		ICPPASTQualifiedName qname;
+		return accessorFactory.createDefinition(declaratorName);
-		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));
 	}
 }

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) {
+		private IASTDeclarator getGetterDeclarator(IASTName declaratorName) {
-			CPPASTName getterName = new CPPASTName();
-			getterName.setName(accessorName.toCharArray());
-
 			// 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) {
+			functionDeclarator.setName(declaratorName);
-				qualifiedName.addName(getterName);
+
-				functionDeclarator.setName(qualifiedName);
-			} else {
-				functionDeclarator.setName(getterName);
-			}
 			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.setDeclSpecifier(getVoidDeclSpec());
-			setter.addDeclarator(getSetterDeclarator(null));
+			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) {
+		private CPPASTFunctionDeclarator getSetterDeclarator(IASTName declaratorName) {
-			CPPASTName setterName = new CPPASTName();
-			setterName.setName(accessorName.toCharArray());
 			CPPASTFunctionDeclarator declarator = new CPPASTFunctionDeclarator();
-			if (qualifiedName != null) {
+			declarator.setName(declaratorName);
-				qualifiedName.addName(setterName);
-				declarator.setName(qualifiedName);
-			} else {
-				declarator.setName(setterName);
-			}
 			CPPASTParameterDeclaration parameterDeclaration = new CPPASTParameterDeclaration();
 			IASTDeclarator parameterDeclarator = fieldDeclarator.copy(CopyStyle.withLocations);
 			parameterDeclarator.setName(getSetterParameterName());

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<IASTNode> getterAndSetters = new ArrayList<IASTNode>();
 		List<IASTFunctionDefinition> definitions = new ArrayList<IASTFunctionDefinition>();
+		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);