From d184e414d04125c66d85cf0c55e6b921d4d7c420 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Mon, 16 Jan 2012 22:10:28 -0800 Subject: [PATCH] Fixed unnecessary passing parameters by reference. --- .../refactoring/ExtractExpression.rts | 4 +- .../resources/refactoring/ExtractMethod.rts | 226 ++++++------------ .../refactoring/ExtractMethodDuplicates.rts | 59 ++--- .../refactoring/ExtractMethodPreprocessor.rts | 18 +- .../ExtractFunctionRefactoringTest.java | 25 +- .../ExtractFunctionTestSuite.java | 12 +- .../ui/refactoring/NodeContainer.java | 146 ++++++----- .../extractfunction/ChooserComposite.java | 13 +- .../ExtractFunctionInformation.java | 20 +- .../ExtractFunctionRefactoring.java | 75 +++--- .../ExtractedFunctionConstructionHelper.java | 10 +- .../extractfunction/SimilarFinderVisitor.java | 15 +- .../ExtractLocalVariableRefactoring.java | 1 - 13 files changed, 236 insertions(+), 388 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts index 283b5f3c248..1f6f14fa15e 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts @@ -662,11 +662,9 @@ void foo() { } //! Extract macro //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest - //@.config filename=test.cpp methodname=bar - //@test.cpp #define five 5 #define ADD(a, b) a + b @@ -681,7 +679,7 @@ int main() { #define five 5 #define ADD(a, b) a + b -int bar(int& i) { +int bar(int i) { return ADD(i, five); } diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts index 368414c0540..7e1a136079d 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts @@ -1,4 +1,4 @@ -//!ExtractFunctionRefactoringTest variable defined in scope +//!Extract function with variable defined in scope //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -74,7 +74,7 @@ int A::foo() { int A::help() { return 42; } -//!ExtractFunctionRefactoringTest with comment +//!Extract function with comment //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -156,10 +156,8 @@ int A::help() { filename=A.cpp methodname=exp replaceduplicates=false -returnvalue=false -returnparameterindex=0 -//!Extract Function first extracted statement with leading comment +//!Extract function first extracted statement with leading comment //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@main.cpp int main() { @@ -186,10 +184,8 @@ int main() { filename=main.cpp methodname=exp replaceduplicates=false -returnvalue=false -returnparameterindex=0 -//!Extract Function last extracted statement with trailling comment +//!Extract function last extracted statement with trailling comment //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@main.cpp int main() { @@ -213,10 +209,8 @@ int main() { filename=main.cpp methodname=exp replaceduplicates=false -returnvalue=false -returnparameterindex=0 -//!ExtractFunctionRefactoringTest with two variable defined in scope +//!Extract function with two variable defined in scope //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config fatalerror=true @@ -259,7 +253,7 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest with named typed field +//!Extract function with named typed field //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -352,7 +346,7 @@ public: #endif /*B_H_*/ -//!ExtractFunctionRefactoringTest with named typed variable defined in scope +//!Extract function with named typed variable defined in scope //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -445,7 +439,7 @@ public: #endif /*B_H_*/ -//!ExtractFunctionRefactoringTest with ObjectStyleMacro +//!Extract function with ObjectStyleMacro //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -530,7 +524,7 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest with FunctionStyleMacro +//!Extract function with FunctionStyleMacro //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -615,7 +609,7 @@ int A::help() { return 42; } -//!ExtractMethod with Pointer +//!Extract method with pointer //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -645,7 +639,7 @@ public: private: int help(); - void exp(int*& i); + void exp(int* i); }; #endif /*A_H_*/ @@ -679,7 +673,7 @@ A::A() { A::~A() { } -void A::exp(int*& i) { +void A::exp(int* i) { ++*i; help(); } @@ -694,7 +688,7 @@ int A::help() { return 42; } -//!ExtractMethod with Pointer and Comment at the end +//!Extract method with pointer and comment at the end //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -711,7 +705,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -724,11 +717,10 @@ public: private: int help(); - void exp(int*& i); + void exp(int* i); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -749,7 +741,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -759,7 +750,7 @@ A::A() { A::~A() { } -void A::exp(int*& i) { +void A::exp(int* i) { ++*i; help(); } @@ -774,8 +765,7 @@ int A::foo() { int A::help() { return 42; } - -//!ExtractMethod with Pointer and Comment +//!Extract method with pointer and comment //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ @@ -792,7 +782,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -805,11 +794,10 @@ public: private: int help(); - void exp(int*& i); + void exp(int* i); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -830,7 +818,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -840,7 +827,7 @@ A::A() { A::~A() { } -void A::exp(int*& i) { +void A::exp(int* i) { ++*i; help(); } @@ -855,12 +842,11 @@ int A::foo() { int A::help() { return 42; } - -//!ExtractFunctionRefactoringTest with Return Value +//!Extract function with return value //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -876,7 +862,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -893,7 +878,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -913,7 +897,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -938,12 +921,11 @@ int A::foo() { int A::help() { return 42; } - -//!ExtractFunctionRefactoringTest with Return Value and Ref Parameter +//!Extract function with return value and ref parameter //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -959,7 +941,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -972,11 +953,10 @@ public: private: int help(); - int exp(int i, int& b); + int exp(int i, int b); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -999,7 +979,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1009,7 +988,7 @@ A::A() { A::~A() { } -int A::exp(int i, int& b) { +int A::exp(int i, int b) { ++i; i = i + b; help(); @@ -1027,12 +1006,11 @@ int A::foo() { int A::help() { return 42; } - -//!ExtractFunctionRefactoringTest with Return Value and Ref Parameter and some more not used aferwards +//!Extract function with return value and ref parameter and some more not used afterwards //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -1048,7 +1026,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1061,11 +1038,10 @@ public: private: int help(); - int exp(int i, B* b, int y, float& x); + int exp(int i, B* b, int y, float x); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1091,7 +1067,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1101,7 +1076,7 @@ A::A() { A::~A() { } -int A::exp(int i, B* b, int y, float& x) { +int A::exp(int i, B* b, int y, float x) { ++i; b->hello(y); i = i + x; @@ -1122,12 +1097,11 @@ int A::foo() { int A::help() { return 42; } - -//!ExtractFunctionRefactoringTest with Return Value take the second and Ref Parameter and some more not used aferwards +//!Extract function with return value take the second and ref parameter and some more not used aferwards //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config returnparameterindex=1 -returnvalue=true +returnvalue=x //@A.h #ifndef A_H_ #define A_H_ @@ -1145,7 +1119,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1164,7 +1137,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1190,7 +1162,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1221,7 +1192,6 @@ int A::foo() { int A::help() { return 42; } - //@B.h #ifndef B_H_ #define B_H_ @@ -1234,12 +1204,11 @@ public: }; #endif /*B_H_*/ - -//!ExtractFunctionRefactoringTest with Return Value and a lot Ref Parameter +//!Extract function with return value and a lot ref parameter //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -1257,7 +1226,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1272,11 +1240,10 @@ public: private: int help(); - int exp(int i, B*& b, int& y, float& x); + int exp(int i, B* b, int y, float x); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1303,7 +1270,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1313,7 +1279,7 @@ A::A() { A::~A() { } -int A::exp(int i, B*& b, int& y, float& x) { +int A::exp(int i, B* b, int y, float x) { ++i; b->hello(y); i = i + x; @@ -1335,7 +1301,6 @@ int A::foo() { int A::help() { return 42; } - //@B.h #ifndef B_H_ #define B_H_ @@ -1348,12 +1313,11 @@ public: }; #endif /*B_H_*/ - -//!ExtractFunctionRefactoringTest with Return Value take the second and Ref Parameter +//!Extract function with return value take the second and ref parameter //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config returnparameterindex=1 -returnvalue=true +returnvalue=b //@A.h #ifndef A_H_ #define A_H_ @@ -1371,7 +1335,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1386,11 +1349,10 @@ public: private: int help(); - B* exp(int& i, B* b, int& y, float& x); + B* exp(int& i, B* b, int y, float x); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1417,7 +1379,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1427,7 +1388,7 @@ A::A() { A::~A() { } -B* A::exp(int& i, B* b, int& y, float& x) { +B* A::exp(int& i, B* b, int y, float x) { ++i; b->hello(y); i = i + x; @@ -1449,7 +1410,6 @@ int A::foo() { int A::help() { return 42; } - //@B.h #ifndef B_H_ #define B_H_ @@ -1462,9 +1422,13 @@ public: }; #endif /*B_H_*/ - -//!ExtractFunctionRefactoringTest protected visibility +//!Extract function with protected visibility //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest +//@.config +filename=A.cpp +methodname=exp +replaceduplicates=false +visibility=protected //@A.h #ifndef A_H_ #define A_H_ @@ -1540,16 +1504,13 @@ int A::foo() { int A::help() { return 42; } +//!Extract function with public visibility +//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=A.cpp methodname=exp replaceduplicates=false -returnvalue=false -returnparameterindex=0 -visibility=protected - -//!ExtractFunctionRefactoringTest public visibility -//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest +visibility=public //@A.h #ifndef A_H_ #define A_H_ @@ -1565,7 +1526,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1582,7 +1542,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1602,7 +1561,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1626,17 +1584,12 @@ int A::foo() { int A::help() { return 42; } - +//!Extract function with const Method Bug # 46 +//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=A.cpp methodname=exp replaceduplicates=false -returnvalue=false -returnparameterindex=0 -visibility=public - -//!ExtractFunctionRefactoringTest const Method Bug # 46 -//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@A.h #ifndef A_H_ #define A_H_ @@ -1652,7 +1605,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1669,7 +1621,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1689,7 +1640,6 @@ int A::foo() const { int A::help() { return 42; } - //= #include "A.h" @@ -1713,14 +1663,6 @@ int A::foo() const { int A::help() { return 42; } - -//@.config -filename=A.cpp -methodname=exp -replaceduplicates=false -returnvalue=false -returnparameterindex=0 - //!Don't return variables that aren't used //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config @@ -1767,7 +1709,7 @@ void function() { continue;/*$$*/ } } -//! Bug #124 Extract Function with a Macro call in selected code "forgets" the macro +//! Bug #124 Extract function with a Macro call in selected code "forgets" the macro //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=Test.cpp @@ -1802,7 +1744,7 @@ void testFuerRainer() { runTest(i); } -//! Bug #124 with comments Extract Function with a Macro call in selected code "forgets" the macro +//! Bug #124 with comments Extract function with a Macro call in selected code "forgets" the macro //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=Test.cpp @@ -1833,7 +1775,7 @@ void testFuerRainer() { runTest(i); } -//! Bug #137 String Array problem in Extract Function +//! Bug #137 String Array problem in Extract function //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=Test.cpp @@ -1873,9 +1815,6 @@ int main() { //!Bug 239059: Double & in signature of extracted functions //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest -//@.config -returnvalue=false -returnparameterindex=0 //@A.h #ifndef A_H_ #define A_H_ @@ -2016,7 +1955,6 @@ void Test::test() { filename=Test.cpp methodname=startTag //@testString.h - namespace test { class string { @@ -2031,7 +1969,6 @@ public: }; } - //@Test.cpp #include "testString.h" @@ -2044,11 +1981,10 @@ test::string toXML() { int main() { return 0; } - //= #include "testString.h" -test::string startTag(test::string& name) { +test::string startTag(test::string name) { return "<" + name + ">"; } @@ -2061,7 +1997,6 @@ test::string toXML() { int main() { return 0; } - //!Bug 248238: Extract Method Produces Wrong Return Type Or Just Fails Typedef //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config @@ -2080,13 +2015,11 @@ public: string2 operator+(const string2& rhs) { return rhs; } string2(char* cp) {} string2() {}; - }; typedef string2 string; } - //@Test.cpp #include "testString.h" @@ -2099,11 +2032,10 @@ test::string toXML() { int main() { return 0; } - //= #include "testString.h" -test::string startTag(test::string& name) { +test::string startTag(test::string name) { return "<" + name + ">"; } @@ -2116,7 +2048,6 @@ test::string toXML() { int main() { return 0; } - //!Bug 248622: Extract function fails to extract several expressions; Selection at the end //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config @@ -2219,7 +2150,7 @@ test::string toXML() { int main() { return 0; } -//!Bug#262000 refactoring "extract function" misinterprets artificial blocks +//!Bug#262000 Extract function misinterprets artificial blocks //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=main.cpp @@ -2349,7 +2280,7 @@ int Test::calculateStuff() { return result; } -//!Bug#281564 Extract Function fails when catching an unnamed exception +//!Bug#281564 Extract function fails when catching an unnamed exception //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=main.cpp @@ -2390,7 +2321,7 @@ int main() { return a; } -//!Bug#282004 Refactor->Extract Function in C Project (not C++) won't extract parameters +//!Bug#282004 Extract function in C Project (not C++) won't extract parameters //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config filename=main.c @@ -2401,9 +2332,8 @@ int main() { /*$*/b=a*2;/*$$*/ return a; } - //= -void exp(int b, int* a) { +void exp(int b, int a) { b = a * 2; } @@ -2412,9 +2342,14 @@ int main() { exp(b, a); return a; } - -//!ExtractFunctionRefactoringTest virtual +//!Extract function with virtual //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest +//@.config +filename=A.cpp +methodname=exp +replaceduplicates=false +virtual=true +visibility=public //@A.h #ifndef A_H_ #define A_H_ @@ -2430,7 +2365,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -2447,7 +2381,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -2468,7 +2401,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -2494,18 +2426,12 @@ int A::foo() { int A::help() { return 42; } - -//@.config -filename=A.cpp -methodname=exp -replaceduplicates=false -returnvalue=false -returnparameterindex=0 -virtual=true -visibility=public - //!Bug 288268: c-refactoring creates c++-parameters //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest +//@.config +filename=main.c +methodname=test +replaceduplicates=false //@main.c int main() { int a, b; @@ -2522,14 +2448,12 @@ int main() { test(a, b); return a; } -//@.config -filename=main.c -methodname=test -replaceduplicates=false -returnvalue=false - //!Handling of blank lines //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest +//@.config +filename=main.c +methodname=fib +replaceduplicates=false //@main.c int main() { int f; @@ -2561,7 +2485,3 @@ int main() { int b = fib(); f = b; } -//@.config -filename=main.c -methodname=fib -replaceduplicates=false diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodDuplicates.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodDuplicates.rts index 3967ebcda7a..078efe5bced 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodDuplicates.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodDuplicates.rts @@ -1,4 +1,4 @@ -//!ExtractFunctionRefactoringTest with duplicates +//!Extract method with duplicates //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true @@ -84,7 +84,7 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest duplicates with different Names +//!Extract method duplicates with different Names //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true @@ -170,11 +170,11 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest duplicate with field +//!Extract method duplicate with field //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=j //@A.h #ifndef A_H_ #define A_H_ @@ -267,11 +267,11 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest duplicate with field in marked scope +//!Extract method duplicate with field in marked scope //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=j //@A.h #ifndef A_H_ #define A_H_ @@ -366,11 +366,11 @@ void A::foo() { int A::help() { return 42; } -//!ExtractFunctionRefactoringTest duplicates with different Names and return type +//!Extract method duplicates with different names and return type //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -465,7 +465,7 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest duplicates with a lot of different Names an variable not used afterwards in the duplicate +//!Extract method duplicates with a lot of different Names an variable not used afterwards in the duplicate //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true @@ -558,7 +558,7 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest with duplicate name used afterwards in duplicate but not in original selection this is no duplicate +//!Extract method with duplicate name used afterwards in duplicate but not in original selection this is no duplicate //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true @@ -645,11 +645,11 @@ int A::help() { return 42; } -//!ExtractFunctionRefactoringTest with Return Value and a lot Ref Parameter and a method call +//!Extract method with return value and a lot ref parameter and a method call //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -667,7 +667,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -682,11 +681,10 @@ public: private: int help(); - int exp(int i, B*& b, int& y, float& x); + int exp(int i, B* b, int y, float x); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -724,7 +722,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -742,7 +739,7 @@ A::~A() { i++; } -int A::exp(int i, B*& b, int& y, float& x) { +int A::exp(int i, B* b, int y, float x) { ++i; b->hello(y); i = i + x; @@ -764,13 +761,11 @@ int A::foo() { int A::help() { return 42; } - //@B.h #ifndef B_H_ #define B_H_ -class B -{ +class B { public: B(); virtual ~B(); @@ -778,12 +773,11 @@ public: }; #endif /*B_H_*/ - -//!ExtractFunctionRefactoringTest with Return Value and a lot Ref Parameter and a method call, duplicate is not similar +//!Extract method with return value and a lot ref parameter and a method call, duplicate is similar //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true -returnvalue=true +returnvalue=i //@A.h #ifndef A_H_ #define A_H_ @@ -801,7 +795,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -816,11 +809,10 @@ public: private: int help(); - int exp(int i, B*& b, int& y, float x); + int exp(int i, B* b, int y, float x); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -857,7 +849,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -869,16 +860,13 @@ A::~A() { float x = i; B* b = new B(); int y = x + i; - ++i; - b->hello(y); - i = i + x; - help(); + i = exp(i, b, y, x); b->hello(y); ++x; i++; } -int A::exp(int i, B*& b, int& y, float x) { +int A::exp(int i, B* b, int y, float x) { ++i; b->hello(y); i = i + x; @@ -899,13 +887,11 @@ int A::foo() { int A::help() { return 42; } - //@B.h #ifndef B_H_ #define B_H_ -class B -{ +class B { public: B(); virtual ~B(); @@ -913,8 +899,7 @@ public: }; #endif /*B_H_*/ - -//!ExtractFunctionRefactoringTest with duplicates and comments +//!Extract method with duplicates and comments //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config replaceduplicates=true diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodPreprocessor.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodPreprocessor.rts index 23d300881a7..34f789a380e 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodPreprocessor.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethodPreprocessor.rts @@ -83,11 +83,10 @@ int A::help() { return 42; } -//!Extract Method return value assinged to Macrocall Bug +//!Extract method return value assigned to macro call //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config -returnvalue=true -returnparameterindex=1 +returnvalue=b //@A.h #ifndef A_H_ #define A_H_ @@ -103,7 +102,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -120,7 +118,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -145,7 +142,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -175,12 +171,10 @@ int A::foo() { int A::help() { return 42; } - -//!Extract Method Methodlength with more than 1 Macrocall Bug +//!Extract method with multiple macros //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest //@.config -returnvalue=true -returnparameterindex=0 +returnvalue=bb replaceduplicates=true //@A.h #ifndef A_H_ @@ -197,7 +191,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -214,7 +207,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -239,7 +231,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -269,4 +260,3 @@ int A::foo() { int A::help() { return 42; } - 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 737b58108fd..020a2bcc54d 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 @@ -8,6 +8,7 @@ * * Contributors: * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.ui.tests.refactoring.extractfunction; @@ -34,7 +35,7 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum; public class ExtractFunctionRefactoringTest extends RefactoringTest { protected String methodName; protected boolean replaceDuplicates; - protected boolean returnValue; + protected String returnValue; protected int returnParameterIndex; protected boolean fatalError; private VisibilityEnum visibility; @@ -74,29 +75,25 @@ public class ExtractFunctionRefactoringTest extends RefactoringTest { info.setMethodName(methodName); info.setReplaceDuplicates(replaceDuplicates); if (info.getMandatoryReturnVariable() == null) { - if (returnValue) { - info.setReturnVariable(info.getNamesUsedAfter().get(returnParameterIndex)); - info.getNamesUsedAfter().get(returnParameterIndex).setUserSetIsReference(false); + if (returnValue != null) { + for (NameInformation nameInfo : info.getParameterCandidates()) { + if (returnValue.equals(String.valueOf(nameInfo.getName().getSimpleID()))) { + info.setReturnVariable(nameInfo); + nameInfo.setUserSetIsReference(false); + break; + } + } } - } else { - info.setReturnVariable(info.getMandatoryReturnVariable()); } info.setVisibility(visibility); info.setVirtual(virtual); - - for (NameInformation name : info.getNamesUsedAfter()) { - if (!name.isUserSetIsReturnValue()) { - name.setUserSetIsReference(name.isOutput()); - } - } } @Override protected void configureRefactoring(Properties refactoringProperties) { methodName = refactoringProperties.getProperty("methodname", "exp"); //$NON-NLS-1$ //$NON-NLS-2$ replaceDuplicates = Boolean.valueOf(refactoringProperties.getProperty("replaceduplicates", "false")).booleanValue(); //$NON-NLS-1$ //$NON-NLS-2$ - returnValue = Boolean.valueOf(refactoringProperties.getProperty("returnvalue", "false")).booleanValue(); //$NON-NLS-1$//$NON-NLS-2$ - returnParameterIndex = new Integer(refactoringProperties.getProperty("returnparameterindex", "0")).intValue(); //$NON-NLS-1$ //$NON-NLS-2$ + returnValue = refactoringProperties.getProperty("returnvalue", null); //$NON-NLS-1$ fatalError = Boolean.valueOf(refactoringProperties.getProperty("fatalerror", "false")).booleanValue(); //$NON-NLS-1$ //$NON-NLS-2$ visibility = VisibilityEnum.getEnumForStringRepresentation(refactoringProperties.getProperty("visibility", VisibilityEnum.v_private.toString())); //$NON-NLS-1$ virtual = Boolean.valueOf(refactoringProperties.getProperty("virtual", "false")).booleanValue(); //$NON-NLS-1$ //$NON-NLS-2$ diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionTestSuite.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionTestSuite.java index 46d16bd0457..42d36bc0513 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionTestSuite.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionTestSuite.java @@ -24,12 +24,12 @@ public class ExtractFunctionTestSuite extends TestSuite { @SuppressWarnings("nls") public static Test suite() throws Exception { TestSuite suite = new ExtractFunctionTestSuite(); - suite.addTest(RefactoringTester.suite("ExtractMethodRefactoringTests", "resources/refactoring/ExtractMethod.rts")); - suite.addTest(RefactoringTester.suite("Extract Expression", "resources/refactoring/ExtractExpression.rts")); - suite.addTest(RefactoringTester.suite("ExtractMethodPreprocessorRefactoringTests", "resources/refactoring/ExtractMethodPreprocessor.rts")); - suite.addTest(RefactoringTester.suite("Extract Function Templates Tests", "resources/refactoring/ExtractFunctionTemplates.rts")); - suite.addTest(RefactoringTester.suite("Extract Method History Test", "resources/refactoring/ExtractMethodHistory.rts")); - suite.addTest(RefactoringTester.suite("Extract Function Duplicates Test", "resources/refactoring/ExtractMethodDuplicates.rts")); + suite.addTest(RefactoringTester.suite("ExtractMethod.rts", "resources/refactoring/ExtractMethod.rts")); + suite.addTest(RefactoringTester.suite("ExtractExpression.rts", "resources/refactoring/ExtractExpression.rts")); + suite.addTest(RefactoringTester.suite("ExtractMethodPreprocessor.rts", "resources/refactoring/ExtractMethodPreprocessor.rts")); + suite.addTest(RefactoringTester.suite("ExtractFunctionTemplates.rts", "resources/refactoring/ExtractFunctionTemplates.rts")); + suite.addTest(RefactoringTester.suite("ExtractMethodHistory.rts", "resources/refactoring/ExtractMethodHistory.rts")); + suite.addTest(RefactoringTester.suite("ExtractFunctionDuplicates.rts", "resources/refactoring/ExtractMethodDuplicates.rts")); return suite; } } 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 419307f18b0..feff003ae47 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 @@ -8,10 +8,12 @@ * * Contributors: * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -47,13 +49,16 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateTypeParameter; import org.eclipse.cdt.ui.CUIPlugin; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; +import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVariableReadWriteFlags; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriter; +import org.eclipse.cdt.internal.core.pdom.dom.PDOMName; public class NodeContainer { public final NameInformation NULL_NAME_INFORMATION = new NameInformation(new CPPASTName()); private final List nodes; private List names; + private List interfaceNames; public class NameInformation { private IASTName name; @@ -214,8 +219,8 @@ public class NodeContainer { return isOutput; } - public void setOutput(boolean output) { - this.isOutput = output; + public void setOutput(boolean isOutput) { + this.isOutput = isOutput; } public boolean isReturnValue() { @@ -334,96 +339,85 @@ public class NodeContainer { IASTName name = nameInfo.getName(); IASTTranslationUnit unit = name.getTranslationUnit(); - IASTName[] decls = unit.getDeclarationsInAST(name.resolveBinding()); - for (IASTName declaration : decls) { - nameInfo.setDeclaration(declaration); - } - if (nameInfo.isReferencedAfterSelection()) { - nameInfo.setOutput(true); + IASTName[] nameDeclarations = unit.getDeclarationsInAST(name.resolveBinding()); + if (nameDeclarations.length != 0) { + nameInfo.setDeclaration(nameDeclarations[nameDeclarations.length - 1]); } } } - /* - * Returns all local names in the selection which are referenced after the selection. + /** + * Returns names that are either parameter or return value candidates. */ - public List getNamesUsedAfter() { - findAllNames(); - - Set declarations = new HashSet(); - List usedAfter = new ArrayList(); - - for (NameInformation nameInfo : names) { - if (declarations.add(nameInfo.getDeclaration())) { - if (nameInfo.isReferencedAfterSelection()) { - usedAfter.add(nameInfo); - } - } - } - - return usedAfter; - } - - public List getNamesUsedAfterChoosenByUser() { - findAllNames(); - - Set declarations = new HashSet(); - List usedAfter = new ArrayList(); - - for (NameInformation nameInfo : names) { - if (declarations.add(nameInfo.getDeclaration())) { - if (nameInfo.isUserSetIsReference() || nameInfo.isUserSetIsReturnValue()) { - usedAfter.add(nameInfo); - } - } - } - - return usedAfter; - } - - public List getUsedNamesUnique() { - findAllNames(); - - Set declarations = new HashSet(); - List usedAfter = new ArrayList(); - - for (NameInformation nameInfo : names) { - if (declarations.add(nameInfo.getDeclaration())) { - usedAfter.add(nameInfo); - } else { - for (NameInformation nameInformation : usedAfter) { - if (nameInfo.isWriteAccess() && - nameInfo.getDeclaration() == nameInformation.getDeclaration()) { - nameInformation.setWriteAccess(true); + private List getInterfaceNames() { + if (interfaceNames == null) { + findAllNames(); + + Set declarations = new HashSet(); + interfaceNames = new ArrayList(); + + for (NameInformation nameInfo : names) { + if (declarations.add(nameInfo.getDeclaration())) { + if (nameInfo.isDeclaredInSelection()) { + if (nameInfo.isReferencedAfterSelection()) { + nameInfo.setReturnValue(true); + interfaceNames.add(nameInfo); + } + } else { + for (NameInformation n2 : names) { + if (n2.getDeclaration() == nameInfo.getDeclaration()) { + int flag = CPPVariableReadWriteFlags.getReadWriteFlags(n2.getName()); + if ((flag & PDOMName.WRITE_ACCESS) != 0) { + nameInfo.setWriteAccess(true); + break; + } + } + } + if (nameInfo.isWriteAccess() && nameInfo.isReferencedAfterSelection()) { + nameInfo.setOutput(true); + } + interfaceNames.add(nameInfo); } } } } - return usedAfter; + return interfaceNames; + } + + private List getInterfaceNames(boolean isReturnValue) { + List selectedNames = null; + + for (NameInformation nameInfo : getInterfaceNames()) { + if (nameInfo.isReturnValue() == isReturnValue) { + if (selectedNames == null) { + selectedNames = new ArrayList(); + } + selectedNames.add(nameInfo); + } + } + if (selectedNames == null) { + selectedNames = Collections.emptyList(); + } + return selectedNames; } /** - * Returns all variables declared in the selection, which will are referenced after - * the selection. + * Returns names that are candidates to be used as function parameters. + */ + public List getParameterCandidates() { + return getInterfaceNames(false); + } + + + /** + * Returns names that are candidates for being used as the function return value. Multiple + * return value candidates mean that the function cannot be extracted. */ public List getReturnValueCandidates() { - Set declarations = new HashSet(); - List usedAfter = new ArrayList(); - - for (NameInformation nameInfo : names) { - if (nameInfo.isDeclaredInSelection() && nameInfo.isReferencedAfterSelection() && - declarations.add(nameInfo.getDeclaration())) { - usedAfter.add(nameInfo); - // It's a return value candidate, set return value to true and reference to false - nameInfo.setReturnValue(true); - nameInfo.setOutput(false); - } - } - - return usedAfter; + return getInterfaceNames(true); } - + public List getNodesToWrite() { return nodes; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java index 19b80179127..81cfb2db0d3 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ChooserComposite.java @@ -50,11 +50,6 @@ public class ChooserComposite extends Composite { GridLayout layout = new GridLayout(); setLayout(layout); - boolean hasNoPredefinedReturnValue = true; - if (info.getMandatoryReturnVariable() != null) { - hasNoPredefinedReturnValue = false; - } - final ArrayList