Attachment #143217 for bug #64245

View | Details | Raw Unified | Return to bug 64245
Collapse All | Expand All




 * Contributors:
 *     IBM Corporation - initial API and implementation
 *     Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Does not replace similar code in parent class of anonymous class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=160853
 *     Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] creates un-needed code - https://bugs.eclipse.org/bugs/show_bug.cgi?id=64245
 *******************************************************************************/
package org.eclipse.jdt.internal.corext.refactoring.code;


		Block result= fAST.newBlock();
		ListRewrite statements= fRewriter.getListRewrite(result, Block.STATEMENTS_PROPERTY);

		// Locals that are not passed as an arguments since the extracted method only
		// writes to them
		IVariableBinding[] methodLocals= fAnalyzer.getMethodLocals();
		for (int i= 0; i < methodLocals.length; i++) {
			if (methodLocals[i] != null) {
				result.statements().add(createDeclaration(methodLocals[i], null));
			}
		}

		for (Iterator iter= fParameterInfos.iterator(); iter.hasNext();) {
			ParameterInfo parameter= (ParameterInfo)iter.next();
			if (parameter.isRenamed()) {

		} else {
			replacementNode= fRewriter.createGroupNode(callNodes);
		}
		IVariableBinding skipMethodLocal= null;
		final ASTNode selectedNode= selectedNodes[0];
		IVariableBinding returnValue= fAnalyzer.getReturnValue();
		if (extractsExpression) {
			// if we have an expression then only one node is selected.
			ITypeBinding binding= fAnalyzer.getExpressionBinding();
			if (binding != null && (!binding.isPrimitive() || !"void".equals(binding.getName()))) { //$NON-NLS-1$
				ReturnStatement rs= fAST.newReturnStatement();
				rs.setExpression((Expression)fRewriter.createMoveTarget(selectedNode));
				statements.insertLast(rs, null);
			} else {
				ExpressionStatement st= fAST.newExpressionStatement((Expression)fRewriter.createMoveTarget(selectedNode));
				statements.insertLast(st, null);
			}
			fRewriter.replace(selectedNode, replacementNode, substitute);
		} else {
			Expression returnExpression= null;
			if (selectedNodes.length == 1) {
				boolean isExpressionStatement= selectedNode.getNodeType() == ASTNode.EXPRESSION_STATEMENT;
				if (isExpressionStatement
						&& returnValue != null
						&& ((ExpressionStatement)selectedNode).getExpression().getNodeType() == ASTNode.ASSIGNMENT
						&& Arrays.asList(fAnalyzer.getMethodLocals()).contains(Bindings.getAssignedVariable((Assignment)((ExpressionStatement)selectedNode).getExpression()))) {
					Assignment assignment= (Assignment)((ExpressionStatement)selectedNode).getExpression();
					returnExpression= (Expression)fRewriter.createMoveTarget(assignment.getRightHandSide());
					skipMethodLocal= Bindings.getAssignedVariable(assignment);
				} else {
					statements.insertLast(fRewriter.createMoveTarget(selectedNode), substitute);
				}
				fRewriter.replace(selectedNode, replacementNode, substitute);
			} else {
				ListRewrite source= fRewriter.getListRewrite(
						selectedNode.getParent(),
						(ChildListPropertyDescriptor)selectedNode.getLocationInParent());
				ASTNode toMove= source.createMoveTarget(
						selectedNode, selectedNodes[selectedNodes.length - 1],
						replacementNode, substitute);
				statements.insertLast(toMove, substitute);
			}
			IVariableBinding returnValue= fAnalyzer.getReturnValue();
			if (returnValue != null) {
				ReturnStatement rs= fAST.newReturnStatement();
				if (returnExpression == null) {
					returnExpression= fAST.newSimpleName(getName(returnValue));
				}
				rs.setExpression(returnExpression);
				statements.insertLast(rs, null);
			}
		}
		// Locals that are not passed as an arguments since the extracted method only
		// writes to them
		IVariableBinding[] methodLocals= fAnalyzer.getMethodLocals();
		for (int i= methodLocals.length-1; i >= 0; i--) {
			boolean skip= false;
			if(skipMethodLocal != null) {
				skip= methodLocals[i].isEqualTo(skipMethodLocal);
			}
			if (methodLocals[i] != null && !skip) {
				statements.insertFirst(createDeclaration(methodLocals[i], null), substitute);
			}
		}
		return result;
	}


Lines 10-15 Link Here

