Completes initial version of the extract function, by Emanuel Graf, bug 226484.

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ModificationScopeStack.java

@@ -117,7 +117,12 @@ public class ModificationScopeStack {
 				}
 			}
 			if(!nodeIsChildOfModifications(actualNode, scopeStack.getFirst())){
-				scopeStack.removeFirst();
+				if(scopeStack.getFirst().get(0).getTargetNode().getTranslationUnit() == actualNode.getTranslationUnit()){
+					scopeStack.removeFirst();
+				}
+				else{
+					return;
+				}
 			}
 			else{
 				return;

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

@@ -328,7 +328,7 @@ struct other
 
 class Klass
 {
-	void set(bool b){};
+	void set(bool b){}
 	void test()
 	{
 	  other o;
@@ -344,16 +344,17 @@ struct other
 
 class Klass
 {
-	void set(bool b){};
+	void set(bool b){}
+
     bool value_from(other o)
     {
         return o.value();
     }
 
     void test()
-	{
+    {
-	  other o;
+        other o;
-	  this->set(value_from(o));
+        this->set(value_from(o));
 	}
 };
 

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

@@ -89,7 +89,8 @@ template<typename T> T exp()
 
 template <typename T>
 int tempFunct(){
-    T p = exp();
+
+	T p = exp();
 	p + 2;
 	return 0;
 }

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

@@ -1,158 +1,3 @@
-//!ExtractFunctionRefactoringTest
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-//@A.h
-#ifndef A_H_
-#define A_H_
-
-class A
-{
-public:
-	A();
-	virtual ~A();
-	int foo();
-	
-private:
-	int help();
-};
-
-#endif /*A_H_*/
-
-//=
-#ifndef A_H_
-#define A_H_
-
-class A
-{
-public:
-	A();
-	virtual ~A();
-	int foo();
-	
-private:
-	int help();
-    void exp(int & i);
-};
-
-#endif /*A_H_*/
-
-//@A.cpp
-#include "A.h"
-
-A::A()
-{
-}
-
-A::~A()
-{
-}
-int A::foo()
-{
-	int i = 2;
-	//$++i;
-	help();$//
-	return i;
-}
-
-int A::help()
-{
-	return 42;
-}	
-
-//=
-#include "A.h"
-
-A::A()
-{
-}
-
-A::~A()
-{
-}
-void A::exp(int & i)
-{
-    ++i;
-    help();
-}
-
-int A::foo()
-{
-	int i = 2;
-    exp(i);
-	return i;
-}
-
-int A::help()
-{
-	return 42;
-}	
-
-//@.config
-filename=A.cpp
-methodname=exp
-replaceduplicates=false
-returnvalue=false
-returnparameterindex=0
-
-//!Extract Function first extracted statement with leading comment
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-//@main.cpp
-int main(){
-	
-	int i;
-	//$i= 7;$//
-	return i;
-}
-
-//=
-void exp(int & i)
-{
-    i = 7;
-}
-
-int main(){
-	
-	int i;
-    exp(i);
-	return i;
-}
-
-//@.config
-filename=main.cpp
-methodname=exp
-replaceduplicates=false
-returnvalue=false
-returnparameterindex=0
-
-//!Extract Function last extracted statement with trailling comment
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-//@main.cpp
-int main(){
-	
-	int i;
-	//$i= 7;$//
-	return i;
-}
-
-//=
-void exp(int & i)
-{
-    i = 7;
-}
-
-int main(){
-	
-	int i;
-    exp(i);
-	return i;
-}
-
-//@.config
-filename=main.cpp
-methodname=exp
-replaceduplicates=false
-returnvalue=false
-returnparameterindex=0
-
 //!ExtractFunctionRefactoringTest variable defined in scope
 //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
 //@A.h
@@ -185,7 +30,7 @@ public:
 	
 private:
 	int help();
-	int exp();
+    int exp();
 };
 
 #endif /*A_H_*/
@@ -242,6 +87,165 @@ int A::help()
 	return 42;
 }	
 
+//!ExtractFunctionRefactoringTest
+//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
+//@A.h
+#ifndef A_H_
+#define A_H_
+
+class A
+{
+public:
+	A();
+	virtual ~A();
+	int foo();
+	
+private:
+	int help();
+};
+
+#endif /*A_H_*/
+
+//=
+#ifndef A_H_
+#define A_H_
+
+class A
+{
+public:
+	A();
+	virtual ~A();
+	int foo();
+	
+private:
+	int help();
+    void exp(int & i);
+};
+
+#endif /*A_H_*/
+
+//@A.cpp
+#include "A.h"
+
+A::A()
+{
+}
+
+A::~A()
+{
+}
+int A::foo()
+{
+	int i = 2;
+	//comment
+	//$++i;
+	help();$//
+	return i;
+}
+
+int A::help()
+{
+	return 42;
+}	
+
+//=
+#include "A.h"
+
+A::A()
+{
+}
+
+A::~A()
+{
+}
+void A::exp(int & i)
+{
+    //comment
+    ++i;
+    help();
+}
+
+int A::foo()
+{
+	int i = 2;
+    exp(i);
+	return i;
+}
+
+int A::help()
+{
+	return 42;
+}	
+
+//@.config
+filename=A.cpp
+methodname=exp
+replaceduplicates=false
+returnvalue=false
+returnparameterindex=0
+
+//!Extract Function first extracted statement with leading comment
+//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
+//@main.cpp
+int main(){
+	
+	int i;
+	// Comment
+	//$i= 7;$//
+	return i;
+}
+
+//=
+void exp(int & i)
+{
+    // Comment
+    i = 7;
+}
+
+int main(){
+	
+	int i;
+    exp(i);
+	return i;
+}
+
+//@.config
+filename=main.cpp
+methodname=exp
+replaceduplicates=false
+returnvalue=false
+returnparameterindex=0
+
+//!Extract Function last extracted statement with trailling comment
+//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
+//@main.cpp
+int main(){
+	
+	int i;
+	//$i= 7;$// // Comment
+	return i;
+}
+
+//=
+void exp(int & i)
+{
+    i = 7; // Comment
+}
+
+int main(){
+	
+	int i;
+    exp(i);
+    return i;
+}
+
+//@.config
+filename=main.cpp
+methodname=exp
+replaceduplicates=false
+returnvalue=false
+returnparameterindex=0
+
 //!ExtractFunctionRefactoringTest with two variable defined in scope
 //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
 //@.config
@@ -802,7 +806,7 @@ public:
 	
 private:
 	int help();
-	void exp(int *& i);
+    void exp(int *& i);
 };
 
 #endif /*A_H_*/
@@ -851,8 +855,7 @@ int A::foo()
 {
 	int* i = new int(2);
     exp(i);
-	//A end-comment
+    return *i;
-	return *i;
 }
 
 int A::help()
@@ -892,7 +895,7 @@ public:
 	
 private:
 	int help();
-	void exp(int *& i);
+    void exp(int *& i);
 };
 
 #endif /*A_H_*/
