Bug 260133: Extract function and extract local variable don't handle type promotion

https://bugs.eclipse.org/bugs/show_bug.cgi?id=260133

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

@@ -446,7 +446,7 @@ class Test
 class Test
 {
   void test();
-    wchar_t greeting();
+    const char greeting();
 };
 
 
@@ -461,7 +461,7 @@ void Test::test()
 //=
 #include "test.h"
 
-wchar_t Test::greeting()
+const char Test::greeting()
 {
     return "hello";
 }
@@ -541,7 +541,7 @@ class Test
 
 void Test::test()
 {
-    float f = /*$*/0.42/*$$*/;
+    float f = /*$*/0.42f/*$$*/;
 }
 
 //=
@@ -549,7 +549,7 @@ void Test::test()
 
 float Test::certainty()
 {
-    return 0.42;
+    return 0.42f;
 }
 
 void Test::test()
@@ -686,49 +686,6 @@ void Test::test()
     bool b = invalid();
 }
 
-//!Extract string constant
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-
-//@.config
-filename=test.cpp
-methodname=greeting
-
-//@test.h
-class Test
-{
-  void test();
-};
-
-
-//=
-class Test
-{
-  void test();
-    wchar_t greeting();
-};
-
-
-//@test.cpp
-#include "test.h"
-
-void Test::test()
-{
-    char* hi = /*$*/"hello"/*$$*/;
-}
-
-//=
-#include "test.h"
-
-wchar_t Test::greeting()
-{
-    return "hello";
-}
-
-void Test::test()
-{
-    char* hi = greeting();
-}
-
 //!Extract int constant
 //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
 
@@ -772,49 +729,6 @@ void Test::test()
     int i = size();
 }
 
-//!Extract float constant
-//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
-
-//@.config
-filename=test.cpp
-methodname=certainty
-
-//@test.h
-class Test
-{
-  void test();
-};
-
-
-//=
-class Test
-{
-  void test();
-    float certainty();
-};
-
-
-//@test.cpp
-#include "test.h"
-
-void Test::test()
-{
-    float f = /*$*/0.42/*$$*/;
-}
-
-//=
-#include "test.h"
-
-float Test::certainty()
-{
-    return 0.42;
-}
-
-void Test::test()
-{
-    float f = certainty();
-}
-
 //!Extract char constant
 //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
 
@@ -972,3 +886,29 @@ int test(foo s) {
 }
 
 
+//!Bug 260133 Extract function and extract local variable don't handle type promotion
+//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest
+
+//@.config
+filename=test.cpp
+methodname=bar
+
+//@test.cpp
+
+void foo(){
+	int x = 3;
+    double y = /*$*/x + 2.5/*$$*/;
+}
+
+//=
+
+double bar(int x)
+{
+    return x + 2.5;
+}
+
+void foo(){
+	int x = 3;
+    double y = bar(x);
+}
+

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

