Fixes remaining failures of implement method, bug 226646.

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 T> class A
+{
+public:
+        A();
+        //$void test();$//
+};
+
+template<class T> A<T>::A()
+{
+}
+
+//=
+template<class T> class A
+{
+public:
+        A();
+        void test();
+};
+
+template<class T> A<T>::A()
+{
+}
+
+template<class T> inline void A<T>::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
@@ -35,36 +148,8 @@ namespace NameSpace
 
     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
@@ -401,7 +469,6 @@ namespace OuterSpace {
 
     int NameSpace::test2()
     {
-		
     }
 }
 //!implement function within namespaces
@@ -436,43 +503,9 @@ namespace OuterSpace {
 
 	    int test2()
 	    {
-			
 	    }
 	}
 }
-//!class template member functions
-//#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest
-//@.config
-filename=A.h
-//@A.h
-template<class T> class A
-{
-public:
-        A();
-        //$void test();$//
-};
-
-template<class T> A<T>::A()
-{
-}
-
-//=
-template<class T> class A
-{
-public:
-        A();
-        void test();
-};
-
-template<class T> A<T>::A()
-{
-}
-
-template<class T> inline void A<T>::test()
-{
-	
-}
-
 //!class template member functions with multiple templates
 //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest
 //@.config
@@ -503,9 +536,10 @@ template<class T, class U> A<T,U>::A()
 
 template<class T, class U> inline void A<T,U>::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()
-{
-	
-}
-

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

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);

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);
+				boolean isKeyword = NameHelper.isKeyword(newName);
+				boolean isValidName = NameHelper.isValidLocalVariableName(newName);
 
-				if (isValid && !isNameAlreadyInUse && !isEmpty) {
+				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 (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 || isKeyword || !isValidName) {
-				
-				if(validationStatus.values().contains(Boolean.FALSE) || isEmpty || isNameAlreadyInUse) {
 					validator.hasErrors();
 				} else {
 					validator.hasNoErrors();

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

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<String> generatedNames = new HashSet<String>();
+	private ParameterHandler parameterHandler;
 	
 	public ImplementMethodRefactoring(IFile file, ISelection selection, ICElement element) {
 		super(file, selection, element);
-	}
+		parameterHandler = new ParameterHandler(this);
-	
-	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();
 	}
 	
 	@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,13 +88,18 @@ 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();
  		IASTNode parent = insertLocation.getPartenOfNodeToInsertBefore();
 		IASTNode insertNode = createFunctionDefinition(targetUnit);
@@ -124,9 +109,7 @@ public class ImplementMethodRefactoring extends CRefactoring {
 	}
 
 	private void findInsertLocation() throws CoreException {
-		insertLocation = MethodDefinitionInsertLocationFinder.find(
+		insertLocation = MethodDefinitionInsertLocationFinder.find(methodDeclaration.getFileLocation(), methodDeclaration.getParent(), file);
-				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);
 		}
 		
@@ -171,33 +155,11 @@ public class ImplementMethodRefactoring extends CRefactoring {
 		newFunctionDeclarator.setName(qname);
 		newFunctionDeclarator.setConst(functionDeclarator.isConst());
 	
-		PseudoNameGenerator pseudoNameGenerator = new PseudoNameGenerator();
 		
-			for(IASTParameterDeclaration parameter : getParameters()) {
+		for(Parameter actParameter : parameterHandler.getParameters()) {
-				if(parameter.getDeclarator().getName().toString().length() > 0) {
+			IASTParameterDeclaration createdParam = NodeFactory.createParameterDeclaration(actParameter.typeName, actParameter.parameterName);
-					pseudoNameGenerator.addExistingName(parameter.getDeclarator().getName().toString());
+			newFunctionDeclarator.addParameterDeclaration(createdParam);
 		}
-			}
-		
-		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);
-		}
-		
-		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 = insertLocation.getInsertPosition();
-		int insertOffset;
-		if(insertLocation.getNodeToInsertBefore() == null) {
-			insertOffset = 0;
-		} else {
-			insertOffset = insertLocation.getPartenOfNodeToInsertBefore().getFileLocation().getNodeOffset();
-		}
 		return NameHelper.createQualifiedNameFor(functionDeclarator.getName(), file, region.getOffset(), insertLocation.getInsertFile(), insertOffset);
 	}
 
@@ -242,7 +191,7 @@ public class ImplementMethodRefactoring extends CRefactoring {
 		return methodDeclaration;
 	}
 
-	public Set<String> getGeneratedNames() {
+	public ParameterHandler getParameterHandler() {
-		return generatedNames;
+		return parameterHandler;
 	}
 }

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()) {
+    	   if(refactoring.getParameterHandler().needsAdditionalArgumentNames()) {
-    		   addPage(new ParameterNamesInputPage(refactoring));
+    		   addPage(new ParameterNamesInputPage(refactoring.getParameterHandler()));
     	   }
        }
 }

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

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);

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

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<Parameter> 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<Parameter>();
+		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<Parameter> getParameters() {
+		return parameters;
+	}
+}

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 type = actParameter.typeName;
-			String content = String.valueOf(parameterDeclaration.getDeclarator().getName().toCharArray());
+			String content = actParameter.parameterName;
-			boolean enabled = parameterDeclaration.getDeclarator().getName().toCharArray().length > 0;
+			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);
+					actParameter.parameterName = newName;
-					
+					updatePreview();
-					if(isValid) {
+					return true;
-						parameterDeclaration.getDeclarator().setName(new CPPASTName(newName.toCharArray()));
-						translationUnitPreview.setPreviewText(refactoring.createFunctionDefinition().getRawSignature());
-					}
-					
-					return isValid;
 				}});
 		}
 
 		translationUnitPreview = new TranslationUnitPreview(new HashMap<String, String>(), 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());
+		}
 	}
 }

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)