@@ -940,7 +943,7 @@ void A::exp(int *& i)
 int A::foo()
 {
 	//A beautiful comment
-    int *i = new int(2);
+	int* i = new int(2);
     exp(i);
 	return *i;
 }
@@ -1489,49 +1492,6 @@ public:
 };
 
 #endif /*B_H_*/
-//!ExtractMethod Bug #18 (problem with Qt's Q_FOREACH macro)
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-//@.config
-methodname=add_to_sum
-filename=main.cpp
-//@main.cpp
-#define for_each_int_in_array_with_index(list) for(int i = 0; i < sizeof(list) / sizeof(int); i++)
-
-struct Foreach
-{
-    int run()
-    {
-        int sum = 0;
-        int list [] = { 5, 6, 7 };
-        for_each_int_in_array_with_index(list)
-        {
-            //$sum += list[i];$//
-        }
-        return sum;
-    }
-};
-
-//=
-#define for_each_int_in_array_with_index(list) for(int i = 0; i < sizeof(list) / sizeof(int); i++)
-
-struct Foreach
-{
-    void add_to_sum(int & sum, int list[], int i)
-    {
-        sum += list[i];
-    }
-
-    int run()
-    {
-        int sum = 0;
-        int list [] = { 5, 6, 7 };
-        for_each_int_in_array_with_index(list)
-        {
-            add_to_sum(sum, list, i);
-        }
-        return sum;
-    }
-};
 
 //!ExtractFunctionRefactoringTest with Return Value take the second and Ref Parameter
 //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
@@ -2002,37 +1962,6 @@ void function()
 	}
 }
 
