From 76f05660346863d2fa73381cd6a71d6ca665c7f2 Mon Sep 17 00:00:00 2001 From: Markus Schorn Date: Thu, 15 May 2008 14:47:11 +0000 Subject: [PATCH] Fixes remaining failures of implement method, bug 226646. --- .../resources/refactoring/ImplementMethod.rts | 266 +++++++++--------- core/org.eclipse.cdt.ui/plugin.properties | 2 +- .../ui/refactoring/dialogs/Messages.java | 1 + .../dialogs/ValidatingLabeledTextField.java | 28 +- .../refactoring/dialogs/messages.properties | 3 + .../ImplementMethodRefactoring.java | 93 ++---- .../ImplementMethodRefactoringWizard.java | 4 +- .../implementmethod/InsertLocation.java | 12 +- .../MethodDefinitionInsertLocationFinder.java | 7 +- .../implementmethod/Parameter.java | 27 ++ .../implementmethod/ParameterHandler.java | 77 +++++ .../ParameterNamesInputPage.java | 45 ++- .../refactoring/actions/messages.properties | 2 +- 13 files changed, 324 insertions(+), 243 deletions(-) create mode 100644 core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Parameter.java create mode 100644 core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterHandler.java diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts index 7d4b3ce4e0d..59577185e1f 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts @@ -1,3 +1,116 @@ +//!class template member functions +//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest +//@.config +filename=A.h +//@A.h +template class A +{ +public: + A(); + //$void test();$// +}; + +template A::A() +{ +} + +//= +template class A +{ +public: + A(); + void test(); +}; + +template A::A() +{ +} + +template inline void A::test() +{ +} + + + +//!member class +//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest +//@.config +filename=A.h +//@A.h + +class Demo +{ + class SubClass + { + //$void test();$// + }; +}; + + +//@A.cpp +#include "A.h" + +//= +#include "A.h" + +void Demo::SubClass::test() +{ +} + + + +//!method declared in otherwise empty class without cpp file +//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest +//@.config +filename=A.h +//@A.h +class A +{ +public: + //$void test();$// +}; + +//= +class A +{ +public: + void test(); +}; + +inline void A::test() +{ +} + + + +//!method declared in otherwise empty class +//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest +//@.config +filename=A.h +//@A.h +class A +{ +public: + //$void test();$// +}; + +//= +class A +{ +public: + void test(); +}; + +//@A.cpp + +//= + + +void A::test() +{ +} + + //!implement in existing namespace //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -33,38 +146,10 @@ namespace NameSpace return 5; } - void ClassInNamespace::test2() - { - - } + void ClassInNamespace::test2() + { + } } -//!method declared in otherwise empty class -//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest -//@.config -filename=A.h -//@A.h -class A -{ -public: - //$void test();$// -}; - -//= -class A -{ -public: - void test(); -}; - -//@A.cpp - -//= - -void A::test() -{ - -} - //!virtual method in the middle of con/destructor, without parameters and void return value //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -127,29 +212,6 @@ void function() void function_with_impl() { } -//!method declared in otherwise empty class without cpp file -//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest -//@.config -filename=A.h -//@A.h -class A -{ -public: - //$void test();$// -}; - -//= -class A -{ -public: - void test(); -}; - -inline void A::test() -{ - -} - //!method at end, without parameters and void return value //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -182,9 +244,10 @@ A::A() void A::foo() { - } + + //!method at beginning, without parameters, void return value and const //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -251,9 +314,10 @@ A::A() int A::foo() { - } + + //!method with two int parameters //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -286,9 +350,10 @@ A::A() int A::foo(int param1, int param2) { - } + + //!method defined in header //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -319,9 +384,10 @@ A::A() inline void A::test() { - } + + //!implement a function at end of source file //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -340,8 +406,9 @@ void function_with_impl() void function() { - } + + //!implement with namespace //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -370,8 +437,9 @@ void NameSpace::ClassInNamespace::other_test() void NameSpace::ClassInNamespace::test() { - } + + //!implement function within namespace //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -399,10 +467,9 @@ namespace OuterSpace { { } - int NameSpace::test2() - { - - } + int NameSpace::test2() + { + } } //!implement function within namespaces //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest @@ -434,45 +501,11 @@ namespace OuterSpace { { } - int test2() - { - - } + int test2() + { + } } } -//!class template member functions -//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest -//@.config -filename=A.h -//@A.h -template class A -{ -public: - A(); - //$void test();$// -}; - -template A::A() -{ -} - -//= -template class A -{ -public: - A(); - void test(); -}; - -template A::A() -{ -} - -template inline void A::test() -{ - -} - //!class template member functions with multiple templates //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -503,9 +536,10 @@ template A::A() template inline void A::test() { - } + + //!with default parameters //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -527,9 +561,10 @@ public: void Class::test(int param1, int param2, int param3) { - } + + //!static method //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config @@ -551,32 +586,7 @@ public: void Class::test() { - } -//!member class -//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest -//@.config -filename=A.h -//@A.h - -class Demo -{ - class SubClass - { - //$void test();$// - }; -}; -//@A.cpp -#include "A.h" - -//= -#include "A.h" - -void Demo::SubClass::test() -{ - -} - diff --git a/core/org.eclipse.cdt.ui/plugin.properties b/core/org.eclipse.cdt.ui/plugin.properties index 1d4804fb00b..2b9955d55e1 100644 --- a/core/org.eclipse.cdt.ui/plugin.properties +++ b/core/org.eclipse.cdt.ui/plugin.properties @@ -160,7 +160,7 @@ Refactoring.renameAction.label=Re&name... Refactoring.extractConstant.label=Extr&act Constant... Refactoring.extractFunction.label=Extract &Function... (work in progress) Refactoring.hideMethod.label=Hide Method... -Refactoring.implementMethod.label=Impl&ement Method... (work in progress) +Refactoring.implementMethod.label=Impl&ement Method... Refactoring.gettersAndSetters.label=Generate Getters and Setters... CEditor.name=C/C++ Editor diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/Messages.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/Messages.java index 0915a7d1202..4e28c25af54 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/Messages.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/Messages.java @@ -30,6 +30,7 @@ public final class Messages extends NLS { public static String ValidatingLabeledTextField_CantBeEmpty; public static String ValidatingLabeledTextField_InvalidCharacters; public static String ValidatingLabeledTextField_DuplicatedNames; + public static String ValidatingLabeledTextField_IsKeyword; static { NLS.initializeMessages(BUNDLE_NAME, Messages.class); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/ValidatingLabeledTextField.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/ValidatingLabeledTextField.java index 364bae35910..14762df70a7 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/ValidatingLabeledTextField.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/ValidatingLabeledTextField.java @@ -30,6 +30,8 @@ import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Listener; import org.eclipse.swt.widgets.Text; +import org.eclipse.cdt.internal.ui.refactoring.utils.NameHelper; + /** * @author Mirko Stocker @@ -66,7 +68,7 @@ public class ValidatingLabeledTextField extends Composite { * @param text the new value of the field * @return whether the value is valid or not */ - public abstract boolean isValidInput(String text); + public boolean isValidInput(String text) { return true; } public String errorMessageForEmptyField() { return Messages.ValidatingLabeledTextField_CantBeEmpty; @@ -79,6 +81,10 @@ public class ValidatingLabeledTextField extends Composite { public String errorMessageForDuplicateValues() { return Messages.ValidatingLabeledTextField_DuplicatedNames; } + + public String errorIsKeywordMessage() { + return Messages.ValidatingLabeledTextField_IsKeyword; + } } public ValidatingLabeledTextField(Composite parent, int style) { @@ -133,20 +139,24 @@ public class ValidatingLabeledTextField extends Composite { boolean isNameAlreadyInUse = nameAlreadyInUse(textField, newName); boolean isValid = validator.isValidInput(newName); - - if (isValid && !isNameAlreadyInUse && !isEmpty) { + boolean isKeyword = NameHelper.isKeyword(newName); + boolean isValidName = NameHelper.isValidLocalVariableName(newName); + + boolean isOk = isValid && !isNameAlreadyInUse && !isEmpty && !isKeyword && isValidName; + if (isOk) { setErrorStatus(EMPTY_STRING); } else if (isEmpty) { setErrorStatus(validator.errorMessageForEmptyField()); - } else if (!isValid) { + } else if (!isValid || !isValidName) { setErrorStatus(validator.errorMessageForInvalidInput()); - } else if (isNameAlreadyInUse) { + } else if (isKeyword) { + setErrorStatus(validator.errorIsKeywordMessage()); + }else if (isNameAlreadyInUse) { setErrorStatus(validator.errorMessageForDuplicateValues()); - } + } + validationStatus.put(textField, isOk); - validationStatus.put(textField, Boolean.valueOf(isValid && !isNameAlreadyInUse && !isEmpty)); - - if(validationStatus.values().contains(Boolean.FALSE) || isEmpty || isNameAlreadyInUse) { + if(validationStatus.values().contains(Boolean.FALSE) || isEmpty || isNameAlreadyInUse || isKeyword || !isValidName) { validator.hasErrors(); } else { validator.hasNoErrors(); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/messages.properties b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/messages.properties index 8a340fe4d45..e51e6e68916 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/messages.properties +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/dialogs/messages.properties @@ -18,3 +18,6 @@ VisibilitySelectionPanel_AccessModifier=&Access modifier: ValidatingLabeledTextField_CantBeEmpty=Cannot be empty ValidatingLabeledTextField_InvalidCharacters=Invalid characters ValidatingLabeledTextField_DuplicatedNames=Duplicated name +ValidatingLabeledTextField_IsKeyword=Keyword +ValidatingLabeledTextField_IsKeyword=Keyword +ValidatingLabeledTextField_IsKeyword=Keyword diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java index 121fa84341f..bed9e8fa686 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java @@ -11,9 +11,6 @@ *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.implementmethod; -import java.util.HashSet; -import java.util.Set; - import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; @@ -32,7 +29,6 @@ import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamedTypeSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateParameter; @@ -42,15 +38,14 @@ import org.eclipse.cdt.core.model.ICElement; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTCompoundStatement; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionDeclarator; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionDefinition; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateDeclaration; import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; import org.eclipse.cdt.internal.ui.refactoring.utils.DefinitionFinder; import org.eclipse.cdt.internal.ui.refactoring.utils.NameHelper; +import org.eclipse.cdt.internal.ui.refactoring.utils.NodeFactory; import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper; -import org.eclipse.cdt.internal.ui.refactoring.utils.PseudoNameGenerator; import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper; /** @@ -64,31 +59,16 @@ public class ImplementMethodRefactoring extends CRefactoring { private IASTSimpleDeclaration methodDeclaration; private InsertLocation insertLocation; - private final Set generatedNames = new HashSet(); + private ParameterHandler parameterHandler; public ImplementMethodRefactoring(IFile file, ISelection selection, ICElement element) { super(file, selection, element); - } - - public boolean needsAdditionalArgumentNames() { - for (IASTParameterDeclaration parameterDeclaration : getParameters()) { - if(parameterDeclaration.getDeclarator().getName().toCharArray().length < 1) { - return true; - } - } - return false; - } - - public IASTParameterDeclaration[] getParameters() { - if(methodDeclaration.getDeclarators().length < 1) { - return null; - } - return ((ICPPASTFunctionDeclarator) methodDeclaration.getDeclarators()[0]).getParameters(); + parameterHandler = new ParameterHandler(this); } @Override public RefactoringStatus checkInitialConditions(IProgressMonitor pm) throws CoreException, OperationCanceledException { - SubMonitor sm = SubMonitor.convert(pm, 8); + SubMonitor sm = SubMonitor.convert(pm, 10); super.checkInitialConditions(sm.newChild(6)); methodDeclaration = SelectionHelper.findFirstSelectedDeclaration(region, unit); @@ -108,14 +88,19 @@ public class ImplementMethodRefactoring extends CRefactoring { if(isProgressMonitorCanceld(sm, initStatus))return initStatus; sm.worked(1); sm.done(); + parameterHandler.initAditionalArgumentNames(); + sm.worked(1); + sm.done(); + findInsertLocation(); + sm.worked(1); + sm.done(); return initStatus; } @Override protected void collectModifications(IProgressMonitor pm, ModificationCollector collector) throws CoreException, OperationCanceledException { - findInsertLocation(); - IASTTranslationUnit targetUnit = insertLocation.getTargetTranslationUnit(); + IASTTranslationUnit targetUnit = insertLocation.getTargetTranslationUnit(); IASTNode parent = insertLocation.getPartenOfNodeToInsertBefore(); IASTNode insertNode = createFunctionDefinition(targetUnit); IASTNode nodeToInsertBefore = insertLocation.getNodeToInsertBefore(); @@ -124,9 +109,7 @@ public class ImplementMethodRefactoring extends CRefactoring { } private void findInsertLocation() throws CoreException { - insertLocation = MethodDefinitionInsertLocationFinder.find( - methodDeclaration.getFileLocation(), methodDeclaration - .getParent(), file); + insertLocation = MethodDefinitionInsertLocationFinder.find(methodDeclaration.getFileLocation(), methodDeclaration.getParent(), file); if (!insertLocation.hasFile()) { insertLocation.setInsertFile(file); @@ -155,7 +138,8 @@ public class ImplementMethodRefactoring extends CRefactoring { ((ICPPASTDeclSpecifier) declSpecifier).setVirtual(false); } - if(Path.fromOSString(methodDeclaration.getNodeLocations()[0].asFileLocation().getFileName()).equals(insertLocation.getInsertFile().getLocation())) { + String currentFileName = methodDeclaration.getNodeLocations()[0].asFileLocation().getFileName(); + if(Path.fromOSString(currentFileName).equals(insertLocation.getInsertFile().getLocation())) { declSpecifier.setInline(true); } @@ -170,35 +154,13 @@ public class ImplementMethodRefactoring extends CRefactoring { CPPASTFunctionDeclarator newFunctionDeclarator = new CPPASTFunctionDeclarator(); newFunctionDeclarator.setName(qname); newFunctionDeclarator.setConst(functionDeclarator.isConst()); + - PseudoNameGenerator pseudoNameGenerator = new PseudoNameGenerator(); - - for(IASTParameterDeclaration parameter : getParameters()) { - if(parameter.getDeclarator().getName().toString().length() > 0) { - pseudoNameGenerator.addExistingName(parameter.getDeclarator().getName().toString()); - } - } - - for(IASTParameterDeclaration parameter : getParameters()) { - if(parameter.getDeclarator().getName().toString().length() < 1) { - - IASTDeclSpecifier parameterDeclSpecifier = parameter.getDeclSpecifier(); - String typeName; - if(parameterDeclSpecifier instanceof ICPPASTNamedTypeSpecifier) { - typeName = ((ICPPASTNamedTypeSpecifier) parameterDeclSpecifier).getName().getRawSignature(); - } else { - typeName = parameterDeclSpecifier.getRawSignature(); - } - - String generateNewName = pseudoNameGenerator.generateNewName(typeName); - generatedNames.add(generateNewName); - parameter.getDeclarator().setName(new CPPASTName(generateNewName.toCharArray())); - } - newFunctionDeclarator.addParameterDeclaration(parameter); + for(Parameter actParameter : parameterHandler.getParameters()) { + IASTParameterDeclaration createdParam = NodeFactory.createParameterDeclaration(actParameter.typeName, actParameter.parameterName); + newFunctionDeclarator.addParameterDeclaration(createdParam); } - removeAllDefaultParameters(newFunctionDeclarator); - func.setDeclarator(newFunctionDeclarator); func.setBody(new CPPASTCompoundStatement()); @@ -213,24 +175,11 @@ public class ImplementMethodRefactoring extends CRefactoring { templateDeclaration.setDeclaration(func); return templateDeclaration; } - return func; } - private void removeAllDefaultParameters(ICPPASTFunctionDeclarator functionDeclarator) { - for (IASTParameterDeclaration parameterDeclaration : functionDeclarator.getParameters()) { - parameterDeclaration.getDeclarator().setInitializer(null); - } - } - private ICPPASTQualifiedName createQualifiedNameFor(IASTFunctionDeclarator functionDeclarator, IASTNode declarationParent) { - - int insertOffset; - if(insertLocation.getNodeToInsertBefore() == null) { - insertOffset = 0; - } else { - insertOffset = insertLocation.getPartenOfNodeToInsertBefore().getFileLocation().getNodeOffset(); - } + int insertOffset = insertLocation.getInsertPosition(); return NameHelper.createQualifiedNameFor(functionDeclarator.getName(), file, region.getOffset(), insertLocation.getInsertFile(), insertOffset); } @@ -242,7 +191,7 @@ public class ImplementMethodRefactoring extends CRefactoring { return methodDeclaration; } - public Set getGeneratedNames() { - return generatedNames; + public ParameterHandler getParameterHandler() { + return parameterHandler; } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java index 453bcf3e0fe..44ec71d2c39 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java @@ -28,8 +28,8 @@ public class ImplementMethodRefactoringWizard extends RefactoringWizard { @Override protected void addUserInputPages() { - if(refactoring.needsAdditionalArgumentNames()) { - addPage(new ParameterNamesInputPage(refactoring)); + if(refactoring.getParameterHandler().needsAdditionalArgumentNames()) { + addPage(new ParameterNamesInputPage(refactoring.getParameterHandler())); } } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java index 1301ce8fc57..b653cdd0a05 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/InsertLocation.java @@ -33,7 +33,7 @@ public class InsertLocation { private IASTTranslationUnit targetTranslationUnit; public boolean hasAnyNode() { - return nodeToInsertAfter == null && nodeToInsertBefore == null; + return nodeToInsertAfter != null || nodeToInsertBefore != null; } public IASTNode getNodeToInsertBefore() { @@ -86,4 +86,14 @@ public class InsertLocation { targetTranslationUnit = TranslationUnitHelper.loadTranslationUnit(insertFile); } } + + public int getInsertPosition() { + if(nodeToInsertBefore != null) { + return nodeToInsertBefore.getFileLocation().getNodeOffset(); + } else if (nodeToInsertAfter != null) { + return nodeToInsertAfter.getFileLocation().getNodeOffset() + nodeToInsertAfter.getFileLocation().getNodeLength(); + } else { + return 0; + } + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java index 3cbb350b383..497871f3335 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder.java @@ -27,6 +27,7 @@ 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.IASTSimpleDeclaration; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.internal.ui.refactoring.utils.DefinitionFinder; import org.eclipse.cdt.internal.ui.refactoring.utils.FileHelper; @@ -44,6 +45,9 @@ public class MethodDefinitionInsertLocationFinder { if(node == null) { return null; } else if(node instanceof IASTFunctionDefinition) { + if(node.getParent() instanceof ICPPASTTemplateDeclaration) { + node = node.getParent(); + } return node; } return findFunctionDefinitionInParents(node.getParent()); @@ -82,9 +86,6 @@ public class MethodDefinitionInsertLocationFinder { } } - if(!result.hasAnyNode()) { - return result; - } IPath path = file.getLocation().removeFileExtension().addFileExtension("cpp"); //$NON-NLS-1$ IFile fileForLocation = ResourcesPlugin.getWorkspace().getRoot().getFileForLocation(path); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Parameter.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Parameter.java new file mode 100644 index 00000000000..426b5029387 --- /dev/null +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Parameter.java @@ -0,0 +1,27 @@ +/******************************************************************************* + * Copyright (c) 2008 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 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Institute for Software - initial API and implementation + *******************************************************************************/ +package org.eclipse.cdt.internal.ui.refactoring.implementmethod; + +/** + * @author Lukas Felber + * + */ +public class Parameter { + public Parameter(String typeName, String parameterName, boolean isChangable) { + this.typeName = typeName; + this.parameterName = parameterName; + this.isChangable = isChangable; + } + public String typeName; + public String parameterName; + public boolean isChangable; +} diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterHandler.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterHandler.java new file mode 100644 index 00000000000..50d8bc24dfc --- /dev/null +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterHandler.java @@ -0,0 +1,77 @@ +package org.eclipse.cdt.internal.ui.refactoring.implementmethod; + +import java.util.ArrayList; +import java.util.Collection; + +import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator; + +import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriter; + +import org.eclipse.cdt.internal.ui.refactoring.utils.NameHelper; +import org.eclipse.cdt.internal.ui.refactoring.utils.PseudoNameGenerator; + +public class ParameterHandler { + private boolean needsAditionalArgumentNames; + private PseudoNameGenerator pseudoNameGenerator; + private ArrayList parameters; + private ImplementMethodRefactoring refactoring; + + public ParameterHandler(ImplementMethodRefactoring refactoring) { + this.refactoring = refactoring; + } + + public boolean needsAdditionalArgumentNames() { + return needsAditionalArgumentNames; + } + + public void initAditionalArgumentNames() { + if(parameters != null) { + return; + } + needsAditionalArgumentNames = false; + parameters = new ArrayList(); + for(IASTParameterDeclaration actParam : getParametersFromMethodNode()) { + String actName = actParam.getDeclarator().getName().toString(); + boolean isChangable = false; + String typeName = NameHelper.getTypeName(actParam); + if(actName.length() == 0) { + needsAditionalArgumentNames = true; + isChangable = true; + actName = findNameForParameter(typeName); + } + parameters.add(new Parameter(typeName, actName, isChangable)); + } + } + + private String findNameForParameter(String typeName) { + if(pseudoNameGenerator == null) { + pseudoNameGenerator = new PseudoNameGenerator(); + + for(IASTParameterDeclaration parameter : getParametersFromMethodNode()) { + if(parameter.getDeclarator().getName().toString().length() != 0) { + pseudoNameGenerator.addExistingName(parameter.getDeclarator().getName().toString()); + } + } + } + return pseudoNameGenerator.generateNewName(typeName); + } + + private IASTParameterDeclaration[] getParametersFromMethodNode() { + if(refactoring.getMethodDeclaration().getDeclarators().length < 1) { + return null; + } + return ((ICPPASTFunctionDeclarator) refactoring.getMethodDeclaration().getDeclarators()[0]).getParameters(); + } + + public String createFunctionDefinitionSignature() { + ASTWriter writer = new ASTWriter(); + IASTNode def = refactoring.createFunctionDefinition(); + return writer.write(def); + } + + public Collection getParameters() { + return parameters; + } +} diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java index 01a6361e246..c0b106542c3 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java @@ -21,29 +21,23 @@ import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Label; -import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration; - -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; - import org.eclipse.cdt.internal.ui.preferences.formatter.TranslationUnitPreview; import org.eclipse.cdt.internal.ui.refactoring.dialogs.ValidatingLabeledTextField; -import org.eclipse.cdt.internal.ui.refactoring.utils.NameHelper; /** - * InputPage used by the ImplementMethod refactoring if its necessary to enteraditional parameter names. + * InputPage used by the ImplementMethod refactoring if its necessary to enter additional parameter names. * * @author Mirko Stocker * */ public class ParameterNamesInputPage extends UserInputWizardPage { - private final ImplementMethodRefactoring refactoring; - + private final ParameterHandler parameterHandler; private TranslationUnitPreview translationUnitPreview; - public ParameterNamesInputPage(ImplementMethodRefactoring implementMethodRefactoring) { + public ParameterNamesInputPage(ParameterHandler parameterHandler) { super(Messages.ParameterNamesInputPage_Title); - this.refactoring = implementMethodRefactoring; + this.parameterHandler = parameterHandler; } public void createControl(Composite parent) { @@ -59,13 +53,13 @@ public class ParameterNamesInputPage extends UserInputWizardPage { ValidatingLabeledTextField validatingLabeledTextField = new ValidatingLabeledTextField(superComposite); validatingLabeledTextField.setLayoutData(new GridData(GridData.GRAB_HORIZONTAL | GridData.HORIZONTAL_ALIGN_FILL)); - for (final IASTParameterDeclaration parameterDeclaration : refactoring.getParameters()) { + for (final Parameter actParameter : parameterHandler.getParameters()) { - String type = parameterDeclaration.getDeclSpecifier().getRawSignature(); - String content = String.valueOf(parameterDeclaration.getDeclarator().getName().toCharArray()); - boolean enabled = parameterDeclaration.getDeclarator().getName().toCharArray().length > 0; + String type = actParameter.typeName; + String content = actParameter.parameterName; + boolean readOnly = !actParameter.isChangable; - validatingLabeledTextField.addElement(type, content, enabled, new ValidatingLabeledTextField.Validator(){ + validatingLabeledTextField.addElement(type, content, readOnly, new ValidatingLabeledTextField.Validator(){ @Override public void hasErrors() { @@ -79,23 +73,22 @@ public class ParameterNamesInputPage extends UserInputWizardPage { @Override public boolean isValidInput(String newName) { - boolean isValid = NameHelper.isValidLocalVariableName(newName); - - if(isValid) { - parameterDeclaration.getDeclarator().setName(new CPPASTName(newName.toCharArray())); - translationUnitPreview.setPreviewText(refactoring.createFunctionDefinition().getRawSignature()); - } - - return isValid; + actParameter.parameterName = newName; + updatePreview(); + return true; }}); } translationUnitPreview = new TranslationUnitPreview(new HashMap(), superComposite); translationUnitPreview.getControl().setLayoutData(new GridData(GridData.GRAB_HORIZONTAL | GridData.HORIZONTAL_ALIGN_FILL)); - translationUnitPreview.setPreviewText(refactoring.createFunctionDefinition().getRawSignature()); + updatePreview(); setControl(superComposite); - - setPageComplete(false); + } + + private void updatePreview() { + if (translationUnitPreview != null) { + translationUnitPreview.setPreviewText(parameterHandler.createFunctionDefinitionSignature()); + } } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/messages.properties b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/messages.properties index 5257a3fcb90..e61fe62b9e7 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/messages.properties +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/messages.properties @@ -12,6 +12,6 @@ CRefactoringActionGroup_menu=Refactor CRenameAction_label=Rename... ExtractConstantAction_label=Extract Constant... GettersAndSetters_label=Generate Getters and Setters... -ImplementMethodAction_label=Implement Method... (work in progress) +ImplementMethodAction_label=Implement Method... HideMethodAction_label=Hide Method... ExtractFunctionAction_label=Extract Function... (work in progress)