diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ASTComparer.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ASTComparer.java index fca045aba8a..345f9413f2f 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ASTComparer.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ASTComparer.java @@ -6,7 +6,7 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Mike Kucera (IBM) - Initial API and implementation + * Mike Kucera (IBM) - Initial API and implementation *******************************************************************************/ package org.eclipse.cdt.core.parser.tests; @@ -28,7 +28,7 @@ public class ASTComparer extends Assert { private static Set methodsToIgnore = new HashSet(Arrays.asList( // Prevent infinite recursion - "getParent", + "getParent", "getTranslationUnit", "getLastName", @@ -39,6 +39,7 @@ public class ASTComparer extends Assert { // Can be different in copy "isFrozen", "getContainingFilename", + "getOriginalNode", // These methods are problematic "getProblem", @@ -62,18 +63,16 @@ public class ASTComparer extends Assert { "isLValue" )); - public static void assertCopy(IASTNode node1, IASTNode node2) { try { assertCopy(node1, node2, 0); - } catch(Exception e) { + } catch (Exception e) { throw new RuntimeException(e); } } - private static void assertCopy(IASTNode node1, IASTNode node2, int n) throws Exception { - if(node1 == null && node2 == null) + if (node1 == null && node2 == null) return; assertNotNull(node1); assertNotNull(node2); @@ -86,11 +85,11 @@ public class ASTComparer extends Assert { BeanInfo beanInfo = Introspector.getBeanInfo(klass1); - for(PropertyDescriptor property : beanInfo.getPropertyDescriptors()) { + for (PropertyDescriptor property : beanInfo.getPropertyDescriptors()) { Method getter = property.getReadMethod(); - if(getter == null) + if (getter == null) continue; - if(methodsToIgnore.contains(getter.getName())) + if (methodsToIgnore.contains(getter.getName())) continue; if (getter.getAnnotation(Deprecated.class) != null) @@ -99,48 +98,43 @@ public class ASTComparer extends Assert { try { Class returnType = getter.getReturnType(); - if(IASTNode.class.isAssignableFrom(returnType)) { + if (IASTNode.class.isAssignableFrom(returnType)) { //System.out.println(spaces(n) + "Testing1: " + getter.getName()); IASTNode result1 = (IASTNode) getter.invoke(node1); IASTNode result2 = (IASTNode) getter.invoke(node2); - assertCopy(result1, result2, n+1); // members must be same - } - else if(returnType.isArray() && IASTNode.class.isAssignableFrom(returnType.getComponentType())) { + assertCopy(result1, result2, n + 1); // members must be same + } else if (returnType.isArray() && IASTNode.class.isAssignableFrom(returnType.getComponentType())) { //System.out.println(spaces(n) + "Testing2: " + getter.getName()); IASTNode[] result1 = (IASTNode[]) getter.invoke(node1); IASTNode[] result2 = (IASTNode[]) getter.invoke(node2); - if(result1 == null && result2 == null) + if (result1 == null && result2 == null) continue; assertNotNull(result1); assertNotNull(result2); assertEquals(result1.length, result2.length); for(int i = 0; i < result1.length; i++) - assertCopy(result1[i], result2[i], n+1); - } - else if((returnType.isPrimitive() || returnType.equals(String.class)) && !returnType.equals(Void.class)) { + assertCopy(result1[i], result2[i], n + 1); + } else if ((returnType.isPrimitive() || returnType.equals(String.class)) && !returnType.equals(Void.class)) { //System.out.println(spaces(n) + "Testing3: " + getter.getName()); Object result1 = getter.invoke(node1); Object result2 = getter.invoke(node2); assertEquals(result1, result2); } - - } catch(AssertionFailedError e) { + } catch (AssertionFailedError e) { System.out.printf("Failure when calling %s.%s() @(%d,%d)\n", node1.getClass().getSimpleName(), getter.getName(), - ((ASTNode)node1).getOffset(), - ((ASTNode)node1).getLength()); + ((ASTNode) node1).getOffset(), + ((ASTNode) node1).getLength()); throw e; } } } - // private static String spaces(int n) { // char[] spaces = new char[n*2]; // Arrays.fill(spaces, ' '); // return new String(spaces); // } - } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTComment.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTComment.java index 0a0a9017145..48f1047adbf 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTComment.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTComment.java @@ -18,6 +18,10 @@ package org.eclipse.cdt.core.dom.ast; * @noimplement This interface is not intended to be implemented by clients. */ public interface IASTComment extends IASTNode { + /** + * @since 5.4 + */ + public final IASTComment[] EMPTY_COMMENT_ARRAY = {}; /** * Set the comment. @@ -39,5 +43,4 @@ public interface IASTComment extends IASTNode { * @return true if this is a blockcomment */ public boolean isBlockComment(); - } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTIdExpression.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTIdExpression.java index 838b56f3a98..9d7934fc4aa 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTIdExpression.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTIdExpression.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; @@ -17,7 +17,6 @@ package org.eclipse.cdt.core.dom.ast; * @noimplement This interface is not intended to be implemented by clients. */ public interface IASTIdExpression extends IASTExpression, IASTNameOwner { - /** * ID_NAME represents the relationship between an * IASTIdExpression and a IASTName. diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFieldReference.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFieldReference.java index 9e1a55b47ea..3036f284c0b 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFieldReference.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTFieldReference.java @@ -6,8 +6,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * John Camelon (IBM) - Initial API and implementation - * Mike Kucera (IBM) + * John Camelon (IBM) - Initial API and implementation + * Mike Kucera (IBM) *******************************************************************************/ package org.eclipse.cdt.core.dom.ast.cpp; @@ -23,7 +23,6 @@ import org.eclipse.cdt.core.dom.ast.IType; * @noimplement This interface is not intended to be implemented by clients. */ public interface ICPPASTFieldReference extends IASTFieldReference, IASTImplicitNameOwner { - /** * Was template keyword used? * diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTNode.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTNode.java index 89d0b9123db..0cf76a44f6c 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTNode.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTNode.java @@ -14,6 +14,7 @@ package org.eclipse.cdt.internal.core.dom.parser; import org.eclipse.cdt.core.dom.ast.ASTNodeProperty; import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.ExpansionOverlapsBoundaryException; +import org.eclipse.cdt.core.dom.ast.IASTCopyLocation; import org.eclipse.cdt.core.dom.ast.IASTFileLocation; import org.eclipse.cdt.core.dom.ast.IASTImageLocation; import org.eclipse.cdt.core.dom.ast.IASTNode; @@ -373,4 +374,19 @@ public abstract class ASTNode implements IASTNode { protected void setCopyLocation(IASTNode originalNode) { locations = new IASTNodeLocation[] { new ASTCopyLocation(originalNode) }; } + + /** + * If the node is a copy of some other node, returns the original node. + * Otherwise returns the node itself. + */ + public IASTNode getOriginalNode() { + IASTNode node = this; + while (true) { + IASTNodeLocation[] locations = node.getNodeLocations(); + if (locations.length == 0 || !(locations[0] instanceof IASTCopyLocation)) + break; + node = ((IASTCopyLocation) locations[0]).getOriginalNode(); + } + return node; + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTFieldReference.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTFieldReference.java index 094e90b0f39..0b247b0f768 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTFieldReference.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTFieldReference.java @@ -6,10 +6,10 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * John Camelon (IBM Rational Software) - Initial API and implementation - * Yuan Zhang / Beth Tibbitts (IBM Research) - * Bryan Wilkinson (QNX) - * Markus Schorn (Wind River Systems) + * John Camelon (IBM Rational Software) - Initial API and implementation + * Yuan Zhang / Beth Tibbitts (IBM Research) + * Bryan Wilkinson (QNX) + * Markus Schorn (Wind River Systems) *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.c; @@ -30,21 +30,19 @@ import org.eclipse.cdt.internal.core.dom.parser.ProblemType; /** * Field reference in C. */ -public class CASTFieldReference extends ASTNode implements IASTFieldReference, IASTAmbiguityParent, IASTCompletionContext { - +public class CASTFieldReference extends ASTNode + implements IASTFieldReference, IASTAmbiguityParent, IASTCompletionContext { private IASTExpression owner; private IASTName name; private boolean ptr; public CASTFieldReference() { } - public CASTFieldReference(IASTName name, IASTExpression owner) { this(name, owner, false); } - public CASTFieldReference(IASTName name, IASTExpression owner, boolean ptr) { setFieldOwner(owner); setFieldName(name); @@ -111,41 +109,40 @@ public class CASTFieldReference extends ASTNode implements IASTFieldReference, I } @Override - public boolean accept( ASTVisitor action ){ - if( action.shouldVisitExpressions ){ - switch( action.visit( this ) ){ - case ASTVisitor.PROCESS_ABORT : return false; - case ASTVisitor.PROCESS_SKIP : return true; - default : break; + public boolean accept(ASTVisitor action) { + if (action.shouldVisitExpressions) { + switch (action.visit(this)) { + case ASTVisitor.PROCESS_ABORT: return false; + case ASTVisitor.PROCESS_SKIP: return true; + default: break; } } - if( owner != null ) if( !owner.accept( action ) ) return false; - if( name != null ) if( !name.accept( action ) ) return false; + if (owner != null && !owner.accept(action)) return false; + if (name != null && !name.accept(action)) return false; - if( action.shouldVisitExpressions ){ - switch( action.leave( this ) ){ - case ASTVisitor.PROCESS_ABORT : return false; - case ASTVisitor.PROCESS_SKIP : return true; - default : break; + if (action.shouldVisitExpressions) { + switch (action.leave(this)) { + case ASTVisitor.PROCESS_ABORT: return false; + case ASTVisitor.PROCESS_SKIP: return true; + default: break; } } return true; } @Override - public int getRoleForName(IASTName n ) { - if( n == this.name ) + public int getRoleForName(IASTName n) { + if (n == this.name) return r_reference; return r_unclear; } @Override public void replace(IASTNode child, IASTNode other) { - if( child == owner) - { - other.setPropertyInParent( child.getPropertyInParent() ); - other.setParent( child.getParent() ); + if (child == owner) { + other.setPropertyInParent(child.getPropertyInParent()); + other.setParent(child.getParent()); owner = (IASTExpression) other; } } @@ -159,7 +156,6 @@ public class CASTFieldReference extends ASTNode implements IASTFieldReference, I return new ProblemType(ISemanticProblem.TYPE_UNKNOWN_FOR_EXPRESSION); } - @Override public boolean isLValue() { if (isPointerDereference()) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFieldReference.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFieldReference.java index c55a50dd697..42df2fd5c8d 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFieldReference.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTFieldReference.java @@ -18,7 +18,11 @@ import static org.eclipse.cdt.core.dom.ast.IASTExpression.ValueCategory.PRVALUE; import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.ExpressionTypes.glvalueType; import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.ExpressionTypes.prvalueType; import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.ExpressionTypes.typeFromFunctionCall; -import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.*; +import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.ALLCVQ; +import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.CVTYPE; +import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.REF; +import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.TDEF; +import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.getUltimateTypeUptoPointers; import java.util.ArrayList; import java.util.Collection; @@ -53,15 +57,13 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CVQualifier; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil; -public class CPPASTFieldReference extends ASTNode implements ICPPASTFieldReference, IASTAmbiguityParent, - ICPPASTCompletionContext { - +public class CPPASTFieldReference extends ASTNode + implements ICPPASTFieldReference, IASTAmbiguityParent, ICPPASTCompletionContext { private boolean isTemplate; private IASTExpression owner; private IASTName name; private boolean isDeref; - - private IASTImplicitName[] implicitNames = null; + private IASTImplicitName[] implicitNames; public CPPASTFieldReference() { } @@ -148,15 +150,15 @@ public class CPPASTFieldReference extends ASTNode implements ICPPASTFieldReferen if (!isDeref) return implicitNames = IASTImplicitName.EMPTY_NAME_ARRAY; - // collect the function bindings + // Collect the function bindings List functionBindings = new ArrayList(); getFieldOwnerType(functionBindings); if (functionBindings.isEmpty()) return implicitNames = IASTImplicitName.EMPTY_NAME_ARRAY; - // create a name to wrap each binding + // Create a name to wrap each binding implicitNames = new IASTImplicitName[functionBindings.size()]; - int i=-1; + int i= -1; for (ICPPFunction op : functionBindings) { if (op != null && !(op instanceof CPPImplicitFunction)) { CPPASTImplicitName operatorName = new CPPASTImplicitName(OverloadableOperator.ARROW, this); @@ -280,7 +282,6 @@ public class CPPASTFieldReference extends ASTNode implements ICPPASTFieldReferen } return fieldType; } - @Override public ValueCategory getValueCategory() { 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 d755f2ac8a5..2d03fbd4db7 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 @@ -12,10 +12,17 @@ *******************************************************************************/ package org.eclipse.cdt.ui.tests.refactoring.extractfunction; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + import junit.framework.Test; +import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.ltk.core.refactoring.Refactoring; +import org.eclipse.cdt.ui.CUIPlugin; +import org.eclipse.cdt.ui.PreferenceConstants; import org.eclipse.cdt.ui.tests.refactoring.RefactoringTestBase; import org.eclipse.cdt.internal.ui.refactoring.NameInformation; @@ -31,6 +38,10 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { private ExtractFunctionInformation refactoringInfo; private String extractedFunctionName = "extracted"; private String returnValue; + // Map from old names to new ones. + private Map parameterRename = new HashMap(); + // New positions of parameters, or null. + private int[] parameterOrder; private VisibilityEnum visibility = VisibilityEnum.v_private; private boolean virtual; private boolean replaceDuplicates = true; @@ -47,6 +58,26 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { return suite(ExtractFunctionRefactoringTest.class); } + @Override + public void setUp() throws Exception { + super.setUp(); + resetPreferences(); + } + + @Override + public void tearDown() throws Exception { + super.tearDown(); + resetPreferences(); + } + + private void resetPreferences() { + getPreferenceStore().setToDefault(PreferenceConstants.FUNCTION_PASS_OUTPUT_PARAMETERS_BY_POINTER); + } + + private IPreferenceStore getPreferenceStore() { + return CUIPlugin.getDefault().getPreferenceStore(); + } + @Override protected Refactoring createRefactoring() { refactoringInfo = new ExtractFunctionInformation(); @@ -61,14 +92,32 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { if (refactoringInfo.getMandatoryReturnVariable() == null) { if (returnValue != null) { for (NameInformation nameInfo : refactoringInfo.getParameters()) { - nameInfo.setReturnValue(returnValue.equals(String.valueOf(nameInfo.getName().getSimpleID()))); + nameInfo.setReturnValue(returnValue.equals(getName(nameInfo))); } } } + if (!parameterRename.isEmpty()) { + for (NameInformation nameInfo : refactoringInfo.getParameters()) { + String newName = parameterRename.get(getName(nameInfo)); + if (newName != null) + nameInfo.setNewName(newName); + } + } + if (parameterOrder != null) { + List parameters = refactoringInfo.getParameters(); + NameInformation[] originalParameters = parameters.toArray(new NameInformation[parameters.size()]); + for (int i = 0; i < parameterOrder.length; i++) { + parameters.set(parameterOrder[i], originalParameters[i]); + } + } refactoringInfo.setVisibility(visibility); refactoringInfo.setVirtual(virtual); } + private String getName(NameInformation nameInfo) { + return String.valueOf(nameInfo.getName().getSimpleID()); + } + //A.h //#ifndef A_H_ //#define A_H_ @@ -536,7 +585,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //A.cpp //#include "A.h" // - //#define ZWO 2 + //#define TWO 2 // //A::A() { //} @@ -547,7 +596,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //int A::foo() { // int i = 2; // /*$*/++i; - // i += ZWO; + // i += TWO; // help();/*$$*/ // return i; //} @@ -558,7 +607,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //==================== //#include "A.h" // - //#define ZWO 2 + //#define TWO 2 // //A::A() { //} @@ -568,7 +617,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { // //int A::extracted(int i) { // ++i; - // i += ZWO; + // i += TWO; // help(); // return i; //} @@ -698,7 +747,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { // //private: // int help(); - // void extracted(int* i); + // void extracted(int* j); //}; // //#endif /*A_H_*/ @@ -731,8 +780,8 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //A::~A() { //} // - //void A::extracted(int* i) { - // ++*i; + //void A::extracted(int* j) { + // ++*j; // help(); //} // @@ -745,7 +794,94 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //int A::help() { // return 42; //} - public void testPointer() throws Exception { + public void testRenamedParameter() throws Exception { + parameterRename.put("i", "j"); + assertRefactoringSuccess(); + } + + //A.c + //struct A { + // int i; + // int j; + //}; + // + //int test() { + // struct A a = { 1, 2 }; + // return /*$*/a.i + a.j/*$$*/; + //} + //==================== + //struct A { + // int i; + // int j; + //}; + // + //int extracted(const struct A* a) { + // return a->i + a->j; + //} + // + //int test() { + // struct A a = { 1, 2 }; + // return extracted(&a); + //} + public void testInputParameterPassedByPointer() throws Exception { + assertRefactoringSuccess(); + } + + //A.c + //int test() { + // int i = 0; + // int j = 1; + // /*$*/int k = i; + // i = j; + // j = k;/*$$*/ + // return i - j; + //} + //==================== + //void swap(int* i, int* j) { + // int k = *i; + // *i = *j; + // *j = k; + //} + // + //int test() { + // int i = 0; + // int j = 1; + // swap(&i, &j); + // return i - j; + //} + public void testOutputParameterPassedByPointer() throws Exception { + extractedFunctionName = "swap"; + returnValue = NO_RETURN_VALUE; + assertRefactoringSuccess(); + } + + //A.h + //class A { + //public: + // int method(); + // int const_method() const; + //}; + + //A.cpp + //#include "A.h" + // + //int test() { + // A a, b; + // return /*$*/a.method() + b.const_method()/*$$*/ + a.const_method(); + //} + //==================== + //#include "A.h" + // + //int extracted(A b, A* a) { + // return a->method() + b.const_method(); + //} + //int test() { + // A a, b; + // return extracted(b, &a) + a.const_method(); + //} + public void _testOutputParameterWithMethodCall() throws Exception { + // Currently fails due to http://bugs.eclipse.org/bugs/show_bug.cgi?id=370887 + getPreferenceStore().setValue(PreferenceConstants.FUNCTION_PASS_OUTPUT_PARAMETERS_BY_POINTER, true); assertRefactoringSuccess(); } @@ -2562,91 +2698,6 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { public void testDuplicates() throws Exception { assertRefactoringSuccess(); } - - //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(); - // int extracted(int i); - //}; - // - //#endif /*A_H_*/ - - //A.cpp - //#include "A.h" - // - //A::A() { - //} - // - //A::~A() { - // int oo = 99; - // ++oo; - // help(); - //} - // - //int A::foo() { - // int i = 2; - // /*$*/++i; - // help();/*$$*/ - // return i; - //} - // - //int A::help() { - // return 42; - //} - //==================== - //#include "A.h" - // - //A::A() { - //} - // - //A::~A() { - // int oo = 99; - // oo = extracted(oo); - //} - // - //int A::extracted(int i) { - // ++i; - // help(); - // return i; - //} - // - //int A::foo() { - // int i = 2; - // i = extracted(i); - // return i; - //} - // - //int A::help() { - // return 42; - //} - public void testDuplicatesWithDifferentNames() throws Exception { - assertRefactoringSuccess(); - } - //A.h //#ifndef A_H_ //#define A_H_ @@ -2930,7 +2981,96 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //int A::help() { // return 42; //} - public void testDuplicatesWithDifferentNamesAndReturnType() throws Exception { + public void testDuplicatesWithDifferentNames() throws Exception { + assertRefactoringSuccess(); + } + + //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(); + // int extracted(int j, int i); + //}; + // + //#endif /*A_H_*/ + + //A.cpp + //#include "A.h" + // + //A::A() { + //} + // + //A::~A() { + // int aa = 9; + // int bb = 99; + // aa += bb; + // help(); + //} + // + //int A::foo() { + // int i = 2; + // int j = 3; + // /*$*/i += j; + // help();/*$$*/ + // return i; + //} + // + //int A::help() { + // return 42; + //} + //==================== + //#include "A.h" + // + //A::A() { + //} + // + //A::~A() { + // int aa = 9; + // int bb = 99; + // aa = extracted(bb, aa); + //} + // + //int A::extracted(int j, int i) { + // i += j; + // help(); + // return i; + //} + // + //int A::foo() { + // int i = 2; + // int j = 3; + // i = extracted(j, i); + // return i; + //} + // + //int A::help() { + // return 42; + //} + public void testDuplicatesWithDifferentNamesAndReordering() throws Exception { + parameterOrder = new int[] { 1, 0 }; assertRefactoringSuccess(); } @@ -3105,7 +3245,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { //int A::help() { // return 42; //} - public void testDuplicateNameUsedAfterwardsInDuplicateButNotInOriginalSelectionThisIsNoDuplicate() throws Exception { + public void testOutputParameterInDuplicateButNotInOriginal() throws Exception { assertRefactoringSuccess(); } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NameInformation.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NameInformation.java index aba38c6b706..cfb7ec04c2e 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NameInformation.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NameInformation.java @@ -21,7 +21,6 @@ import org.eclipse.cdt.core.dom.ast.IASTArrayDeclarator; import org.eclipse.cdt.core.dom.ast.IASTArrayModifier; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTDeclarator; -import org.eclipse.cdt.core.dom.ast.IASTFileLocation; 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; @@ -30,6 +29,7 @@ import org.eclipse.cdt.core.dom.ast.IASTPointerOperator; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.INodeFactory; import org.eclipse.cdt.core.dom.ast.IType; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.cpp.ICPPNodeFactory; import org.eclipse.cdt.core.dom.rewrite.TypeHelper; import org.eclipse.cdt.core.model.ITranslationUnit; @@ -40,13 +40,16 @@ import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor; * Additional information about an IASTName in code being refactored. */ public class NameInformation { + public static enum Indirection { NONE, POINTER, REFERENCE } + public static final int INDEX_FOR_ADDED = -1; private final IASTName name; private IASTName declarationName; private final List references; - private List referencesAfterCached; - private int lastCachedReferencesHash; + private final List referencesBeforeSelection; + private final List referencesInSelection; + private final List referencesAfterSelection; private boolean isOutput; private boolean mustBeReturnValue; private boolean isWriteAccess; @@ -57,11 +60,15 @@ public class NameInformation { private boolean isDeleted; private String defaultValue; private String newTypeName; + private Indirection indirection; public NameInformation(IASTName name) { this.name = name; this.newName = String.valueOf(name.getSimpleID()); references = new ArrayList(); + referencesBeforeSelection = new ArrayList(); + referencesInSelection = new ArrayList(); + referencesAfterSelection = new ArrayList(); } public static NameInformation createInfoForAddedParameter(String type, String name, @@ -99,6 +106,7 @@ public class NameInformation { void setOutput(boolean isOutput) { this.isOutput = isOutput; + indirection = null; } public boolean isOutputParameter() { @@ -111,6 +119,7 @@ public class NameInformation { public void setMustBeReturnValue(boolean mustBeReturnValue) { this.mustBeReturnValue = mustBeReturnValue; + indirection = null; } public boolean isReturnValue() { @@ -120,6 +129,7 @@ public class NameInformation { public void setReturnValue(boolean isReturnValue) { Assert.isTrue(isReturnValue || !mustBeReturnValue); this.isReturnValue = isReturnValue; + indirection = null; } public String getNewName() { @@ -136,6 +146,7 @@ public class NameInformation { void setWriteAccess(boolean isWriteAceess) { this.isWriteAccess = isWriteAceess; + indirection = null; } public boolean isDeleted() { @@ -182,6 +193,7 @@ public class NameInformation { void setDeclarationName(IASTName declarationName) { Assert.isTrue(declarationName.getParent() instanceof IASTDeclarator); this.declarationName = declarationName; + indirection = null; } public IASTName getName() { @@ -189,11 +201,19 @@ public class NameInformation { } public boolean isRenamed() { - return name == null ? newName != null : String.valueOf(name.getSimpleID()).equals(name); + return name == null ? newName != null : !String.valueOf(name.getSimpleID()).equals(newName); } - void addReference(IASTName name) { + void addReference(IASTName name, int startOffset, int endOffset) { references.add(name); + int nodeOffset = name.getFileLocation().getNodeOffset(); + if (nodeOffset >= endOffset) { + referencesAfterSelection.add(name); + } else if (nodeOffset >= startOffset) { + referencesInSelection.add(name); + } else { + referencesBeforeSelection.add(name); + } } public String getTypeName() { @@ -224,22 +244,20 @@ public class NameInformation { return writer.toString(); } - public List getReferencesAfterSelection(int endOffset) { - if (referencesAfterCached == null || lastCachedReferencesHash != references.hashCode()) { - lastCachedReferencesHash = references.hashCode(); - referencesAfterCached = new ArrayList(); - for (IASTName ref : references) { - IASTFileLocation loc = ref.getFileLocation(); - if (loc.getNodeOffset() >= endOffset) { - referencesAfterCached.add(ref); - } - } - } - return referencesAfterCached; + public List getReferencesBeforeSelection() { + return referencesBeforeSelection; } - public boolean isReferencedAfterSelection(int endOffset) { - return !getReferencesAfterSelection(endOffset).isEmpty(); + public List getReferencesInSelection() { + return referencesInSelection; + } + + public List getReferencesAfterSelection() { + return referencesAfterSelection; + } + + public boolean isReferencedAfterSelection() { + return !referencesAfterSelection.isEmpty(); } public IASTParameterDeclaration getParameterDeclaration(INodeFactory nodeFactory) { @@ -251,29 +269,47 @@ public class NameInformation { IASTDeclSpecifier declSpec = safeCopy(getDeclSpecifier()); IASTDeclarator declarator = createDeclarator(nodeFactory, sourceDeclarator, paramName); - if (isOutputParameter()) { - if (nodeFactory instanceof ICPPNodeFactory && !passOutputByPointer) { - declarator.addPointerOperator(((ICPPNodeFactory) nodeFactory).newReferenceOperator(false)); - } else { - declarator.addPointerOperator(nodeFactory.newPointer()); - } - } else if (declSpec != null && !isWriteAccess) { - IType type = TypeHelper.createType(sourceDeclarator); - if (TypeHelper.shouldBePassedByReference(type, declarationName.getTranslationUnit())) { - if (nodeFactory instanceof ICPPNodeFactory) { - declarator.addPointerOperator(((ICPPNodeFactory) nodeFactory).newReferenceOperator(false)); - } else { - declarator.addPointerOperator(nodeFactory.newPointer()); - } - declSpec.setConst(true); - } + Indirection indirection = getIndirection(); + if (indirection == Indirection.POINTER) { + declarator.addPointerOperator(nodeFactory.newPointer()); + } else if (indirection == Indirection.REFERENCE) { + declarator.addPointerOperator(((ICPPNodeFactory) nodeFactory).newReferenceOperator(false)); + } + + if (indirection != Indirection.NONE && !isWriteAccess && declSpec != null) { + declSpec.setConst(true); } declarator.setNestedDeclarator(sourceDeclarator.getNestedDeclarator()); - return nodeFactory.newParameterDeclaration(declSpec, declarator); } + public Indirection getIndirection() { + if (indirection == null) { + indirection = Indirection.NONE; + boolean isCpp = declarationName.getTranslationUnit() instanceof ICPPASTTranslationUnit; + if (isOutputParameter()) { + if (isCpp && !passOutputByPointer) { + indirection = Indirection.REFERENCE; + } else { + indirection = Indirection.POINTER; + } + } else { + IType type = TypeHelper.createType(getDeclarator()); + if (TypeHelper.shouldBePassedByReference(type, declarationName.getTranslationUnit())) { + if (isCpp) { + if (!isWriteAccess) { + indirection = Indirection.REFERENCE; + } + } else { + indirection = Indirection.POINTER; + } + } + } + } + return indirection; + } + private IASTDeclarator createDeclarator(INodeFactory nodeFactory, IASTDeclarator sourceDeclarator, String name) { IASTName astName = name != null ? @@ -311,5 +347,6 @@ public class NameInformation { public void setPassOutputByPointer(boolean passOutputByPointer) { this.passOutputByPointer = passOutputByPointer; + indirection = null; } } \ No newline at end of file diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java index 48131146709..0d544788747 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/NodeContainer.java @@ -28,6 +28,7 @@ import org.eclipse.core.runtime.preferences.IPreferencesService; import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.DOMException; import org.eclipse.cdt.core.dom.ast.IASTDeclarator; +import org.eclipse.cdt.core.dom.ast.IASTFieldReference; import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation; import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; @@ -73,6 +74,8 @@ public class NodeContainer { return; } names = new ArrayList(); + final int startOffset = getStartOffset(); + final int endOffset = getEndOffset(); IPreferencesService preferences = Platform.getPreferencesService(); final boolean passOutputByPointer = preferences.getBoolean(CUIPlugin.PLUGIN_ID, @@ -87,34 +90,36 @@ public class NodeContainer { @Override public int visit(IASTName name) { - IBinding binding = name.resolveBinding(); - - if (binding instanceof ICPPBinding && !(binding instanceof ICPPTemplateTypeParameter)) { - ICPPBinding cppBinding = (ICPPBinding) binding; - try { - if (!cppBinding.isGloballyQualified()) { - NameInformation nameInfo = new NameInformation(name); - nameInfo.setPassOutputByPointer(passOutputByPointer); - IASTName[] refs = name.getTranslationUnit().getReferences(binding); - for (IASTName ref : refs) { - nameInfo.addReference(ref); + if (name.getPropertyInParent() != IASTFieldReference.FIELD_NAME) { + IBinding binding = name.resolveBinding(); + + if (binding instanceof ICPPBinding && !(binding instanceof ICPPTemplateTypeParameter)) { + ICPPBinding cppBinding = (ICPPBinding) binding; + try { + if (!cppBinding.isGloballyQualified()) { + NameInformation nameInfo = new NameInformation(name); + nameInfo.setPassOutputByPointer(passOutputByPointer); + IASTName[] refs = name.getTranslationUnit().getReferences(binding); + for (IASTName ref : refs) { + nameInfo.addReference(ref, startOffset, endOffset); + } + names.add(nameInfo); } - names.add(nameInfo); + } catch (DOMException e) { + ILog logger = CUIPlugin.getDefault().getLog(); + IStatus status = new Status(IStatus.WARNING, CUIPlugin.PLUGIN_ID, + e.getMessage(), e); + logger.log(status); } - } 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); + } else if (binding instanceof IVariable) { + NameInformation nameInformation = new NameInformation(name); + + IASTName[] refs = name.getTranslationUnit().getReferences(binding); + for (IASTName ref : refs) { + nameInformation.addReference(ref, startOffset, endOffset); + } + names.add(nameInformation); } - } else if (binding instanceof IVariable) { - NameInformation nameInformation = new NameInformation(name); - - IASTName[] refs = name.getTranslationUnit().getReferences(binding); - for (IASTName ref : refs) { - nameInformation.addReference(ref); - } - names.add(nameInformation); } return super.visit(name); } @@ -152,12 +157,11 @@ public class NodeContainer { Set declarations = new HashSet(); interfaceNames = new ArrayList(); - int endOffset = getEndOffset(); for (NameInformation nameInfo : names) { IASTName declarationName = nameInfo.getDeclarationName(); if (declarations.add(declarationName)) { if (isDeclaredInSelection(nameInfo)) { - if (nameInfo.isReferencedAfterSelection(endOffset)) { + if (nameInfo.isReferencedAfterSelection()) { nameInfo.setMustBeReturnValue(true); interfaceNames.add(nameInfo); } @@ -173,7 +177,7 @@ public class NodeContainer { } } } - if (nameInfo.isWriteAccess() && nameInfo.isReferencedAfterSelection(endOffset)) { + if (nameInfo.isWriteAccess() && nameInfo.isReferencedAfterSelection()) { nameInfo.setOutput(true); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java index d7be786dfcf..9409b833d72 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2012 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 @@ -8,11 +8,13 @@ * * Contributors: * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.eclipse.text.edits.TextEditGroup; @@ -24,7 +26,6 @@ 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; import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle; @@ -44,7 +45,6 @@ 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.CPPASTReturnStatement; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclSpecifier; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration; @@ -58,30 +58,27 @@ import org.eclipse.cdt.internal.ui.refactoring.NameInformation; * @author Mirko Stocker */ public class ExtractExpression extends ExtractedFunctionConstructionHelper { - private final static char[] ZERO= { '0' }; - @Override - public void constructMethodBody(IASTCompoundStatement compound, List list, - ASTRewrite rewrite, TextEditGroup group) { + public void constructMethodBody(IASTCompoundStatement compound, List nodes, + List parameters, ASTRewrite rewrite, TextEditGroup group) { CPPASTReturnStatement statement = new CPPASTReturnStatement(); - IASTExpression nullReturnExp = - new CPPASTLiteralExpression(IASTLiteralExpression.lk_integer_constant, ZERO); - statement.setReturnValue(nullReturnExp); - ASTRewrite nestedRewrite = rewrite.insertBefore(compound, null, statement, group); - - nestedRewrite.replace(nullReturnExp, getExpression(list), group); + statement.setReturnValue(getExpression(nodes)); + ASTRewrite subRewrite = rewrite.insertBefore(compound, null, statement, group); + Map changedParameters = getChangedParameterReferences(parameters); + INodeFactory nodeFactory = nodes.get(0).getTranslationUnit().getASTNodeFactory(); + adjustParameterReferences(statement, changedParameters, nodeFactory, subRewrite, group); } - private IASTExpression getExpression(List list) { - if (list.size() > 1) { + private IASTExpression getExpression(List nodes) { + if (nodes.size() > 1) { CPPASTBinaryExpression expression = new CPPASTBinaryExpression(); - expression.setParent(list.get(0).getParent()); - expression.setOperand1((IASTExpression) list.get(0).copy(CopyStyle.withLocations)); - expression.setOperator(((IASTBinaryExpression) list.get(1).getParent()).getOperator()); - expression.setOperand2(getExpression(list.subList(1, list.size()))); + expression.setParent(nodes.get(0).getParent()); + expression.setOperand1((IASTExpression) nodes.get(0).copy(CopyStyle.withLocations)); + expression.setOperator(((IASTBinaryExpression) nodes.get(1).getParent()).getOperator()); + expression.setOperand2(getExpression(nodes.subList(1, nodes.size()))); return expression; } else { - return (IASTExpression) list.get(0).copy(CopyStyle.withLocations); + return (IASTExpression) nodes.get(0).copy(CopyStyle.withLocations); } } 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 3e8b22e9dff..cb72987ec55 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 @@ -61,6 +61,7 @@ import org.eclipse.cdt.core.dom.ast.IASTReturnStatement; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTStandardFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.IASTStatement; +import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression; import org.eclipse.cdt.core.dom.ast.IBasicType; import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IField; @@ -101,7 +102,6 @@ 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.CPPASTSimpleDeclaration; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateDeclaration; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPNodeFactory; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor; @@ -113,6 +113,7 @@ import org.eclipse.cdt.internal.ui.refactoring.MethodContext; import org.eclipse.cdt.internal.ui.refactoring.MethodContext.ContextType; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; import org.eclipse.cdt.internal.ui.refactoring.NameInformation; +import org.eclipse.cdt.internal.ui.refactoring.NameInformation.Indirection; import org.eclipse.cdt.internal.ui.refactoring.NodeContainer; import org.eclipse.cdt.internal.ui.refactoring.utils.ASTHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.CPPASTAllVisitor; @@ -138,8 +139,8 @@ public class ExtractFunctionRefactoring extends CRefactoring { HashMap nameTrail; - private ExtractedFunctionConstructionHelper extractedFunctionConstructionHelper; - private final INodeFactory nodeFactory = CPPNodeFactory.getDefault(); + private ExtractedFunctionConstructionHelper functionConstructionHelper; + private INodeFactory nodeFactory; DefaultCodeFormatterOptions formattingOptions; public ExtractFunctionRefactoring(IFile file, ISelection selection, @@ -167,6 +168,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { return status; } + nodeFactory = ast.getASTNodeFactory(); container = findExtractableNodes(); sm.worked(1); @@ -212,7 +214,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { PreferenceConstants.getPreferenceScopes(project.getProject())); info.sortParameters(outFirst); - extractedFunctionConstructionHelper = + functionConstructionHelper = ExtractedFunctionConstructionHelper.createFor(container.getNodesToWrite()); boolean isExtractExpression = container.getNodesToWrite().get(0) instanceof IASTExpression; @@ -354,7 +356,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { } } - private void createMethodCalls(final IASTName astMethodName, MethodContext context, + private void createMethodCalls(IASTName methodName, MethodContext context, ModificationCollector collector) throws CoreException { String title; if (context.getType() == MethodContext.ContextType.METHOD) { @@ -363,7 +365,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { title = Messages.ExtractFunctionRefactoring_CreateFunctionCall; } - IASTNode methodCall = getMethodCall(astMethodName); + IASTNode methodCall = getMethodCall(methodName); IASTNode firstNodeToWrite = container.getNodesToWrite().get(0); ASTRewrite rewriter = collector.rewriterForTranslationUnit( @@ -377,7 +379,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { insertCallIntoTree(methodCall, container.getNodesToWrite(), rewriter, editGroup); if (info.isReplaceDuplicates()) { - replaceSimilar(collector, astMethodName); + replaceSimilar(collector, methodName); } for (IASTNode node : container.getNodesToWrite()) { @@ -415,7 +417,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { return binExp; } - private void createMethodDefinition(final IASTName astMethodName, MethodContext context, + private void createMethodDefinition(final IASTName methodName, MethodContext context, IASTNode firstNode, ModificationCollector collector) { IASTFunctionDefinition node = NodeHelper.findFunctionDefinitionInAncestors(firstNode); if (node != null) { @@ -427,7 +429,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { } ASTRewrite rewriter = collector.rewriterForTranslationUnit(node.getTranslationUnit()); - addMethod(astMethodName, context, rewriter, node, new TextEditGroup(title)); + addMethod(methodName, context, rewriter, node, new TextEditGroup(title)); } } @@ -618,17 +620,17 @@ public class ExtractFunctionRefactoring extends CRefactoring { return true; } - private void addMethod(IASTName astMethodName, MethodContext context, ASTRewrite rewriter, + private void addMethod(IASTName methodName, MethodContext context, ASTRewrite rewrite, IASTNode insertPoint, TextEditGroup group) { ICPPASTQualifiedName qname = new CPPASTQualifiedName(); if (context.getType() == ContextType.METHOD) { if (context.getMethodQName() != null) { - for (int i = 0; i < (context.getMethodQName().getNames().length - 1); i++) { + for (int i = 0; i < context.getMethodQName().getNames().length - 1; i++) { qname.addName(new CPPASTName(context.getMethodQName().getNames()[i].toCharArray())); } } } - qname.addName(astMethodName); + qname.addName(methodName); IASTFunctionDefinition func = new CPPASTFunctionDefinition(); func.setParent(ast); @@ -637,32 +639,33 @@ public class ExtractFunctionRefactoring extends CRefactoring { func.setDeclSpecifier(returnType); IASTStandardFunctionDeclarator createdFunctionDeclarator = - extractedFunctionConstructionHelper.createFunctionDeclarator(qname, + functionConstructionHelper.createFunctionDeclarator(qname, info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(), - info.getParameters(), ast.getASTNodeFactory()); + info.getParameters(), nodeFactory); func.setDeclarator(createdFunctionDeclarator); IASTCompoundStatement compound = new CPPASTCompoundStatement(); func.setBody(compound); - ASTRewrite subRW; - if (insertPoint.getParent() instanceof ICPPASTTemplateDeclaration) { + ASTRewrite subRewrite; + IASTNode parent = insertPoint.getParent(); + if (parent instanceof ICPPASTTemplateDeclaration) { + ICPPASTTemplateDeclaration parentTemplate = (ICPPASTTemplateDeclaration) parent; CPPASTTemplateDeclaration templateDeclaration = new CPPASTTemplateDeclaration(); templateDeclaration.setParent(ast); - for (ICPPASTTemplateParameter param : ((ICPPASTTemplateDeclaration) insertPoint.getParent()).getTemplateParameters()) { + for (ICPPASTTemplateParameter param : parentTemplate.getTemplateParameters()) { templateDeclaration.addTemplateParameter(param.copy(CopyStyle.withLocations)); } templateDeclaration.setDeclaration(func); - subRW = rewriter.insertBefore(insertPoint.getParent().getParent(), insertPoint.getParent(), - templateDeclaration, group); + subRewrite = rewrite.insertBefore(parent.getParent(), parent, templateDeclaration, group); } else { - subRW = rewriter.insertBefore(insertPoint.getParent(), insertPoint, func, group); + subRewrite = rewrite.insertBefore(parent, insertPoint, func, group); } - - extractedFunctionConstructionHelper.constructMethodBody(compound, - container.getNodesToWrite(), subRW, group); + + functionConstructionHelper.constructMethodBody(compound, container.getNodesToWrite(), + info.getParameters(), subRewrite, group); // Set return value NameInformation returnVariable = info.getReturnVariable(); @@ -675,7 +678,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { expr.setName(new CPPASTName(returnVariable.getNewName().toCharArray())); } returnStmt.setReturnValue(expr); - subRW.insertBefore(compound, null, returnStmt, group); + subRewrite.insertBefore(compound, null, returnStmt, group); } } @@ -686,7 +689,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { private IASTDeclSpecifier getReturnType() { IASTNode firstNodeToWrite = container.getNodesToWrite().get(0); NameInformation returnVariable = info.getReturnVariable(); - return extractedFunctionConstructionHelper.determineReturnType(firstNodeToWrite, + return functionConstructionHelper.determineReturnType(firstNodeToWrite, returnVariable); } @@ -755,28 +758,28 @@ public class ExtractFunctionRefactoring extends CRefactoring { return getReturnAssignment(stmt, callExpression, retName); } - private IASTNode getMethodCall(IASTName astMethodName) { - IASTExpressionStatement stmt = new CPPASTExpressionStatement(); + private IASTNode getMethodCall(IASTName methodName) { + IASTExpressionStatement statement = new CPPASTExpressionStatement(); IASTFunctionCallExpression callExpression = new CPPASTFunctionCallExpression(); IASTIdExpression idExpression = new CPPASTIdExpression(); - idExpression.setName(new CPPASTName(astMethodName.toCharArray())); + idExpression.setName(new CPPASTName(methodName.toCharArray())); List args = getCallParameters(); callExpression.setArguments(args.toArray(new IASTInitializerClause[args.size()])); callExpression.setFunctionNameExpression(idExpression); if (info.getReturnVariable() == null) { - return getReturnAssignment(stmt, callExpression); + return getReturnAssignment(statement, callExpression); } - IASTName retname = newName(info.getReturnVariable().getName()); - return getReturnAssignment(stmt, callExpression, retname); + IASTName retName = newName(info.getReturnVariable().getName()); + return getReturnAssignment(statement, callExpression, retName); } private IASTNode getReturnAssignment(IASTExpressionStatement stmt, IASTFunctionCallExpression callExpression, IASTName retname) { if (info.getReturnVariable().equals(info.getMandatoryReturnVariable())) { - IASTSimpleDeclaration orgDecl = NodeHelper.findSimpleDeclarationInParents(info - .getReturnVariable().getDeclarationName()); + IASTSimpleDeclaration orgDecl = NodeHelper.findSimpleDeclarationInParents( + info.getReturnVariable().getDeclarationName()); IASTSimpleDeclaration decl = new CPPASTSimpleDeclaration(); decl.setDeclSpecifier(orgDecl.getDeclSpecifier().copy(CopyStyle.withLocations)); @@ -810,15 +813,15 @@ public class ExtractFunctionRefactoring extends CRefactoring { private IASTNode getReturnAssignment(IASTExpressionStatement stmt, IASTExpression callExpression) { IASTNode node = container.getNodesToWrite().get(0); - return extractedFunctionConstructionHelper.createReturnAssignment(node, stmt, callExpression); + return functionConstructionHelper.createReturnAssignment(node, stmt, callExpression); } private IASTSimpleDeclaration getDeclaration(IASTName name) { IASTSimpleDeclaration simpleDecl = new CPPASTSimpleDeclaration(); IASTStandardFunctionDeclarator declarator = - extractedFunctionConstructionHelper.createFunctionDeclarator(name, + functionConstructionHelper.createFunctionDeclarator(name, info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(), - info.getParameters(), ast.getASTNodeFactory()); + info.getParameters(), nodeFactory); simpleDecl.addDeclarator(declarator); return simpleDecl; } @@ -831,9 +834,9 @@ public class ExtractFunctionRefactoring extends CRefactoring { } simpleDecl.setParent(ast); IASTStandardFunctionDeclarator declarator = - extractedFunctionConstructionHelper.createFunctionDeclarator(name, + functionConstructionHelper.createFunctionDeclarator(name, info.getDeclarator(), info.getReturnVariable(), container.getNodesToWrite(), - info.getParameters(), ast.getASTNodeFactory()); + info.getParameters(), nodeFactory); simpleDecl.addDeclarator(declarator); return simpleDecl; } @@ -881,8 +884,10 @@ public class ExtractFunctionRefactoring extends CRefactoring { if (!container.isDeclaredInSelection(nameInfo)) { IASTName declaration = nameInfo.getDeclarationName(); if (declarations.add(declaration)) { - IASTIdExpression expression = new CPPASTIdExpression(); - expression.setName(newName(declaration)); + IASTExpression expression = nodeFactory.newIdExpression(newName(declaration)); + if (nameInfo.getIndirection() == Indirection.POINTER) { + expression = nodeFactory.newUnaryExpression(IASTUnaryExpression.op_amper, expression); + } args.add(expression); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractStatement.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractStatement.java index 5e3c0fa0cf3..b9af77c0177 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractStatement.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2012 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 @@ -8,10 +8,12 @@ * * Contributors: * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; import java.util.List; +import java.util.Map; import org.eclipse.text.edits.TextEditGroup; @@ -19,9 +21,11 @@ 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.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.IASTSimpleDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.INodeFactory; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclSpecifier; @@ -35,9 +39,12 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.ASTHelper; public class ExtractStatement extends ExtractedFunctionConstructionHelper { @Override public void constructMethodBody(IASTCompoundStatement compound, List nodes, - ASTRewrite rewrite, TextEditGroup group) { + List parameters, ASTRewrite rewrite, TextEditGroup group) { + Map changedParameters = getChangedParameterReferences(parameters); + INodeFactory nodeFactory = nodes.get(0).getTranslationUnit().getASTNodeFactory(); for (IASTNode node : nodes) { - rewrite.insertBefore(compound, null, node, group); + ASTRewrite subRewrite = rewrite.insertBefore(compound, null, node, group); + adjustParameterReferences(node, changedParameters, nodeFactory, subRewrite, group); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java index ab3f2637d12..0ba32f0e614 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractedFunctionConstructionHelper.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2012 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 @@ -8,31 +8,41 @@ * * Contributors: * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; 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; +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.IASTParameterDeclaration; import org.eclipse.cdt.core.dom.ast.IASTPointerOperator; import org.eclipse.cdt.core.dom.ast.IASTStandardFunctionDeclarator; +import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression; import org.eclipse.cdt.core.dom.ast.INodeFactory; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; +import org.eclipse.cdt.internal.core.dom.parser.ASTNode; + import org.eclipse.cdt.internal.ui.refactoring.NameInformation; +import org.eclipse.cdt.internal.ui.refactoring.NameInformation.Indirection; /** * @author Mirko Stocker @@ -46,8 +56,8 @@ public abstract class ExtractedFunctionConstructionHelper { return new ExtractStatement(); } - public abstract void constructMethodBody(IASTCompoundStatement compound, List list, - ASTRewrite rewrite, TextEditGroup group); + public abstract void constructMethodBody(IASTCompoundStatement compound, List nodes, + List parameters, ASTRewrite rewrite, TextEditGroup group); public abstract IASTDeclSpecifier determineReturnType(IASTNode extractedNode, NameInformation returnVariable); @@ -99,4 +109,83 @@ public abstract class ExtractedFunctionConstructionHelper { } return result; } + + /** + * Adjusts parameter references under the given node to account for renamed parameters and + * parameters passed by pointer. + * + * @param node the root node of the AST subtree to be adjusted + * @param changedParameters the map from references to changed parameters to parameters + * themselves + * @param rewrite the rewrite for the node + * @param group the edit group to add the changes to + */ + protected static void adjustParameterReferences(IASTNode node, + final Map changedParameters, final INodeFactory nodeFactory, + final ASTRewrite rewrite, final TextEditGroup group) { + if (changedParameters.isEmpty()) + return; + node.accept(new ASTVisitor() { + { + shouldVisitNames = true; + } + + @Override + public int visit(IASTName name) { + NameInformation param = changedParameters.get(((ASTNode) name).getOriginalNode()); + if (param != null) { + IASTName newName = null; + if (param.isRenamed()) { + newName = nodeFactory.newName(param.getNewName().toCharArray()); + } + if (param.getIndirection() == Indirection.POINTER && + name.getPropertyInParent() == IASTIdExpression.ID_NAME) { + IASTIdExpression idExp = (IASTIdExpression) name.getParent(); + if (idExp.getPropertyInParent() == IASTFieldReference.FIELD_OWNER && + !((IASTFieldReference) idExp.getParent()).isPointerDereference()) { + IASTFieldReference dotRef = (IASTFieldReference) idExp.getParent(); + IASTFieldReference arrowRef = dotRef.copy(CopyStyle.withLocations); + arrowRef.setIsPointerDereference(true); + if (newName != null) { + idExp = (IASTIdExpression) arrowRef.getFieldOwner(); + idExp.setName(newName); + } + rewrite.replace(dotRef, arrowRef, group); + } else { + IASTIdExpression newIdExp = idExp.copy(CopyStyle.withLocations); + IASTUnaryExpression starExp = + nodeFactory.newUnaryExpression(IASTUnaryExpression.op_star, newIdExp); + if (newName != null) { + newIdExp.setName(newName); + } + rewrite.replace(idExp, starExp, group); + } + } else if (newName != null) { + rewrite.replace(name, newName, group); + } + } + return super.visit(name); + } + }); + } + + /** + * Returns a map from references to parameters inside the body of the extracted function to + * the parameters themselves. The map includes only the parameters that are either renamed, + * or passed by pointer. + * + * @param parameters the function parameters. + * @return a map from references to parameters to parameters themselves. + */ + protected static Map getChangedParameterReferences(List parameters) { + final Map referenceLookupMap = new HashMap(); + for (NameInformation param : parameters) { + if (param.isRenamed() || param.getIndirection() == Indirection.POINTER) { + for (IASTName name : param.getReferencesInSelection()) { + referenceLookupMap.put(name, param); + } + } + } + return referenceLookupMap; + } }