-//!ExtractMethod with Macros Bug #120 Extract Function generates superfluous characters
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-//@.config
-filename=Test.cpp
-methodname=createSuite
-//@Test.cpp
-#define CUTE(name) cute::test((&name),(#name))
-
-void runSuite(){
-	//$cute::suite s;
-	s.push_back(CUTE(thisIsATest));
-	s.push_back(CUTE(testFuerRainer));$//
-	cute::ide_listener lis;
-	cute::makeRunner(lis)(s, "The Suite");
-}
-//=
-#define CUTE(name) cute::test((&name),(#name))
-
-cute::suite createSuite()
-{
-    cute::suite s;
-    s.push_back(CUTE(thisIsATest));
-    s.push_back(CUTE(testFuerRainer));
-    return s;
-}
-
-void runSuite(){
-	cute::suite s = createSuite();
-	cute::ide_listener lis;
-	cute::makeRunner(lis)(s, "The Suite");
-}
 //! Bug #124 Extract Function with a Macro call in selected code "forgets" the macro
 //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
 //@.config
@@ -2045,7 +1974,9 @@ methodname=runTest
 void testFuerRainer(){
         int i=int();
         //$++i;
+        //Leading Comment
         ASSERT (i);
+        //Trailling Comment
         --i;$//
 }
 