(-)test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java (+5 lines)
10	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Does not replace similar code in parent class of anonymous class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=160853	10	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Does not replace similar code in parent class of anonymous class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=160853
11	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Missing return value, while extracting code out of a loop - https://bugs.eclipse.org/bugs/show_bug.cgi?id=213519	11	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Missing return value, while extracting code out of a loop - https://bugs.eclipse.org/bugs/show_bug.cgi?id=213519
12	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] missing return type when code can throw exception - https://bugs.eclipse.org/bugs/show_bug.cgi?id=97413	12	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] missing return type when code can throw exception - https://bugs.eclipse.org/bugs/show_bug.cgi?id=97413
		13	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] creates un-needed code - https://bugs.eclipse.org/bugs/show_bug.cgi?id=64245
13	*******************************************************************************/	14	*******************************************************************************/
14	package org.eclipse.jdt.ui.tests.refactoring;	15	package org.eclipse.jdt.ui.tests.refactoring;
15		16
Lines 1419-1424 Link Here
1419	public void test622() throws Exception {	1420	public void test622() throws Exception {
1420	expressionTest();	1421	expressionTest();
1421	}	1422	}
		1423
		1424	public void test623() throws Exception {
		1425	expressionTest();
		1426	}
1422		1427
1423	//---- Test nested methods and constructor	1428	//---- Test nested methods and constructor
1424		1429




package expression_in;

public class A_test622 {
	private void f() {
		Object o= null;
		String s= null;
		s = extracted(o);
		System.out.println(s);
	}

	protected String extracted(Object o) {
		return (String)o;
	}
}




package expression_in;

public class A_test622 {
	private void f() {
		Object o= null;
		String s= null;
		/*[*/
		s= (String)o;
		/*]*/
		System.out.println(s);
	}
}

Return to bug 64245

Lines 8-13 Link Here

(-)core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java (-20 / +40 lines)
8	* Contributors:	8	* Contributors:
9	* IBM Corporation - initial API and implementation	9	* IBM Corporation - initial API and implementation
10	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Does not replace similar code in parent class of anonymous class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=160853	10	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Does not replace similar code in parent class of anonymous class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=160853
		11	* Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] creates un-needed code - https://bugs.eclipse.org/bugs/show_bug.cgi?id=64245
