diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts index 683aa03e439..bac0e169ad0 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts +++ b/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_ */ diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterContext.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterContext.java index 6cd78af1bd2..09697a87b08 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterContext.java +++ b/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 getWrappedFields() { if (wrappedFields == null) { wrappedFields = new ArrayList(); - for (IASTSimpleDeclaration currentField : existingFields) { + for (IASTSimpleDeclaration field : existingFields) { FieldWrapper wrapper = new FieldWrapper(); - wrapper.field = currentField; - wrapper.getter = getGetterForField(currentField); - wrapper.setter = getSetterForField(currentField); + wrapper.field = field; + wrapper.getter = getGetterForField(field); + 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) { - IASTDeclarator declarator = fieldDeclaration.getDeclarators()[0]; + private FunctionWrapper getSetterForField(IASTSimpleDeclaration field) { + 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) { - FunctionWrapper wrapper = new FunctionWrapper(); - String name = GetterSetterNameGenerator.generateSetterName(getFieldDeclarationName(currentField)); - setFunctionToWrapper(wrapper, name); - return wrapper; + + private static boolean isAssignable(IASTSimpleDeclaration declaration) { + IASTName name = getDeclarationName(declaration); + IBinding binding = name.resolveBinding(); + if (!(binding instanceof ICPPField)) + 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; } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterFactory.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterFactory.java index a428917cd12..070d7c2d1f9 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GetterSetterFactory.java +++ b/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;