Bug 319278 - Generate getters and setters handles arrays incorrectly.

core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts

@@ -1451,3 +1451,31 @@ public:
 	int test;
 };
 #endif /* A_H_ */
+//!Bug ??? - Getter for an array field
+//#org.eclipse.cdt.ui.tests.refactoring.gettersandsetters.GenerateGettersAndSettersTest
+//@.config
+filename=A.h
+getters=a
+//@A.h
+#ifndef A_H_
+#define A_H_
+
+class A {
+private:
+	int /*$*/a[2]/*$$*/;
+};
+#endif /* A_H_ */
+//=
+#ifndef A_H_
+#define A_H_
+
+class A {
+private:
+	int a[2];
+
+public:
+	const int* getA() const {
+		return a;
+	}
+};
+#endif /* A_H_ */

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterContext.java

@@ -23,6 +23,15 @@ import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration;
+import org.eclipse.cdt.core.dom.ast.IArrayType;
+import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.core.dom.ast.IPointerType;
+import org.eclipse.cdt.core.dom.ast.IQualifierType;
+import org.eclipse.cdt.core.dom.ast.IType;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPField;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPReferenceType;
+
+import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil;
 
 import org.eclipse.cdt.internal.ui.refactoring.gettersandsetters.GetterSetterInsertEditProvider.AccessorKind;
 