11	*******************************************************************************/	12	*******************************************************************************/
12	package org.eclipse.jdt.internal.corext.refactoring.code;	13	package org.eclipse.jdt.internal.corext.refactoring.code;
13		14
Lines 906-920 Link Here
906	Block result= fAST.newBlock();	907	Block result= fAST.newBlock();
907	ListRewrite statements= fRewriter.getListRewrite(result, Block.STATEMENTS_PROPERTY);	908	ListRewrite statements= fRewriter.getListRewrite(result, Block.STATEMENTS_PROPERTY);
908		909
909	// Locals that are not passed as an arguments since the extracted method only
910	// writes to them
911	IVariableBinding[] methodLocals= fAnalyzer.getMethodLocals();
912	for (int i= 0; i < methodLocals.length; i++) {
913	if (methodLocals[i] != null) {
914	result.statements().add(createDeclaration(methodLocals[i], null));
915	}
916	}
917
918	for (Iterator iter= fParameterInfos.iterator(); iter.hasNext();) {	910	for (Iterator iter= fParameterInfos.iterator(); iter.hasNext();) {
919	ParameterInfo parameter= (ParameterInfo)iter.next();	911	ParameterInfo parameter= (ParameterInfo)iter.next();
920	if (parameter.isRenamed()) {	912	if (parameter.isRenamed()) {
Lines 935-972 Link Here
935	} else {	927	} else {
936	replacementNode= fRewriter.createGroupNode(callNodes);	928	replacementNode= fRewriter.createGroupNode(callNodes);
937	}	929	}
		930	IVariableBinding skipMethodLocal= null;
		931	final ASTNode selectedNode= selectedNodes[0];
		932	IVariableBinding returnValue= fAnalyzer.getReturnValue();
938	if (extractsExpression) {	933	if (extractsExpression) {
939	// if we have an expression then only one node is selected.	934	// if we have an expression then only one node is selected.
940	ITypeBinding binding= fAnalyzer.getExpressionBinding();	935	ITypeBinding binding= fAnalyzer.getExpressionBinding();
941	if (binding != null && (!binding.isPrimitive() \|\| !"void".equals(binding.getName()))) { //$NON-NLS-1$	936	if (binding != null && (!binding.isPrimitive() \|\| !"void".equals(binding.getName()))) { //$NON-NLS-1$
942	ReturnStatement rs= fAST.newReturnStatement();	937	ReturnStatement rs= fAST.newReturnStatement();
943	rs.setExpression((Expression)fRewriter.createMoveTarget(selectedNodes[0]));	938	rs.setExpression((Expression)fRewriter.createMoveTarget(selectedNode));
944	statements.insertLast(rs, null);	939	statements.insertLast(rs, null);
945	} else {	940	} else {
946	ExpressionStatement st= fAST.newExpressionStatement((Expression)fRewriter.createMoveTarget(selectedNodes[0]));	941	ExpressionStatement st= fAST.newExpressionStatement((Expression)fRewriter.createMoveTarget(selectedNode));
947	statements.insertLast(st, null);	942	statements.insertLast(st, null);
948	}	943	}
949	fRewriter.replace(selectedNodes[0], replacementNode, substitute);	944	fRewriter.replace(selectedNode, replacementNode, substitute);
950	} else {	945	} else {
		946	Expression returnExpression= null;
951	if (selectedNodes.length == 1) {	947	if (selectedNodes.length == 1) {
952	statements.insertLast(fRewriter.createMoveTarget(selectedNodes[0]), substitute);	948	boolean isExpressionStatement= selectedNode.getNodeType() == ASTNode.EXPRESSION_STATEMENT;
953	fRewriter.replace(selectedNodes[0], replacementNode, substitute);	949	if (isExpressionStatement
		950	&& returnValue != null
		951	&& ((ExpressionStatement)selectedNode).getExpression().getNodeType() == ASTNode.ASSIGNMENT
		952	&& Arrays.asList(fAnalyzer.getMethodLocals()).contains(Bindings.getAssignedVariable((Assignment)((ExpressionStatement)selectedNode).getExpression()))) {
		953	Assignment assignment= (Assignment)((ExpressionStatement)selectedNode).getExpression();
		954	returnExpression= (Expression)fRewriter.createMoveTarget(assignment.getRightHandSide());
		955	skipMethodLocal= Bindings.getAssignedVariable(assignment);
		956	} else {
		957	statements.insertLast(fRewriter.createMoveTarget(selectedNode), substitute);
		958	}
		959	fRewriter.replace(selectedNode, replacementNode, substitute);
954	} else {	960	} else {
955	ListRewrite source= fRewriter.getListRewrite(	961	ListRewrite source= fRewriter.getListRewrite(
956	selectedNodes[0].getParent(),	962	selectedNode.getParent(),
957	(ChildListPropertyDescriptor)selectedNodes[0].getLocationInParent());	963	(ChildListPropertyDescriptor)selectedNode.getLocationInParent());
958	ASTNode toMove= source.createMoveTarget(	964	ASTNode toMove= source.createMoveTarget(
959	selectedNodes[0], selectedNodes[selectedNodes.length - 1],	965	selectedNode, selectedNodes[selectedNodes.length - 1],
960	replacementNode, substitute);	966	replacementNode, substitute);
961	statements.insertLast(toMove, substitute);	967	statements.insertLast(toMove, substitute);
962	}	968	}
963	IVariableBinding returnValue= fAnalyzer.getReturnValue();
964	if (returnValue != null) {	969	if (returnValue != null) {
965	ReturnStatement rs= fAST.newReturnStatement();	970	ReturnStatement rs= fAST.newReturnStatement();
966	rs.setExpression(fAST.newSimpleName(getName(returnValue)));	971	if (returnExpression == null) {
		972	returnExpression= fAST.newSimpleName(getName(returnValue));
		973	}
		974	rs.setExpression(returnExpression);
967	statements.insertLast(rs, null);	975	statements.insertLast(rs, null);
968	}	976	}
969	}	977	}
		978	// Locals that are not passed as an arguments since the extracted method only
		979	// writes to them
		980	IVariableBinding[] methodLocals= fAnalyzer.getMethodLocals();
		981	for (int i= methodLocals.length-1; i >= 0; i--) {
		982	boolean skip= false;
		983	if(skipMethodLocal != null) {
		984	skip= methodLocals[i].isEqualTo(skipMethodLocal);
		985	}
		986	if (methodLocals[i] != null && !skip) {
		987	statements.insertFirst(createDeclaration(methodLocals[i], null), substitute);
		988	}
		989	}
970	return result;	990	return result;
971	}	991	}
972		992

Added Link Here

(-)resources/ExtractMethodWorkSpace/ExtractMethodTests/expression_out/A_test623.java (+14 lines)
1	package expression_in;
2
3	public class A_test622 {
4	private void f() {
5	Object o= null;
6	String s= null;
7	s = extracted(o);
8	System.out.println(s);
9	}
10
11	protected String extracted(Object o) {
12	return (String)o;
13	}
14	}