@@ -2056,7 +1987,9 @@ void testFuerRainer(){
 void runTest(int i)
 {
     ++i;
+    //Leading Comment
     ASSERT (i);
+    //Trailling Comment
     --i;
 }
 
@@ -2132,7 +2065,7 @@ string *runTest(int m_capacity)
 
 int main(){
 	int m_capacity;
-	string *newElements = runTest(m_capacity);
+    string *newElements = runTest(m_capacity);
 	newElements[0] = "s";
 }
 

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionTestSuite.java

@@ -25,10 +25,9 @@ public class ExtractFunctionTestSuite extends TestSuite {
 	@SuppressWarnings("nls")
 	public static Test suite() throws Exception {
 		TestSuite suite = new ExtractFunctionTestSuite();
-		suite.addTest(RefactoringTester.suite("Extract Expression", "resources/refactoring/ExtractExpression.rts"));
 		suite.addTest(RefactoringTester.suite("ExtractMethodRefactoringTests", "resources/refactoring/ExtractMethod.rts"));
+		suite.addTest(RefactoringTester.suite("Extract Expression", "resources/refactoring/ExtractExpression.rts"));
 		suite.addTest(RefactoringTester.suite("ExtractMethodPreprocessorRefactoringTests", "resources/refactoring/ExtractMethodPreprocessor.rts"));
-		suite.addTest(RefactoringTester.suite("ExtractMethodDuplicatesTests", "resources/refactoring/ExtractMethodDuplicates.rts"));
 		suite.addTest(RefactoringTester.suite("Extract Function Templates Tests", "resources/refactoring/ExtractFunctionTemplates.rts"));
 		return suite;
 	}

core/org.eclipse.cdt.ui/plugin.properties

@@ -158,7 +158,7 @@ CodingActionSet.description= Action set containing coding related C/C++ actions
 Refactoring.menu.label= Refac&tor
 Refactoring.renameAction.label=Re&name...
 Refactoring.extractConstant.label=Extr&act Constant...
-Refactoring.extractFunction.label=Extract &Function... (work in progress)
+Refactoring.extractFunction.label=Extract &Function...
 Refactoring.hideMethod.label=Hide Method...
 Refactoring.implementMethod.label=Impl&ement Method...
 Refactoring.gettersAndSetters.label=Generate Getters and Setters...

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java

@@ -62,9 +62,8 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper;
  *
  */
 public abstract class CRefactoring extends Refactoring {
-	protected static final String EMPTY_STRING = ""; //$NON-NLS-1$
+
 	private static final int AST_STYLE = ITranslationUnit.AST_CONFIGURE_USING_SOURCE_CONTEXT | ITranslationUnit.AST_SKIP_INDEXED_HEADERS;
-	public static final String NEWLINE = "\n";  //$NON-NLS-1$
 
 	protected String name = Messages.Refactoring_name; 
 	protected IFile file;

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionComposite.java

@@ -40,7 +40,8 @@ public class ExtractFunctionComposite extends Composite {
 		Group returnGroup = createReturnGroup(nameVisiComp);
 		createReturnValueChooser(returnGroup, info, ip);		
 
-		createReplaceCheckBox(nameVisiComp);
+		// Disabled for now
+		//createReplaceCheckBox(nameVisiComp);
 		
 		if (info.getMethodContext().getType() == MethodContext.ContextType.METHOD) {
 			visibilityPanelSetVisible(true);
@@ -105,13 +106,13 @@ public class ExtractFunctionComposite extends Composite {
 	}
 
 	
-	private void createReplaceCheckBox(Composite parent) {
+//	private void createReplaceCheckBox(Composite parent) {
-		replaceSimilar = new Button(parent, SWT.CHECK | SWT.LEFT);
+//		replaceSimilar = new Button(parent, SWT.CHECK | SWT.LEFT);
-		GridData buttonLayoutData = new GridData(SWT.None);
+//		GridData buttonLayoutData = new GridData(SWT.None);
-		buttonLayoutData.verticalIndent = 5;
+//		buttonLayoutData.verticalIndent = 5;
-		replaceSimilar.setLayoutData(buttonLayoutData);
+//		replaceSimilar.setLayoutData(buttonLayoutData);
-		replaceSimilar.setText(Messages.ExtractFunctionComposite_ReplaceDuplicates); 
+//		replaceSimilar.setText(Messages.ExtractFunctionComposite_ReplaceDuplicates); 
-	}
+//	}
 
 	
 	public ChooserComposite getReturnChooser() {

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionInputPage.java

@@ -16,8 +16,6 @@ import org.eclipse.swt.events.KeyEvent;
 import org.eclipse.swt.events.KeyListener;
 import org.eclipse.swt.events.MouseEvent;
 import org.eclipse.swt.events.MouseListener;
-import org.eclipse.swt.events.SelectionEvent;
-import org.eclipse.swt.events.SelectionListener;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
@@ -71,7 +69,9 @@ public class ExtractFunctionInputPage extends UserInputWizardPage {
 			});
 		}
 		
-		comp.getReplaceSimilarButton().addSelectionListener(new SelectionListener(){
+		/* Disable until it works again.
+		 * 
+		 * comp.getReplaceSimilarButton().addSelectionListener(new SelectionListener(){
 
 			public void widgetDefaultSelected(SelectionEvent e) {
 				info.setReplaceDuplicates(comp.getReplaceSimilarButton().isEnabled());	
@@ -81,7 +81,7 @@ public class ExtractFunctionInputPage extends UserInputWizardPage {
 				widgetDefaultSelected(e);		
 			}
 			
-		});
+		});*/
 		
 		setControl(comp);
 		

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java

@@ -12,7 +12,6 @@
 package org.eclipse.cdt.internal.ui.refactoring.extractfunction;
 
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Vector;
 import java.util.Map.Entry;
@@ -34,9 +33,9 @@ import org.eclipse.text.edits.TextEditGroup;
 
 import org.eclipse.cdt.core.dom.ast.DOMException;
 import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
-import org.eclipse.cdt.core.dom.ast.IASTComment;
 import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
+import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTExpression;
 import org.eclipse.cdt.core.dom.ast.IASTExpressionList;
@@ -47,11 +46,11 @@ import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
 import org.eclipse.cdt.core.dom.ast.IASTInitializerExpression;
 import org.eclipse.cdt.core.dom.ast.IASTName;
-import org.eclipse.cdt.core.dom.ast.IASTNamedTypeSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
 import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
 import org.eclipse.cdt.core.dom.ast.IASTReturnStatement;
+import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTStandardFunctionDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTStatement;
@@ -60,14 +59,11 @@ import org.eclipse.cdt.core.dom.ast.IField;
 import org.eclipse.cdt.core.dom.ast.IParameter;
 import org.eclipse.cdt.core.dom.ast.cpp.CPPASTVisitor;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTConversionName;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTOperatorName;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTSimpleDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateId;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateParameter;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPBinding;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod;
 import org.eclipse.cdt.core.dom.rewrite.ASTRewrite;
@@ -75,6 +71,7 @@ import org.eclipse.cdt.ui.CUIPlugin;
 
 import org.eclipse.cdt.internal.core.dom.parser.c.CASTBinaryExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTCompoundStatement;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarationStatement;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarator;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTExpressionList;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTExpressionStatement;
@@ -85,6 +82,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTInitializerExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTReturnStatement;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclSpecifier;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateDeclaration;
 
@@ -97,17 +95,11 @@ import org.eclipse.cdt.internal.ui.refactoring.NodeContainer;
 import org.eclipse.cdt.internal.ui.refactoring.MethodContext.ContextType;
 import org.eclipse.cdt.internal.ui.refactoring.NodeContainer.NameInformation;
 import org.eclipse.cdt.internal.ui.refactoring.utils.ASTHelper;
-import org.eclipse.cdt.internal.ui.refactoring.utils.CPPASTAllVisitor;
 import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper;
 import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper;
 
 public class ExtractFunctionRefactoring extends CRefactoring {
 
-//  egtodo	
-//	private static final String COMMA_SPACE = ", "; //$NON-NLS-1$
-//	private static final String TYPENAME = "typename "; //$NON-NLS-1$
-//	private static final String TEMPLATE_START = "template <"; //$NON-NLS-1$
-
 	static final Integer NULL_INTEGER = Integer.valueOf(0);
 
 	NodeContainer container;
@@ -312,18 +304,16 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 				.rewriterForTranslationUnit(firstNodeToWrite
 						.getTranslationUnit());
 		TextEditGroup editGroup = new TextEditGroup(title);
+		if(methodCall instanceof IASTDeclaration){
+			CPPASTDeclarationStatement declarationStatement = new CPPASTDeclarationStatement((IASTDeclaration) methodCall);
+			methodCall = declarationStatement;
+		}
 		rewriter.replace(firstNodeToWrite, methodCall, editGroup);
 		for (IASTNode node : container.getNodesToWrite()) {
 			if (node != firstNodeToWrite) {
 				rewriter.remove(node, editGroup);
 			}
-		}
+		}		
-
-		//Replace Dublicates not yet implemented
-		/* 
-		 * if(info.isReplaceDuplicates()){ replaceSimilar(astMethodName,
-		 * changesTreeSet, implementationFile, context.getType()); }
-		 */
 	}
 
 	private void createMethodDefinition(final IASTName astMethodName,
@@ -363,6 +353,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 
 		AddDeclarationNodeToClassChange.createChange(classDeclaration, info
 				.getVisibility(), methodDeclaration, false, collector);
+
 	}
 
 	private boolean isMethodAllreadyDefined(
@@ -431,209 +422,34 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 		return true;
 	}
 
-// egtodo
-//	private int calculateLength(IASTFunctionDefinition node) {
-//		int diff = 0;
-//		if (node.getParent() instanceof ICPPASTTemplateDeclaration) {
-//			ICPPASTTemplateDeclaration tempDec = (ICPPASTTemplateDeclaration) node
-//					.getParent();
-//			diff = node.getFileLocation().getNodeOffset()
-//					- tempDec.getFileLocation().getNodeOffset();
-//		}
-//
-//		return diff;
-//	}
-//
-//	private void replaceSimilar(final IASTName astMethodName,
-//			final ChangeTreeSet changesTreeSet, final IFile implementationFile,
-//			final ContextType contextType) {
-//		// Find similar code
-//		final List<IASTNode> nodesToRewriteWithoutComments = new LinkedList<IASTNode>();
-//
-//		for (IASTNode node : container.getNodesToWrite()) {
-//			if (!(node instanceof IASTComment)) {
-//				nodesToRewriteWithoutComments.add(node);
-//			}
-//		}
-//
-//		final Vector<IASTNode> initTrail = getTrail(nodesToRewriteWithoutComments);
-//		final String title;
-//		if (contextType == MethodContext.ContextType.METHOD) {
-//			title = Messages.ExtractFunctionRefactoring_CreateMethodCall;
-//		} else {
-//			title = Messages.ExtractFunctionRefactoring_CreateFunctionCall;
-//		}
-//
-//		if (!hasNameResolvingForSimilarError) {
-//			unit.accept(new SimilarFinderVisitor(this, changesTreeSet,
-//					initTrail, implementationFile, astMethodName,
-//					nodesToRewriteWithoutComments, title));
-//		}
-//	}
-
-	protected Vector<IASTNode> getTrail(List<IASTNode> stmts) {
-		final Vector<IASTNode> trail = new Vector<IASTNode>();
-
-		nameTrail = new HashMap<String, Integer>();
-		final Container<Integer> trailCounter = new Container<Integer>(
-				NULL_INTEGER);
-
-		for (IASTNode node : stmts) {
-			node.accept(new CPPASTAllVisitor() {
-				@Override
-				public int visitAll(IASTNode node) {
-
-					if (node instanceof IASTComment) {
-						// Visit Comment, but don't add them to the trail
-						return super.visitAll(node);
-					} else if (node instanceof IASTNamedTypeSpecifier) {
-						// Skip if somewhere is a named Type Specifier
-						trail.add(node);
-						return PROCESS_SKIP;
-					} else if (node instanceof IASTName) {
-						if (node instanceof ICPPASTConversionName
-								&& node instanceof ICPPASTOperatorName
-								&& node instanceof ICPPASTTemplateId) {
-							trail.add(node);
-							return super.visitAll(node);
-						}
-						// Save Name Sequenz Number
-						IASTName name = (IASTName) node;
-						TrailName trailName = new TrailName();
-						int actCount = trailCounter.getObject().intValue();
-						if (nameTrail.containsKey(name.getRawSignature())) {
-							Integer value = nameTrail.get(name
-									.getRawSignature());
-							actCount = value.intValue();
-						} else {
-							trailCounter.setObject(Integer
-									.valueOf(++actCount));
-							nameTrail.put(name.getRawSignature(),
-									trailCounter.getObject());
-						}
-						trailName.setNameNumber(actCount);
-						trailName.setRealName(name);
-
-						if (info.getReturnVariable() != null
-								&& info.getReturnVariable().getName()
-										.getRawSignature().equals(
-												name.getRawSignature())) {
-							returnNumber.setObject(Integer
-									.valueOf(actCount));
-						}
-
-						// Save type informations for the name
-						IBinding bind = name.resolveBinding();
-						IASTName[] declNames = name.getTranslationUnit()
-								.getDeclarationsInAST(bind);
-						if (declNames.length > 0) {
-							IASTNode tmpNode = ASTHelper
-									.getDeclarationForNode(declNames[0]);
-
-							IBinding declbind = declNames[0]
-									.resolveBinding();
-							if (declbind instanceof ICPPBinding) {
-								ICPPBinding cppBind = (ICPPBinding) declbind;
-								try {
-									trailName.setGloballyQualified(cppBind
-											.isGloballyQualified());
-								} catch (DOMException e) {
-									ILog logger = CUIPlugin.getDefault()
-											.getLog();
-									IStatus status = new Status(
-											IStatus.WARNING,
-											CUIPlugin.PLUGIN_ID,
-											IStatus.OK, e.getMessage(), e);
-
-									logger.log(status);
-								}
-							}
-
-							if (tmpNode != null) {
-								trailName.setDeclaration(tmpNode);
-							} else {
-								hasNameResolvingForSimilarError = true;
-							}
-						}
-
-						trail.add(trailName);
-						return PROCESS_SKIP;
-					} else {
-						trail.add(node);
-						return super.visitAll(node);
-					}
-				}
-			});
-
-		}
-
-		return trail;
-	}
-
-	protected boolean isStatementInTrail(IASTStatement stmt,
-			final Vector<IASTNode> trail) {
-		final Container<Boolean> same = new Container<Boolean>(Boolean.TRUE);
-		final TrailNodeEqualityChecker equalityChecker = new TrailNodeEqualityChecker(
-				names, namesCounter);
-
-		stmt.accept(new CPPASTAllVisitor() {
-			@Override
-			public int visitAll(IASTNode node) {
-
-				int pos = trailPos.getObject().intValue();
-
-				if (trail.size() <= 0 || pos >= trail.size()) {
-					same.setObject(Boolean.FALSE);
-					return PROCESS_ABORT;
-				}
-
-				if (node instanceof IASTComment) {
-					// Visit Comment, but they are not in the trail
-					return super.visitAll(node);
-				}
-
-				IASTNode trailNode = trail.get(pos);
-				trailPos.setObject(Integer.valueOf(pos + 1));
-
-				if (equalityChecker.isEquals(trailNode, node)) {
-					if (node instanceof ICPPASTQualifiedName
-							|| node instanceof IASTNamedTypeSpecifier) {
-						return PROCESS_SKIP;
-					}
-					return super.visitAll(node);
-
-				}
-				same.setObject(new Boolean(false));
-				return PROCESS_ABORT;
-			}
-		});
-
-		return same.getObject().booleanValue();
-	}
-
 	private void addMethod(IASTName astMethodName, MethodContext context,
 			ASTRewrite rewriter, IASTNode insertpoint, TextEditGroup group) {
 		
 		ICPPASTQualifiedName qname = new CPPASTQualifiedName();
 		if (context.getType() == ContextType.METHOD) {
 			for (int i = 0; i < (context.getMethodQName().getNames().length - 1); i++) {
-				qname.addName(context.getMethodQName().getNames()[i]);
+				qname.addName(new CPPASTName(context.getMethodQName().getNames()[i].toCharArray()));
 			}
 		}
 		qname.addName(astMethodName);
 
 		IASTFunctionDefinition func = new CPPASTFunctionDefinition();
 		func.setParent(unit);
-		func.setDeclSpecifier(getReturnType());
+
-		func.setDeclarator(extractedFunctionConstructionHelper
+		ICPPASTSimpleDeclSpecifier dummyDeclSpecifier = new CPPASTSimpleDeclSpecifier();
+		func.setDeclSpecifier(dummyDeclSpecifier);
+		dummyDeclSpecifier.setType(IASTSimpleDeclSpecifier.t_void);
+		
+		IASTStandardFunctionDeclarator createdFunctionDeclarator = extractedFunctionConstructionHelper
 				.createFunctionDeclarator(qname, info.getDeclarator(), info
 						.getReturnVariable(), container.getNodesToWrite(), info
-						.getAllUsedNames()));
+						.getAllUsedNames());
+		func.setDeclarator(createdFunctionDeclarator);
 
 		IASTCompoundStatement compound = new CPPASTCompoundStatement();
 		func.setBody(compound);
 		
-		
+		ASTRewrite subRW;
 		if(insertpoint.getParent() instanceof ICPPASTTemplateDeclaration) {
 			
 			CPPASTTemplateDeclaration templateDeclaration = new CPPASTTemplateDeclaration();
@@ -645,16 +461,17 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 			
 			templateDeclaration.setDeclaration(func);
 
-			rewriter.insertBefore(insertpoint.getParent().getParent(), insertpoint.getParent(), templateDeclaration, group);
+			subRW = rewriter.insertBefore(insertpoint.getParent().getParent(), insertpoint.getParent(), templateDeclaration, group);
 
 		} else {
 			
-			rewriter.insertBefore(insertpoint.getParent(), insertpoint, func, group);
+			subRW = rewriter.insertBefore(insertpoint.getParent(), insertpoint, func, group);
 		}
 		
-
+		subRW.replace(dummyDeclSpecifier, getReturnType(), group);
+		
 		extractedFunctionConstructionHelper.constructMethodBody(compound,
-				container.getNodesToWrite(), rewriter, group);
+				container.getNodesToWrite(), subRW, group);
 
 		// Set return value
 		if (info.getReturnVariable() != null) {
@@ -673,7 +490,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 				}
 				returnStmt.setReturnValue(expr);
 			}
-			rewriter.insertBefore(compound, null, returnStmt, group);
+			subRW.insertBefore(compound, null, returnStmt, group);
 		}
 
 	}
@@ -767,7 +584,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 
 		IASTFunctionCallExpression callExpression = new CPPASTFunctionCallExpression();
 		IASTIdExpression idExpression = new CPPASTIdExpression();
-		idExpression.setName(astMethodName);
+		idExpression.setName(new CPPASTName(astMethodName.toCharArray()));
 		IASTExpressionList paramList = getCallParameters();
 		callExpression.setParameterExpression(paramList);
 		callExpression.setFunctionNameExpression(idExpression);

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java

@@ -1,156 +0,0 @@
-/*******************************************************************************
- * 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.extractfunction;
-
-import java.util.List;
-import java.util.Vector;
-import java.util.Map.Entry;
-
-import org.eclipse.core.resources.IFile;
-
-import org.eclipse.cdt.core.dom.ast.IASTName;
-import org.eclipse.cdt.core.dom.ast.IASTNode;
-import org.eclipse.cdt.core.dom.ast.IASTStatement;
-import org.eclipse.cdt.core.dom.ast.cpp.CPPASTVisitor;
-
-import org.eclipse.cdt.internal.ui.refactoring.ChangeTreeSet;
-import org.eclipse.cdt.internal.ui.refactoring.NodeContainer;
-import org.eclipse.cdt.internal.ui.refactoring.NodeContainer.NameInformation;
-
-final class SimilarFinderVisitor extends CPPASTVisitor {
-
-	private final ExtractFunctionRefactoring extractFunctionRefactoring;
-
-	// egtodo
-	// private final ChangeTreeSet set;
-//	private final IFile file;
-//	private final IASTName name;
-
-	private final Vector<IASTNode> trail;
-	private final List<IASTNode> stmts;
-	private int i = 0;
-//	private int start;
-	private NodeContainer similarContainer;
-//	private final String title;
-
-	SimilarFinderVisitor(ExtractFunctionRefactoring extractFunctionRefactoring, ChangeTreeSet set, Vector<IASTNode> trail, IFile file, IASTName name, List<IASTNode> stmts, String title) {
-		this.extractFunctionRefactoring = extractFunctionRefactoring;
-//		this.set = set;
-		this.trail = trail;
-//		this.file = file;
-//		this.name = name;
-		this.stmts = stmts;
-//		this.title = title;
-
-		this.similarContainer = new NodeContainer();
-	}
-
-	{
-		shouldVisitStatements = true;
-	}
-
-	@Override
-	public int visit(IASTStatement stmt) {
-
-		boolean isAllreadyInMainRefactoring = isInSelection(stmt);
-
-		if( (!isAllreadyInMainRefactoring)
-				&& this.extractFunctionRefactoring.isStatementInTrail(stmt, trail)){
-			if(i == 0){
-//				start = stmt.getFileLocation().getNodeOffset();
-			}
-			similarContainer.add(stmt);	
-			++i;
-
-			if(i==stmts.size()){
-				//found similar code
-
-				boolean similarOnReturnWays = true;
-				for (NameInformation nameInfo : similarContainer.getAllAfterUsedNames()) {
-					if(this.extractFunctionRefactoring.names.containsKey(nameInfo.getDeclaration().getRawSignature())){
-						Integer nameOrderNumber = this.extractFunctionRefactoring.names.get(nameInfo.getDeclaration().getRawSignature());
-						if(this.extractFunctionRefactoring.nameTrail.containsValue(nameOrderNumber)){
-							String orgName = null;
-							boolean found = false;
-							for (Entry<String, Integer> entry : this.extractFunctionRefactoring.nameTrail.entrySet()) {
-								if(entry.getValue().equals(nameOrderNumber)){
-									orgName = entry.getKey();
-								}
-							}
-							if(orgName != null){
-								for (NameInformation orgNameInfo : this.extractFunctionRefactoring.container.getAllAfterUsedNamesChoosenByUser()) {
-									if( orgName.equals(orgNameInfo.getDeclaration().getRawSignature()) ){
-										found = true;
-									}
-								}
-							}
-
-							if(!found){
-								similarOnReturnWays = false;
-							}
-						}
-					}
-				}
-
-				if (similarOnReturnWays) {
-					// egtodo
-//					CTextFileChange replace = new CTextFileChange(title, file);
-//					IASTFileLocation loc = stmt.getFileLocation();
-//					int end = loc.getNodeOffset() + loc.getNodeLength() - start;
-//					try {
-//						end = this.extractFunctionRefactoring
-//								.getLengthWithNewLine(file, start, loc
-//										.getNodeOffset()
-//										+ loc.getNodeLength() - start);
-//					} catch (CoreException e) {
-//						// Keep current length
-//					}
-//					ReplaceEdit replaceEdit = new ReplaceEdit(start, end,
-//							this.extractFunctionRefactoring.getMethodCall(name,
-//									this.extractFunctionRefactoring.nameTrail,
-//									this.extractFunctionRefactoring.names,
-//									this.extractFunctionRefactoring.container,
-//									similarContainer));
-//					replace.setEdit(replaceEdit);
-//					set.add(replace);
-				}
-
-				clear();
-			}
-
-			return PROCESS_SKIP;
-		}
-		clear();
-		return super.visit(stmt);
-
-	}
-
-	private boolean isInSelection(IASTStatement stmt) {
-		List<IASTNode>nodes = this.extractFunctionRefactoring.container.getNodesToWrite();
-		for (IASTNode node : nodes) {
-			if(node.equals(stmt)) {
-				return true;
-			}
-		}
-		return false;
-
-		//			return container.getNodesToWrite().contains(stmt);
-	}
-
-	private void clear() {
-		i = 0;
-		this.extractFunctionRefactoring.names.clear();
-		similarContainer = new NodeContainer();
-		this.extractFunctionRefactoring.namesCounter.setObject(ExtractFunctionRefactoring.NULL_INTEGER);
-		this.extractFunctionRefactoring.trailPos.setObject(ExtractFunctionRefactoring.NULL_INTEGER);
-	}
-}

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/messages.properties

@@ -14,4 +14,4 @@ ExtractConstantAction_label=Extract Constant...
 GettersAndSetters_label=Generate Getters and Setters... 
 ImplementMethodAction_label=Implement Method...
 HideMethodAction_label=Hide Method...
-ExtractFunctionAction_label=Extract Function... (work in progress)
+ExtractFunctionAction_label=Extract Function...