@@ -45,7 +54,7 @@ public class GetterSetterContext implements ITreeContentProvider {
 				if (!wrapper.getter.exists()) {
 					wrapper.childNodes.add(createGetterInserter(wrapper.field));
 				}
-				if (!wrapper.setter.exists() && !wrapper.field.getDeclSpecifier().isConst()) {
+				if (!wrapper.setter.exists() && isAssignable(wrapper.field)) {
 					wrapper.childNodes.add(createSetterInserter(wrapper.field));
 				}
 			}
@@ -55,12 +64,12 @@ public class GetterSetterContext implements ITreeContentProvider {
 	}
 
 	public GetterSetterInsertEditProvider createGetterInserter(IASTSimpleDeclaration simpleDeclaration) {
-		IASTName fieldName = getFieldDeclarationName(simpleDeclaration);
+		IASTName fieldName = getDeclarationName(simpleDeclaration);
 		return new GetterSetterInsertEditProvider(fieldName, simpleDeclaration, AccessorKind.GETTER);
 	}
 
 	public GetterSetterInsertEditProvider createSetterInserter(IASTSimpleDeclaration simpleDeclaration) {
-		IASTName fieldName = getFieldDeclarationName(simpleDeclaration);
+		IASTName fieldName = getDeclarationName(simpleDeclaration);
 		return new GetterSetterInsertEditProvider(fieldName, simpleDeclaration, AccessorKind.SETTER);
 	}
 
@@ -115,11 +124,11 @@ public class GetterSetterContext implements ITreeContentProvider {
 	private ArrayList<FieldWrapper> getWrappedFields() {
 		if (wrappedFields == null) {
 			wrappedFields = new ArrayList<FieldWrapper>();
-			for (IASTSimpleDeclaration currentField : existingFields) {
+			for (IASTSimpleDeclaration field : existingFields) {
 				FieldWrapper wrapper = new FieldWrapper();
-				wrapper.field = currentField;
+				wrapper.field = field;
-				wrapper.getter = getGetterForField(currentField);
+				wrapper.getter = getGetterForField(field);
-				wrapper.setter = getSetterForField(currentField);
+				wrapper.setter = getSetterForField(field);
 				if (wrapper.missingGetterOrSetter()) {
 					wrappedFields.add(wrapper);
 				}
@@ -130,35 +139,52 @@ public class GetterSetterContext implements ITreeContentProvider {
 
 	private FunctionWrapper getGetterForField(IASTSimpleDeclaration currentField) {
 		FunctionWrapper wrapper = new FunctionWrapper();
-		String name = GetterSetterNameGenerator.generateGetterName(getFieldDeclarationName(currentField));
+		String name = GetterSetterNameGenerator.generateGetterName(getDeclarationName(currentField));
 		setFunctionToWrapper(wrapper, name);
 		return wrapper;
 	}
 
-	private IASTName getFieldDeclarationName(IASTSimpleDeclaration fieldDeclaration) {
+	private FunctionWrapper getSetterForField(IASTSimpleDeclaration field) {
-		IASTDeclarator declarator = fieldDeclaration.getDeclarators()[0];
+		FunctionWrapper wrapper = new FunctionWrapper();
+		String name = GetterSetterNameGenerator.generateSetterName(getDeclarationName(field));
+		setFunctionToWrapper(wrapper, name);
+		return wrapper;
+	}
+
+	private static IASTName getDeclarationName(IASTSimpleDeclaration declaration) {
+		IASTDeclarator declarator = declaration.getDeclarators()[0];
 		while (declarator.getNestedDeclarator() != null) {
 			declarator = declarator.getNestedDeclarator();
 		}
 		return declarator.getName();
 	}
-	
+
-	private FunctionWrapper getSetterForField(IASTSimpleDeclaration currentField) {
+	private static boolean isAssignable(IASTSimpleDeclaration declaration) {
-		FunctionWrapper wrapper = new FunctionWrapper();
+		IASTName name = getDeclarationName(declaration);
-		String name = GetterSetterNameGenerator.generateSetterName(getFieldDeclarationName(currentField));
+		IBinding binding = name.resolveBinding();
-		setFunctionToWrapper(wrapper, name);
+		if (!(binding instanceof ICPPField))
-		return wrapper;
+			return false;
+		ICPPField field = (ICPPField) binding;
+		IType type = field.getType();
+		type = SemanticUtil.getNestedType(type, SemanticUtil.TDEF);
+		if (type instanceof IArrayType || type instanceof ICPPReferenceType)
+			return false;
+		if (type instanceof IPointerType && ((IPointerType) type).isConst())
+			return false;
+		if (type instanceof IQualifierType && ((IQualifierType) type).isConst())
+			return false;
+		return true;
 	}
 
-	private void setFunctionToWrapper(FunctionWrapper wrapper, String getterName) {
+	private void setFunctionToWrapper(FunctionWrapper wrapper, String accessorName) {
 		for (IASTFunctionDefinition currentDefinition : existingFunctionDefinitions) {
-			if (currentDefinition.getDeclarator().getName().toString().endsWith(getterName)) {
+			if (currentDefinition.getDeclarator().getName().toString().equals(accessorName)) {
 				wrapper.functionDefinition = currentDefinition;
 			}
 		}
 		
 		for (IASTSimpleDeclaration currentDeclaration : existingFunctionDeclarations) {
-			if (getFieldDeclarationName(currentDeclaration).toString().endsWith(getterName)) {
+			if (getDeclarationName(currentDeclaration).toString().equals(accessorName)) {
 				wrapper.functionDeclaration = currentDeclaration;
 			}
 		}

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterFactory.java

@@ -14,6 +14,7 @@ package org.eclipse.cdt.internal.ui.refactoring.gettersandsetters;
 
 import java.util.Arrays;
 
+import org.eclipse.cdt.core.dom.ast.IASTArrayDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
@@ -25,12 +26,16 @@ import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
 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;
 
+import org.eclipse.cdt.internal.core.dom.parser.c.CASTDeclarator;
+import org.eclipse.cdt.internal.core.dom.parser.c.CASTPointer;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTBinaryExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTCompoundStatement;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarator;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTExpressionStatement;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFieldReference;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionDeclarator;
@@ -39,6 +44,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTIdExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTLiteralExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTParameterDeclaration;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTPointer;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTReferenceOperator;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTReturnStatement;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclSpecifier;
@@ -90,12 +96,23 @@ public class GetterSetterFactory {
 		// Copy declarator hierarchy
 		IASTDeclarator topDeclarator = fieldDeclaration.getDeclarators()[0].copy(CopyStyle.withLocations);
 		
+		if (topDeclarator instanceof IASTArrayDeclarator) {
+			boolean isCpp = topDeclarator instanceof ICPPASTArrayDeclarator;
+			IASTDeclarator decl = isCpp ? new CPPASTDeclarator() : new CASTDeclarator();
+			decl.setName(topDeclarator.getName());
+			decl.setNestedDeclarator(topDeclarator.getNestedDeclarator());
+			decl.addPointerOperator(isCpp ? new CPPASTPointer() : new CASTPointer());
+			for (IASTPointerOperator pointer : topDeclarator.getPointerOperators()) {
+				decl.addPointerOperator(pointer);
+			}
+			topDeclarator = decl;
+		}
 		// Find the innermost declarator in hierarchy
 		IASTDeclarator innermost = topDeclarator;
 		while (innermost.getNestedDeclarator() != null) {
 			innermost = innermost.getNestedDeclarator();
 		}
-		
+
 		// Create a new innermost function declarator based on the field declarator 
 		CPPASTFunctionDeclarator functionDeclarator = new CPPASTFunctionDeclarator();
 		functionDeclarator.setConst(true);
@@ -203,7 +220,7 @@ public class GetterSetterFactory {
 
 	private IASTDeclSpecifier getParamOrReturnDeclSpecifier() {
 		IASTDeclSpecifier declSpec = fieldDeclaration.getDeclSpecifier().copy(CopyStyle.withLocations);
-		if (passByReference) {
+		if (passByReference || fieldDeclaration.getDeclarators()[0] instanceof IASTArrayDeclarator) {
 			declSpec.setConst(true);
 		}
 		return declSpec;