@@ -2381,7 +2381,7 @@ int main() {
 //=
 #include "testString.h"
 
-wchar_t endTag(test::string name)
+const char endTag(test::string name)
 {
     return "</" + name + ">";
 }
@@ -2437,7 +2437,7 @@ int main() {
 //=
 #include "testString.h"
 
-wchar_t exp()
+const char exp()
 {
     return ">" + "</";
 }
@@ -2493,7 +2493,7 @@ int main() {
 //=
 #include "testString.h"
 
-wchar_t endTag(test::string name)
+const char endTag(test::string name)
 {
     return "</" + name + ">";
 }
@@ -2549,7 +2549,7 @@ int main() {
 //=
 #include "testString.h"
 
-wchar_t exp()
+const char exp()
 {
     return ">" + "</";
 }

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2008, 2011 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 
@@ -11,16 +11,19 @@
  *******************************************************************************/
 package org.eclipse.cdt.internal.ui.refactoring.extractfunction;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.text.edits.TextEditGroup;
 
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
 import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTExpression;
 import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement;
 import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
+import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
 import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
@@ -28,29 +31,23 @@ import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IBasicType;
 import org.eclipse.cdt.core.dom.ast.IBasicType.Kind;
 import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.core.dom.ast.INodeFactory;
 import org.eclipse.cdt.core.dom.ast.IType;
 import org.eclipse.cdt.core.dom.ast.ITypedef;
 import org.eclipse.cdt.core.dom.ast.IVariable;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTBinaryExpression;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNewExpression;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName;
-import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType;
 import org.eclipse.cdt.core.dom.rewrite.ASTRewrite;
+import org.eclipse.cdt.core.dom.rewrite.DeclarationGenerator;
 
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTBinaryExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFieldReference;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionDefinition;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTIdExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTLiteralExpression;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNamedTypeSpecifier;
-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.CPPBasicType;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPFunction;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
 
 import org.eclipse.cdt.internal.ui.refactoring.NodeContainer.NameInformation;
 
@@ -93,170 +90,51 @@ public class ExtractExpression extends ExtractedFunctionConstructionHelper {
 
 	@Override
 	public IASTDeclSpecifier determineReturnType(IASTNode extractedNode, NameInformation _) {
-		IASTDeclSpecifier declSpecifier = null;
+		List<ITypedef> typdefs = getTypdefs(extractedNode);
-		
+		if (extractedNode instanceof IASTExpression) {
-		if (extractedNode instanceof ICPPASTBinaryExpression) {
+			IASTExpression exp = (IASTExpression) extractedNode;
-			declSpecifier = handleBinaryExpression((ICPPASTBinaryExpression) extractedNode);
+			INodeFactory factory = extractedNode.getTranslationUnit().getASTNodeFactory();
-		}
+			DeclarationGenerator generator = DeclarationGenerator.create(factory);
-		
+			IType expressionType = exp.getExpressionType();
-		if (extractedNode instanceof ICPPASTNewExpression) {
+			for (ITypedef typedef : typdefs) {
-			declSpecifier = handleNewExpression((ICPPASTNewExpression) extractedNode);
+				if (typedef.getType().isSameType(expressionType)) {
-		}
+					return generator.createDeclSpecFromType(typedef);
-
-		if (extractedNode instanceof IASTFunctionCallExpression) {
-			declSpecifier = handleFunctionCallExpression((IASTFunctionCallExpression) extractedNode);
-		}
-		
-		if (extractedNode instanceof IASTLiteralExpression) {
-		    declSpecifier = handleLiteralExpression((IASTLiteralExpression) extractedNode);
-		}
-		
-		if(declSpecifier == null) {
-			if(extractedNode instanceof IASTExpression) {
-				IType type = ((IASTExpression)extractedNode).getExpressionType();
-				if(type instanceof IBasicType) {
-					return createSimpleDeclSpecifier(((IBasicType)type).getKind());
 				}
 			}
+			return generator.createDeclSpecFromType(expressionType);
+		} else {// Fallback
 			return createSimpleDeclSpecifier(Kind.eVoid);
 		}
-		if(declSpecifier.isFrozen()) {
-			return declSpecifier.copy();
-		}else {
-			return declSpecifier;
-		}
 	}
 
-	private IASTDeclSpecifier handleLiteralExpression(IASTLiteralExpression extractedNode) {
+	private List<ITypedef> getTypdefs(IASTNode extractedNode) {
-        switch(extractedNode.getKind()){
+		final ArrayList<ITypedef> typeDefs = new ArrayList<ITypedef>();
-          case IASTLiteralExpression.lk_char_constant:
+		extractedNode.accept(new ASTVisitor() {
-              return createSimpleDeclSpecifier(Kind.eChar);
+			{
-          case IASTLiteralExpression.lk_float_constant:
+				shouldVisitExpressions = true;
-              return createSimpleDeclSpecifier(Kind.eFloat);
-          case IASTLiteralExpression.lk_integer_constant:
-              return createSimpleDeclSpecifier(Kind.eInt);
-          case IASTLiteralExpression.lk_string_literal:
-              return createSimpleDeclSpecifier(Kind.eWChar);
-          case IASTLiteralExpression.lk_false: 
-              //Like lk_true a boolean type
-          case IASTLiteralExpression.lk_true:
-              return createSimpleDeclSpecifier(Kind.eBoolean);
-          default:
-              return null;
-          }
-    }
-
-    private IASTDeclSpecifier handleNewExpression(ICPPASTNewExpression expression) {
-		return expression.getTypeId().getDeclSpecifier();
-	}
-
-	private IASTDeclSpecifier handleBinaryExpression(ICPPASTBinaryExpression node) {
-		
-		switch (node.getOperator()) {
-		case IASTBinaryExpression.op_equals:
-		case IASTBinaryExpression.op_notequals:
-		case IASTBinaryExpression.op_logicalOr:
-		case IASTBinaryExpression.op_logicalAnd:
-		case IASTBinaryExpression.op_greaterEqual:
-		case IASTBinaryExpression.op_greaterThan:
-		case IASTBinaryExpression.op_lessEqual:
-		case IASTBinaryExpression.op_lessThan:
-		
-			/* We assume that these operations evaluate to bool and don't 
-			 * consider overriden operators from custom types for now.*/
-			return createSimpleDeclSpecifier(Kind.eBoolean);
-			
-		case IASTBinaryExpression.op_plus:
-		case IASTBinaryExpression.op_plusAssign:
-		case IASTBinaryExpression.op_minus:
-		case IASTBinaryExpression.op_minusAssign:
-		case IASTBinaryExpression.op_multiply:
-		case IASTBinaryExpression.op_multiplyAssign:
-		case IASTBinaryExpression.op_divide:
-		case IASTBinaryExpression.op_divideAssign:
-		case IASTBinaryExpression.op_assign:
-
-			
-			return getTypeFromBinaryExp(node);
-		}
-		
-		return null /* not yet handled */;
-	}
-
-	private IASTDeclSpecifier getTypeFromBinaryExp(ICPPASTBinaryExpression node) {
-		if(node.getOperand1() instanceof ICPPASTBinaryExpression) {
-			IASTDeclSpecifier ret = getTypeFromBinaryExp(((CPPASTBinaryExpression)node.getOperand1()));
-			if(ret != null) return ret;
-		}else {
-			if(node.getOperand1() instanceof CPPASTIdExpression) {
-				return getBinaryExpressionType((CPPASTIdExpression) node.getOperand1());
-			}
-		}
-
-		if(node.getOperand2() instanceof ICPPASTBinaryExpression) {
-			IASTDeclSpecifier ret = getTypeFromBinaryExp(((CPPASTBinaryExpression)node.getOperand2()));
-			if(ret != null) return ret;
-		}else {
-			if(node.getOperand2() instanceof CPPASTIdExpression) {
-				return getBinaryExpressionType((CPPASTIdExpression) node.getOperand2());
-			}
-		}
-		return null;
-	}
-
-	private IASTDeclSpecifier getBinaryExpressionType(CPPASTIdExpression expression) {
-		IType expressionType = ((IVariable) expression.getName().resolveBinding()).getType();
-		if (expressionType instanceof ITypedef) {
-			ITypedef typdef = (ITypedef) expressionType;
-			IType expandedType = expression.getExpressionType();
-			if(typdef.getType().isSameType(expandedType)) {
-				return getDeclSpecifierForType(typdef);
-			}else{
-				return getDeclSpecifierForType(expandedType);
 			}
 			
-		}
+			@Override
-		return getDeclSpecifierForType(expressionType);
+			public int visit(IASTExpression expression) {
-
+				if (expression instanceof IASTIdExpression) {
-	}
+					IASTIdExpression id = (IASTIdExpression) expression;
-
+					IBinding binding = id.getName().resolveBinding();
-	private IASTDeclSpecifier getDeclSpecifierForType(IType expressionType) {
+					IType expressionType = null;
-
+					if (binding instanceof IVariable) {
-		if (expressionType instanceof CPPBasicType) {
+						expressionType = ((IVariable) binding).getType();
-			
+					}
-			CPPBasicType basicType = (CPPBasicType) expressionType;
+					if (binding instanceof IType) {
-			return createSimpleDeclSpecifier(basicType.getKind());
+						expressionType = (IType) binding;
-			
+					}
-		} else if (expressionType instanceof ITypedef) {
+					if (expressionType != null && expressionType instanceof ITypedef) {
-			return getDeclSpecForType((ITypedef) expressionType);
+						ITypedef typdef = (ITypedef) expressionType;
-		} else if (expressionType instanceof ICPPClassType) {
+						typeDefs.add(typdef);
-			return getDeclSpecForType((ICPPClassType)expressionType);
+					}
-		}
+				}
-		return null;
+				return PROCESS_CONTINUE;
-	}
+			}
-
+		});
-	private IASTDeclSpecifier getDeclSpecForType(ICPPClassType expressionType) {
+		return typeDefs;
-		IASTName name = null;
-
-		char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(expressionType);
-		name = new CPPASTQualifiedName();
-		for (char[] cs : qualifiedNameCharArray) {
-			((ICPPASTQualifiedName) name).addName(new CPPASTName(cs));
-		}
-
-		return new CPPASTNamedTypeSpecifier(name);
-	}
-
-	private CPPASTNamedTypeSpecifier getDeclSpecForType(ITypedef classType) {
-
-		IASTName name = null;
-		char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(classType);
-		name = new CPPASTQualifiedName();
-		for (char[] cs : qualifiedNameCharArray) {
-			((ICPPASTQualifiedName) name).addName(new CPPASTName(cs));
-		}
-
-		return new CPPASTNamedTypeSpecifier(name);
 	}
 
 	private static IASTDeclSpecifier createSimpleDeclSpecifier(IBasicType.Kind type) {
@@ -279,34 +157,6 @@ public class ExtractExpression extends ExtractedFunctionConstructionHelper {
 		return functionName;
 	}
 
-	private static IASTDeclSpecifier handleFunctionCallExpression(IASTFunctionCallExpression callExpression) {
-		IASTName functionName = findCalledFunctionName(callExpression);
-		if(functionName != null) {
-			IBinding binding = functionName.resolveBinding();
-			if (binding instanceof CPPFunction) {
-				CPPFunction function =  (CPPFunction) binding;
-				if(function.getDefinition() != null) {
-					IASTNode parent = function.getDefinition().getParent();
-					if(parent instanceof CPPASTFunctionDefinition) {
-						CPPASTFunctionDefinition definition = (CPPASTFunctionDefinition) parent;
-						return definition.getDeclSpecifier();
-					}
-				} else if(hasDeclaration(function)) {
-					IASTNode parent = function.getDeclarations()[0].getParent();
-					if (parent instanceof CPPASTSimpleDeclaration) {
-						CPPASTSimpleDeclaration declaration = (CPPASTSimpleDeclaration) parent;
-						return declaration.getDeclSpecifier();
-					}
-				}
-			}else if(binding instanceof ITypedef) {
-				ITypedef typedef = (ITypedef) binding;
-				return new CPPASTNamedTypeSpecifier(new CPPASTName(typedef.getNameCharArray()));
-			}
-
-		}			
-		return null;
-	}
-
 	@Override
 	protected boolean isReturnTypeAPointer(IASTNode node) {
 		if(node instanceof ICPPASTNewExpression) {