From 772066afce8712974bbcdd987c91c44f53df527c Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Sat, 30 Nov 2013 22:39:34 -0800 Subject: [PATCH] Bug 422727 - Extract Function doesn't properly handle auto types --- .../ExtractFunctionRefactoringTest.java | 49 +++++++++- .../ui/refactoring/NameInformation.java | 27 +++++- .../extractfunction/ExpressionExtractor.java | 96 ++++--------------- .../ExtractFunctionRefactoring.java | 27 ++++-- .../extractfunction/FunctionExtractor.java | 30 +++--- .../extractfunction/StatementExtractor.java | 13 ++- .../ui/refactoring/utils/ASTHelper.java | 44 ++------- 7 files changed, 135 insertions(+), 151 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java index 4fb1c6935d9..389ea08f74a 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java @@ -2167,6 +2167,51 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { assertRefactoringSuccess(); } + //Test.h + //#ifndef TEST_H_ + //#define TEST_H_ + // + //struct A { + // typedef A B; + // const B* m(const char* p); + //}; + // + //#endif // TEST_H_ + //==================== + //#ifndef TEST_H_ + //#define TEST_H_ + // + //struct A { + // typedef A B; + // const B* m(const char* p); + //}; + // + //#endif // TEST_H_ + + //Test.cpp + //#include "Test.h" + // + //void test() { + // auto x = new A(); + // const auto* y = ""; + // auto r = /*$*/x->m(y)/*$$*/; + //} + //==================== + //#include "Test.h" + // + //const A::B* extracted(A* x, const char* y) { + // return x->m(y); + //} + // + //void test() { + // auto x = new A(); + // const auto* y = ""; + // auto r = extracted(x, y); + //} + public void testAuto_Bug422727() throws Exception { + assertRefactoringSuccess(); + } + //testString.h //namespace test { // @@ -2300,7 +2345,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //==================== //#include "testString.h" // - //const char endTag(test::string name) { + //const char* endTag(test::string name) { // return ""; //} // @@ -2350,7 +2395,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //==================== //#include "testString.h" // - //const char extracted() { + //const char* extracted() { // return ">" + " pointerOperators) { + IType returnType = determineReturnType(extractedNode); + INodeFactory factory = extractedNode.getTranslationUnit().getASTNodeFactory(); + DeclarationGenerator generator = DeclarationGenerator.create(factory); + IASTDeclarator declarator = generator.createDeclaratorFromType(returnType, null); + ArrayUtil.addAll(pointerOperators, declarator.getPointerOperators()); + return generator.createDeclSpecFromType(returnType); + } + + private IType determineReturnType(IASTNode extractedNode) { List typedefs = getTypedefs(extractedNode); if (extractedNode instanceof IASTExpression) { - IASTExpression exp = (IASTExpression) extractedNode; - INodeFactory factory = extractedNode.getTranslationUnit().getASTNodeFactory(); - DeclarationGenerator generator = DeclarationGenerator.create(factory); - IType expressionType = exp.getExpressionType(); + IType expressionType = ((IASTExpression) extractedNode).getExpressionType(); for (ITypedef typedef : typedefs) { if (typedef.getType().isSameType(expressionType)) { - return generator.createDeclSpecFromType(typedef); + return typedef; } } - return generator.createDeclSpecFromType(expressionType); + return expressionType; } else { // Fallback - return createSimpleDeclSpecifier(Kind.eVoid); + return new CPPBasicType(Kind.eVoid, 0); } } @@ -136,63 +137,6 @@ public class ExpressionExtractor extends FunctionExtractor { return typeDefs; } - private static IASTDeclSpecifier createSimpleDeclSpecifier(IBasicType.Kind type) { - IASTSimpleDeclSpecifier declSpec = new CPPASTSimpleDeclSpecifier(); - declSpec.setType(type); - return declSpec; - } - - private static IASTName findCalledFunctionName(IASTFunctionCallExpression callExpression) { - IASTExpression functionNameExpression = callExpression.getFunctionNameExpression(); - IASTName functionName = null; - - if (functionNameExpression instanceof CPPASTIdExpression) { - CPPASTIdExpression idExpression = (CPPASTIdExpression) functionNameExpression; - functionName = idExpression.getName(); - } else if (functionNameExpression instanceof CPPASTFieldReference) { - CPPASTFieldReference fieldReference = (CPPASTFieldReference) functionNameExpression; - functionName = fieldReference.getFieldName(); - } - return functionName; - } - - @Override - protected boolean hasPointerReturnType(IASTNode node) { - if (node instanceof ICPPASTNewExpression) { - return true; - } else if (!(node instanceof IASTFunctionCallExpression)) { - return false; - } - - IASTName functionName = findCalledFunctionName((IASTFunctionCallExpression) node); - 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.getDeclarator().getPointerOperators().length > 0; - } - } else if (hasDeclaration(function)) { - IASTNode parent = function.getDeclarations()[0].getParent(); - if (parent instanceof CPPASTSimpleDeclaration) { - CPPASTSimpleDeclaration declaration = (CPPASTSimpleDeclaration) parent; - return declaration.getDeclarators().length > 0 && - declaration.getDeclarators()[0].getPointerOperators().length > 0; - } - } - } - } - return false; - } - - private static boolean hasDeclaration(CPPFunction function) { - return function != null && function.getDeclarations() != null && - function.getDeclarations().length > 0; - } - @Override public IASTNode createReturnAssignment(IASTNode node, IASTExpressionStatement stmt, IASTExpression callExpression) { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java index 1dba60fc06b..7c195270662 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2013 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 @@ -70,6 +70,7 @@ 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.ICPPASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNameSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTOperatorName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; @@ -592,7 +593,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { private void addMethod(IASTName methodName, MethodContext context, ASTRewrite rewrite, IASTNode functionToExtractFrom, TextEditGroup group) { - ICPPASTQualifiedName qname = new CPPASTQualifiedName(); + ICPPASTQualifiedName qname = new CPPASTQualifiedName((ICPPASTName) methodName); if (context.getType() == ContextType.METHOD) { if (context.getMethodQName() != null) { for (ICPPASTNameSpecifier segment : context.getMethodQName().getQualifier()) { @@ -600,19 +601,22 @@ public class ExtractFunctionRefactoring extends CRefactoring { } } } - qname.addName(methodName); IASTFunctionDefinition func = new CPPASTFunctionDefinition(); func.setParent(ast); - IASTDeclSpecifier returnType = getReturnType(); + List pointerOperators = new ArrayList(); + IASTDeclSpecifier returnType = getReturnType(pointerOperators); func.setDeclSpecifier(returnType); - IASTStandardFunctionDeclarator createdFunctionDeclarator = + IASTStandardFunctionDeclarator declarator = extractor.createFunctionDeclarator(qname, info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(), info.getParameters(), nodeFactory); - func.setDeclarator(createdFunctionDeclarator); + for (IASTPointerOperator operator : pointerOperators) { + declarator.addPointerOperator(operator); + } + func.setDeclarator(declarator); IASTCompoundStatement compound = new CPPASTCompoundStatement(); func.setBody(compound); @@ -680,11 +684,10 @@ public class ExtractFunctionRefactoring extends CRefactoring { return new CPPASTName(declaration.toCharArray()); } - private IASTDeclSpecifier getReturnType() { + private IASTDeclSpecifier getReturnType(List pointerOperators) { IASTNode firstNodeToWrite = container.getNodesToWrite().get(0); NameInformation returnVariable = info.getReturnVariable(); - return extractor.determineReturnType(firstNodeToWrite, - returnVariable); + return extractor.determineReturnType(firstNodeToWrite, returnVariable, pointerOperators); } protected IASTNode getMethodCall(IASTName astMethodName, Map trailNameTable, @@ -820,7 +823,8 @@ public class ExtractFunctionRefactoring extends CRefactoring { } private IASTSimpleDeclaration getDeclaration(ModificationCollector collector, IASTName name) { - IASTDeclSpecifier declSpec = getReturnType(); + List pointerOperators = new ArrayList(); + IASTDeclSpecifier declSpec = getReturnType(pointerOperators); IASTSimpleDeclaration simpleDecl = nodeFactory.newSimpleDeclaration(declSpec); if (info.isVirtual() && declSpec instanceof ICPPASTDeclSpecifier) { ((ICPPASTDeclSpecifier) declSpec).setVirtual(true); @@ -830,6 +834,9 @@ public class ExtractFunctionRefactoring extends CRefactoring { extractor.createFunctionDeclarator(name, info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(), info.getParameters(), nodeFactory); + for (IASTPointerOperator operator : pointerOperators) { + declarator.addPointerOperator(operator); + } simpleDecl.addDeclarator(declarator); return simpleDecl; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/FunctionExtractor.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/FunctionExtractor.java index 41ce0f3b2d7..877ef66836d 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/FunctionExtractor.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/FunctionExtractor.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2013 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 @@ -23,7 +23,6 @@ import org.eclipse.text.edits.TextEditGroup; import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; -import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement; import org.eclipse.cdt.core.dom.ast.IASTFieldReference; @@ -59,16 +58,21 @@ public abstract class FunctionExtractor { public abstract void constructMethodBody(IASTCompoundStatement compound, List nodes, List parameters, ASTRewrite rewrite, TextEditGroup group); + /** + * Returns the declarator specifier and the pointer operators for the return type of + * the extracted function. + * + * @param extractedNode the first extracted node, used for expression extraction + * @param returnVariable the return variable or {@code null} if the there is no return variable + * @param pointerOperators output parameter - pointer operators for the function declarator + * @return the declarator specifier of the function + */ public abstract IASTDeclSpecifier determineReturnType(IASTNode extractedNode, - NameInformation returnVariable); + NameInformation returnVariable, List pointerOperators); public abstract IASTNode createReturnAssignment(IASTNode node, IASTExpressionStatement stmt, IASTExpression callExpression); - protected boolean hasPointerReturnType(IASTNode node) { - return false; - } - IASTStandardFunctionDeclarator createFunctionDeclarator(IASTName name, IASTStandardFunctionDeclarator functionDeclarator, NameInformation returnVariable, List nodesToWrite, Collection allUsedNames, @@ -82,22 +86,10 @@ public abstract class FunctionExtractor { } } - if (returnVariable != null) { - IASTDeclarator decl = returnVariable.getDeclarator(); - IASTPointerOperator[] pointers = decl.getPointerOperators(); - for (IASTPointerOperator operator : pointers) { - declarator.addPointerOperator(operator.copy(CopyStyle.withLocations)); - } - } - for (IASTParameterDeclaration param : getParameterDeclarations(allUsedNames, nodeFactory)) { declarator.addParameterDeclaration(param); } - if (hasPointerReturnType(nodesToWrite.get(0))) { - declarator.addPointerOperator(nodeFactory.newPointer()); - } - return declarator; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/StatementExtractor.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/StatementExtractor.java index 88aa42d6b14..e7d9675f95c 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/StatementExtractor.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/StatementExtractor.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2013 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 @@ -19,11 +19,13 @@ import org.eclipse.text.edits.TextEditGroup; import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement; 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.IASTPointerOperator; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier; import org.eclipse.cdt.core.dom.ast.INodeFactory; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; @@ -55,9 +57,14 @@ public class StatementExtractor extends FunctionExtractor { @Override public IASTDeclSpecifier determineReturnType(IASTNode extractedNode, - NameInformation returnVariable) { + NameInformation returnVariable, List pointerOperators) { if (returnVariable != null) { - IASTNode decl = ASTHelper.getDeclarationForNode(returnVariable.getDeclarationName()); + IASTName declarationName = returnVariable.getDeclarationName(); + IASTDeclarator declarator = ASTHelper.getDeclaratorForNode(declarationName); + for (IASTPointerOperator pointerOperator : declarator.getPointerOperators()) { + pointerOperators.add(pointerOperator.copy(CopyStyle.withLocations)); + } + IASTNode decl = ASTHelper.getDeclarationForNode(declarationName); IASTDeclSpecifier declSpec = ASTHelper.getDeclarationSpecifier(decl); return declSpec != null ? declSpec.copy(CopyStyle.withLocations) : null; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/ASTHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/ASTHelper.java index a0a9b14d803..02f4cf34f9e 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/ASTHelper.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/ASTHelper.java @@ -30,10 +30,7 @@ import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNameSpecifier; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamespaceDefinition; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTParameterDeclaration; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUsingDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUsingDirective; @@ -43,15 +40,15 @@ public class ASTHelper { private ASTHelper() { } - static public IASTNode getDeclarationForNode(IASTNode tmpNode) { - while (tmpNode != null && !(tmpNode instanceof IASTSimpleDeclaration) && !(tmpNode instanceof IASTParameterDeclaration)) { - tmpNode = tmpNode.getParent(); + static public IASTNode getDeclarationForNode(IASTNode node) { + while (node != null && !(node instanceof IASTSimpleDeclaration) && !(node instanceof IASTParameterDeclaration)) { + node = node.getParent(); } - return tmpNode; + return node; } - static public IASTDeclarator getDeclaratorForNode(IASTNode aNode) { - IASTNode tmpNode = getDeclarationForNode(aNode); + static public IASTDeclarator getDeclaratorForNode(IASTNode node) { + IASTNode tmpNode = getDeclarationForNode(node); IASTDeclarator declarator = null; if (tmpNode instanceof IASTSimpleDeclaration) { @@ -107,36 +104,9 @@ public class ASTHelper { return false; } - public static ArrayList getNamespaces(IASTNode node) { - ArrayList namespaces = new ArrayList(); - for (IASTNode aktNode = node; aktNode != null; aktNode = aktNode.getParent()) { - if (aktNode instanceof ICPPASTNamespaceDefinition) { - namespaces.add(0, (ICPPASTNamespaceDefinition) aktNode); - } else if (aktNode instanceof ICPPASTQualifiedName) { - namespaces.addAll(getNamespaces((ICPPASTQualifiedName) aktNode)); - } - } - return namespaces; - } - - public static ArrayList getNamespaces(ICPPASTQualifiedName qualifiedName) { - ArrayList namespaces = new ArrayList(); - for (ICPPASTNameSpecifier aktQualifiedPartName : qualifiedName.getAllSegments()) { - IBinding binding = aktQualifiedPartName.resolveBinding(); - for (IASTName aktResolvedName : qualifiedName.getTranslationUnit().getDefinitionsInAST(binding)) { - if (aktResolvedName.getParent() instanceof ICPPASTNamespaceDefinition) { - namespaces.add((ICPPASTNamespaceDefinition) aktResolvedName.getParent()); - break; - } - } - } - return namespaces; - } - - public static Collection getCompositTypeSpecifiers(IASTNode baseNode) { + public static Collection getCompositeTypeSpecifiers(IASTNode baseNode) { final Collection specifiers = new ArrayList(); ASTVisitor visitor = new ASTVisitor() { - @Override public int visit(IASTDeclSpecifier declSpec) { specifiers.add(declSpec);