Fixed ExtractFunctionRefactoringTest.testOutputParameterWithMethodCall

test.

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/CxxAstUtils.java

@@ -32,6 +32,7 @@ import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle;
 import org.eclipse.cdt.core.dom.ast.IASTNodeSelector;
 import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTPreprocessorMacroExpansion;
@@ -170,7 +171,7 @@ public final class CxxAstUtils {
 			simpleDeclaration.addDeclarator(declarator);
 			return simpleDeclaration;
 		} else { // Fallback - return a `void` declaration
-			IASTDeclarator declarator = factory.newDeclarator(astName.copy());
+			IASTDeclarator declarator = factory.newDeclarator(astName.copy(CopyStyle.withLocations));
 			IASTSimpleDeclSpecifier declspec = factory.newSimpleDeclSpecifier();
 			declspec.setType(Kind.eInt);
 			IASTSimpleDeclaration simpleDeclaration = factory.newSimpleDeclaration(declspec);
@@ -273,9 +274,9 @@ public final class CxxAstUtils {
 						if (child instanceof IASTParameterDeclaration) {
 							if (nthParam == targetParameterNum) {
 								IASTParameterDeclaration pd = (IASTParameterDeclaration) child;
-								IASTDeclSpecifier declspec = pd.getDeclSpecifier().copy();
+								IASTDeclSpecifier declspec = pd.getDeclSpecifier().copy(CopyStyle.withLocations);
-								IASTDeclarator declarator = pd.getDeclarator().copy();
+								IASTDeclarator declarator = pd.getDeclarator().copy(CopyStyle.withLocations);
-								setNameInNestedDeclarator(declarator, astName.copy());
+								setNameInNestedDeclarator(declarator, astName.copy(CopyStyle.withLocations));
 								IASTSimpleDeclaration declaration = factory.newSimpleDeclaration(declspec);
 								declaration.addDeclarator(declarator);
 								return declaration;

core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/WhitespaceHandlingTest.java

@@ -13,8 +13,9 @@ package org.eclipse.cdt.core.parser.tests.rewrite.changegenerator.replace;
 
 import junit.framework.Test;
 
-import org.eclipse.cdt.core.dom.ast.IASTStatement;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle;
+import org.eclipse.cdt.core.dom.ast.IASTStatement;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTForStatement;
 import org.eclipse.cdt.core.parser.tests.rewrite.changegenerator.ChangeGeneratorTest;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPNodeFactory;
@@ -60,7 +61,7 @@ public class WhitespaceHandlingTest extends ChangeGeneratorTest {
 					ICPPASTForStatement forStatement = (ICPPASTForStatement) statement;
 					CPPNodeFactory nf = CPPNodeFactory.getDefault();
 					
-					ICPPASTForStatement newFor = forStatement.copy();
+					ICPPASTForStatement newFor = forStatement.copy(CopyStyle.withLocations);
 					newFor.setBody(nf.newNullStatement());
 					
 					ASTModification modification = new ASTModification(ASTModification.ModificationKind.REPLACE, forStatement, newFor, null);

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTNodeLocation.java

@@ -6,7 +6,7 @@
  * http://www.eclipse.org/legal/epl-v10.html
  *
  * Contributors:
- *    Doug Schaefer (IBM) - Initial API and implementation
+ *     Doug Schaefer (IBM) - Initial API and implementation
  *******************************************************************************/
 package org.eclipse.cdt.core.dom.ast;
 
@@ -18,6 +18,8 @@ package org.eclipse.cdt.core.dom.ast;
  * @noimplement This interface is not intended to be implemented by clients.
  */
 public interface IASTNodeLocation {
+	/** @since 5.4 */
+	public static final IASTNodeLocation[] EMPTY_ARRAY = {};
 
 	/**
 	 * This is the offset within either the file or a macro-expansion.
@@ -33,5 +35,4 @@ public interface IASTNodeLocation {
      * Return a file location that best maps into this location.
      */
     public IASTFileLocation asFileLocation();
-
 }

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTNode.java

@@ -8,6 +8,7 @@
  * Contributors:
  *     John Camelon - Initial API and implementation
  *     Markus Schorn (Wind River Systems)
+ *     Sergey Prigoin (Google)
  *******************************************************************************/
 package org.eclipse.cdt.internal.core.dom.parser;
 
@@ -36,7 +37,6 @@ import org.eclipse.cdt.internal.core.parser.scanner.Token;
  */
 public abstract class ASTNode implements IASTNode {
 	protected static final ICPPFunction UNINITIALIZED_FUNCTION = new CPPFunction(null);
-    private static final IASTNodeLocation[] EMPTY_LOCATION_ARRAY = {};
 
     private IASTNode parent;
     private ASTNodeProperty property;
@@ -139,18 +139,18 @@ public abstract class ASTNode implements IASTNode {
 
     @Override
 	public IASTNodeLocation[] getNodeLocations() {
-        if (locations != null)
+        if (locations == null) {
-            return locations;
+	        if (length != 0) {
-        if (length == 0) {
+	        	final IASTTranslationUnit tu= getTranslationUnit();
-        	locations= EMPTY_LOCATION_ARRAY;
+	        	if (tu != null) {
-        } else {
+	        		ILocationResolver l= (ILocationResolver) tu.getAdapter(ILocationResolver.class);
-        	final IASTTranslationUnit tu= getTranslationUnit();
+	        		if (l != null) {
-        	if (tu != null) {
+	        			locations= l.getLocations(getOffset(), length);
-        		ILocationResolver l= (ILocationResolver) tu.getAdapter(ILocationResolver.class);
+	        		}
-        		if (l != null) {
+	        	}
-        			locations= l.getLocations(getOffset(), length);
+	        }
-        		}
+	        if (locations == null)
-        	}
+	        	locations= IASTNodeLocation.EMPTY_ARRAY;
         }
         return locations;
     }

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTArrayDeclarator.java

@@ -46,7 +46,7 @@ public class CASTArrayDeclarator extends CASTDeclarator implements IASTArrayDecl
 		CASTArrayDeclarator copy = new CASTArrayDeclarator();
 		copyBaseDeclarator(copy, style);
 		for (IASTArrayModifier modifier : getArrayModifiers())
-			copy.addArrayModifier(modifier == null ? null : modifier.copy());
+			copy.addArrayModifier(modifier == null ? null : modifier.copy(style));
 		if (style == CopyStyle.withLocations) {
 			copy.setCopyLocation(this);
 		}

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTCaseStatement.java

@@ -40,7 +40,7 @@ public class CASTCaseStatement extends ASTNode implements IASTCaseStatement, IAS
 
 	@Override
 	public CASTCaseStatement copy(CopyStyle style) {
-		CASTCaseStatement copy = new CASTCaseStatement(expression == null ? null : expression.copy());
+		CASTCaseStatement copy = new CASTCaseStatement(expression == null ? null : expression.copy(style));
 		copy.setOffsetAndLength(this);
 		if (style == CopyStyle.withLocations) {
 			copy.setCopyLocation(this);

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTDesignatedInitializer.java

@@ -100,7 +100,7 @@ public class CASTDesignatedInitializer extends ASTNode implements ICASTDesignate
 			return (IASTInitializer) rhs;
 		}
 		if (rhs instanceof IASTExpression) {
-			CASTEqualsInitializer init = new CASTEqualsInitializer(((IASTExpression)rhs).copy());
+			CASTEqualsInitializer init = new CASTEqualsInitializer(((IASTExpression) rhs).copy());
 			init.setParent(this);
 			init.setPropertyInParent(OPERAND);
 			return init;

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTBinaryTypeIdExpression.java

@@ -44,8 +44,8 @@ public class CPPASTBinaryTypeIdExpression extends ASTNode implements IASTBinaryT
 	@Override
 	public CPPASTBinaryTypeIdExpression copy(CopyStyle style) {
 		CPPASTBinaryTypeIdExpression copy = new CPPASTBinaryTypeIdExpression(fOperator, 
-				fOperand1 == null ? null : fOperand1.copy(),
+				fOperand1 == null ? null : fOperand1.copy(style),
-				fOperand2 == null ? null : fOperand2.copy());
+				fOperand2 == null ? null : fOperand2.copy(style));
 		copy.setOffsetAndLength(this);
 		if (style == CopyStyle.withLocations) {
 			copy.setCopyLocation(this);

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTIdExpression.java

@@ -69,7 +69,7 @@ public class CPPASTIdExpression extends ASTNode implements IASTIdExpression, ICP
 
 	@Override
 	public CPPASTIdExpression copy(CopyStyle style) {
-		CPPASTIdExpression copy = new CPPASTIdExpression(name == null ? null : name.copy());
+		CPPASTIdExpression copy = new CPPASTIdExpression(name == null ? null : name.copy(style));
 		copy.setOffsetAndLength(this);
 		if (style == CopyStyle.withLocations) {
 			copy.setCopyLocation(this);

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTRangeBasedForStatement.java

@@ -152,7 +152,8 @@ public class CPPASTRangeBasedForStatement extends ASTNode implements ICPPASTRang
 				CPPASTName name = new CPPASTName(CPPVisitor.BEGIN);
 				name.setOffset(position.getOffset());
 				CPPASTIdExpression fname = new CPPASTIdExpression(name);
-				IASTExpression expr= new CPPASTFunctionCallExpression(fname, new IASTInitializerClause[] {forInit.copy()});
+				IASTExpression expr= new CPPASTFunctionCallExpression(fname,
+						new IASTInitializerClause[] { forInit.copy() });
 				expr.setParent(this);
 				expr.setPropertyInParent(ICPPASTRangeBasedForStatement.INITIALIZER);
 				

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPNodeFactory.java

@@ -113,12 +113,10 @@ import org.eclipse.cdt.core.parser.IScanner;
 import org.eclipse.cdt.internal.core.dom.parser.NodeFactory;
 import org.eclipse.cdt.internal.core.parser.scanner.CPreprocessor;
 
-
 /**
  * Abstract factory implementation that creates C++ AST nodes.
  */
 public class CPPNodeFactory extends NodeFactory implements ICPPNodeFactory {
-
 	private static final CPPNodeFactory DEFAULT_INSTANCE = new CPPNodeFactory();
 	
 	public static CPPNodeFactory getDefault() {

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/ASTLiteralNode.java

@@ -61,7 +61,7 @@ public class ASTLiteralNode implements IASTNode {
 
 	@Override
 	public IASTNodeLocation[] getNodeLocations() {
-		return null;
+		return IASTNodeLocation.EMPTY_ARRAY;
 	}
 
 	@Override

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/DeclarationGeneratorImpl.java

@@ -23,6 +23,7 @@ import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTExpression;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNamedTypeSpecifier;
+import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle;
 import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTPointer;
 import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
@@ -164,7 +165,7 @@ public class DeclarationGeneratorImpl extends DeclarationGenerator {
 						arrayType = (IArrayType) type;
 						IASTExpression arraySizeExpression = arrayType.getArraySizeExpression();
 						arrayDeclarator.addArrayModifier(factory.newArrayModifier(arraySizeExpression == null
-								? null : arraySizeExpression.copy()));
+								? null : arraySizeExpression.copy(CopyStyle.withLocations)));
 						type = arrayType.getType();
 					}
 					returnedDeclarator = arrayDeclarator;
@@ -291,7 +292,7 @@ public class DeclarationGeneratorImpl extends DeclarationGenerator {
 				int nbQualifiedNames = fullQualifiedName.getNames().length;
 				if (nbQualifiedNames > 1) {
 					for (int i = 0; i < nbQualifiedNames - 1; i++) {
-						newQualifiedName.addName(fullQualifiedName.getNames()[i].copy());
+						newQualifiedName.addName(fullQualifiedName.getNames()[i].copy(CopyStyle.withLocations));
 					}
 				}
 				newQualifiedName.addName(tempId);
@@ -309,7 +310,7 @@ public class DeclarationGeneratorImpl extends DeclarationGenerator {
 
 	private ICPPASTTemplateId getTemplateId(ICPPTemplateInstance type, IASTName templateName) {
 		ICPPNodeFactory cppFactory = (ICPPNodeFactory) factory;
-		ICPPASTTemplateId tempId = cppFactory.newTemplateId(templateName.copy());
+		ICPPASTTemplateId tempId = cppFactory.newTemplateId(templateName.copy(CopyStyle.withLocations));
 		for (ICPPTemplateArgument arg : type.getTemplateArguments()) {
 			IASTDeclSpecifier argDeclSpec = createDeclSpecFromType(arg.isTypeValue() ?
 					arg.getTypeValue() : arg.getTypeOfNonTypeValue());

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/scanner/ASTPreprocessorName.java

@@ -164,9 +164,9 @@ class ASTBuiltinName extends ASTPreprocessorDefinition {
 	@Override
 	public IASTNodeLocation[] getNodeLocations() {
 		if (fFileLocation == null) {
-			return new IASTNodeLocation[0];
+			return IASTNodeLocation.EMPTY_ARRAY;
 		}
-		return new IASTNodeLocation[]{fFileLocation};
+		return new IASTNodeLocation[] { fFileLocation };
 	}
 
 	@Override

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

@@ -880,7 +880,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase {
 	//	A a, b;
 	//	return extracted(b, &a) + a.const_method();
 	//}
-	public void _testOutputParameterWithMethodCall() throws Exception {
+	public void testOutputParameterWithMethodCall() throws Exception {
 		getPreferenceStore().setValue(PreferenceConstants.FUNCTION_PASS_OUTPUT_PARAMETERS_BY_POINTER, true);
 		assertRefactoringSuccess();
 	}

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

@@ -57,7 +57,12 @@ import org.eclipse.cdt.internal.ui.refactoring.NameInformation;
  * 
  * @author Mirko Stocker
  */
-public class ExtractExpression extends ExtractedFunctionConstructionHelper {
+public class ExpressionExtractor extends FunctionExtractor {
+	@Override
+	public boolean canChooseReturnValue() {
+		return false;
+	}
+
 	@Override
 	public void constructMethodBody(IASTCompoundStatement compound, List<IASTNode> nodes,
 			List<NameInformation> parameters, ASTRewrite rewrite, TextEditGroup group) {

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

@@ -139,7 +139,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 
 	HashMap<String, Integer> nameTrail;
 
-	private ExtractedFunctionConstructionHelper functionConstructionHelper;
+	private FunctionExtractor extractor;
 	private INodeFactory nodeFactory;
 	DefaultCodeFormatterOptions formattingOptions;
 
@@ -204,7 +204,9 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 				if (initStatus.hasFatalError())
 					return initStatus;
 
-				if (info.getMandatoryReturnVariable() == null) {
+				extractor =	FunctionExtractor.createFor(container.getNodesToWrite());
+
+				if (extractor.canChooseReturnValue() && info.getMandatoryReturnVariable() == null) {
 					chooseReturnVariable();
 				}
 
@@ -214,9 +216,6 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 						PreferenceConstants.getPreferenceScopes(project.getProject()));
 				info.sortParameters(outFirst);
 
-				functionConstructionHelper =
-						ExtractedFunctionConstructionHelper.createFor(container.getNodesToWrite());
-
 				boolean isExtractExpression = container.getNodesToWrite().get(0) instanceof IASTExpression;
 				info.setExtractExpression(isExtractExpression);
 
@@ -639,7 +638,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 		func.setDeclSpecifier(returnType);
 		
 		IASTStandardFunctionDeclarator createdFunctionDeclarator =
-				functionConstructionHelper.createFunctionDeclarator(qname,
+				extractor.createFunctionDeclarator(qname,
 						info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(),
 						info.getParameters(), nodeFactory);
 		func.setDeclarator(createdFunctionDeclarator);
@@ -664,7 +663,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 			subRewrite = rewrite.insertBefore(parent, insertPoint, func, group);
 		}
 
-		functionConstructionHelper.constructMethodBody(compound, container.getNodesToWrite(),
+		extractor.constructMethodBody(compound, container.getNodesToWrite(),
 				info.getParameters(), subRewrite, group);
 
 		// Set return value
@@ -689,7 +688,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 	private IASTDeclSpecifier getReturnType() {
 		IASTNode firstNodeToWrite = container.getNodesToWrite().get(0);
 		NameInformation returnVariable = info.getReturnVariable();
-		return functionConstructionHelper.determineReturnType(firstNodeToWrite,
+		return extractor.determineReturnType(firstNodeToWrite,
 				returnVariable);
 	}
 
@@ -813,13 +812,13 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 	private IASTNode getReturnAssignment(IASTExpressionStatement stmt,
 			IASTExpression callExpression) {
 		IASTNode node = container.getNodesToWrite().get(0);
-		return functionConstructionHelper.createReturnAssignment(node, stmt, callExpression);
+		return extractor.createReturnAssignment(node, stmt, callExpression);
 	}
 	
 	private IASTSimpleDeclaration getDeclaration(IASTName name) {
 		IASTSimpleDeclaration simpleDecl = new CPPASTSimpleDeclaration();
 		IASTStandardFunctionDeclarator declarator =
-				functionConstructionHelper.createFunctionDeclarator(name,
+				extractor.createFunctionDeclarator(name,
 						info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(),
 						info.getParameters(), nodeFactory);
 		simpleDecl.addDeclarator(declarator);
@@ -834,7 +833,7 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 		}
 		simpleDecl.setParent(ast);
 		IASTStandardFunctionDeclarator declarator =
-				functionConstructionHelper.createFunctionDeclarator(name,
+				extractor.createFunctionDeclarator(name,
 						info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(),
 						info.getParameters(), nodeFactory);
 		simpleDecl.addDeclarator(declarator);

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

@@ -47,15 +47,17 @@ import org.eclipse.cdt.internal.ui.refactoring.NameInformation.Indirection;
 /**
  * @author Mirko Stocker
  */
-public abstract class ExtractedFunctionConstructionHelper {
+public abstract class FunctionExtractor {
 	
-	public static ExtractedFunctionConstructionHelper createFor(List<IASTNode> list) {
+	public static FunctionExtractor createFor(List<IASTNode> list) {
 		if (list.get(0) instanceof IASTExpression) {
-			return new ExtractExpression();
+			return new ExpressionExtractor();
 		}
-		return new ExtractStatement();
+		return new StatementExtractor();
 	}
 	
+	public abstract boolean canChooseReturnValue();
+
 	public abstract void constructMethodBody(IASTCompoundStatement compound, List<IASTNode> nodes,
 			List<NameInformation> parameters, ASTRewrite rewrite, TextEditGroup group);
 

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

@@ -36,7 +36,12 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.ASTHelper;
 /**
  * @author Mirko Stocker
  */
-public class ExtractStatement extends ExtractedFunctionConstructionHelper {
+public class StatementExtractor extends FunctionExtractor {
+	@Override
+	public boolean canChooseReturnValue() {
+		return true;
+	}
+
 	@Override
 	public void constructMethodBody(IASTCompoundStatement compound, List<IASTNode> nodes,
 			List<NameInformation> parameters, ASTRewrite rewrite, TextEditGroup group) {

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NamespaceHelper.java

@@ -64,7 +64,7 @@ public class NamespaceHelper {
 			@Override
 			public int visit(ICPPASTNamespaceDefinition namespace) {
 				if (checkFileNameAndLocation(translationUnit.getLocation(), offset, namespace)) {
-					qualifiedName.addName((namespace).getName().copy()); 
+					qualifiedName.addName((namespace).getName().copy(CopyStyle.withLocations)); 
 				}
 				
 				return super.visit(namespace);