Add support for extracting method to outer class (#1479)

- add new findFieldReferencesForType() method to ExtractMethodAnalyzer to find field references in the inner type where selection is made - add new logic to ExtractMethodRefactoring to recognize a move to an outer class where fields are referenced in inner in which case pass an extra parameter of the inner type and qualify any field references using this extra parameter - add new test to ExtractMethodTests - fixes #1458
eclipse-jdt · Jun 27, 2024 · 7e703b9 · 7e703b9
1 parent 7c250b5
commit 7e703b9
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 0 deletions.
diff --git a/...n/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java b/...n/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java
@@ -36,6 +36,7 @@
 import org.eclipse.jdt.core.dom.AST;
 import org.eclipse.jdt.core.dom.ASTNode;
 import org.eclipse.jdt.core.dom.ASTVisitor;
+import org.eclipse.jdt.core.dom.AbstractTypeDeclaration;
 import org.eclipse.jdt.core.dom.Annotation;
 import org.eclipse.jdt.core.dom.AnnotationTypeDeclaration;
 import org.eclipse.jdt.core.dom.AnonymousClassDeclaration;
@@ -454,6 +455,29 @@ public boolean visit(Assignment node) {
 		return problems[0];
 	}
 
+	public List<SimpleName> findFieldReferencesForType(AbstractTypeDeclaration type) {
+		final List<SimpleName> fieldList= new ArrayList<>();
+		ASTNode[] selectedNodes= getSelectedNodes();
+		ITypeBinding typeBinding= type.resolveBinding();
+		if (typeBinding != null) {
+			for (ASTNode astNode : selectedNodes) {
+				astNode.accept(new ASTVisitor() {
+					@Override
+					public boolean visit(SimpleName node) {
+						IBinding binding= node.resolveBinding();
+						if (binding instanceof IVariableBinding varBinding && varBinding.isField()) {
+							if (typeBinding.isEqualTo(varBinding.getDeclaringClass())) {
+								fieldList.add(node);
+							}
+						}
+						return true;
+					}
+				});
+			}
+		}
+		return fieldList;
+	}
+
 	private String canHandleBranches() {
 		if (fReturnValue != null)
 			return RefactoringCoreMessages.ExtractMethodAnalyzer_branch_mismatch;

diff --git a/...efactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java b/...efactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java
@@ -180,6 +180,8 @@ public class ExtractMethodRefactoring extends Refactoring {
 	private LinkedProposalModelCore fLinkedProposalModel;
 	private Map<String,String> fFormatterOptions;
 	private boolean fHasYield;
+	private List<SimpleName> fFieldAccesses= new ArrayList<>();
+	private String fPassedTypeName;
 
 	private static final String EMPTY= ""; //$NON-NLS-1$
 
@@ -1048,6 +1050,9 @@ private ASTNode[] createCallNodes(SnippetFinder.Match duplicate, int modifiers)
 		}
 
 		List<Expression> arguments= invocation.arguments();
+		if (!fFieldAccesses.isEmpty()) {
+			arguments.add(fAST.newThisExpression());
+		}
 		for (ParameterInfo parameter : fParameterInfos) {
 			arguments.add(ASTNodeFactory.newName(fAST, getMappedName(duplicate, parameter)));
 		}
@@ -1221,7 +1226,22 @@ private MethodDeclaration createNewMethodDeclaration() {
 
 		ImportRewriteContext context= new ContextSensitiveImportRewriteContext(enclosingBodyDeclaration, fImportRewriter);
 
+		ASTNode enclosingType= ASTNodes.getFirstAncestorOrNull(enclosingBodyDeclaration, AbstractTypeDeclaration.class, AnonymousClassDeclaration.class);
+		if (enclosingType != fDestination && enclosingType instanceof AbstractTypeDeclaration typeDeclaration) {
+			fFieldAccesses= fAnalyzer.findFieldReferencesForType(typeDeclaration);
+		}
+
 		List<SingleVariableDeclaration> parameters= result.parameters();
+		if (!fFieldAccesses.isEmpty()) {
+			SingleVariableDeclaration parameter= fAST.newSingleVariableDeclaration();
+			ITypeBinding typeBinding= ((AbstractTypeDeclaration)enclosingType).resolveBinding();
+			parameter.setType(fImportRewriter.addImport(typeBinding, fAST));
+			fImportRewriter.removeImport(typeBinding.getQualifiedName());
+			fPassedTypeName= "passed" + ((AbstractTypeDeclaration)enclosingType).getName().getFullyQualifiedName(); //$NON-NLS-1$
+			parameter.setName(fAST.newSimpleName(fPassedTypeName));
+			parameters.add(parameter);
+		}
+
 		for (ParameterInfo info : fParameterInfos) {
 			VariableDeclaration infoDecl= getVariableDeclaration(info);
 			SingleVariableDeclaration parameter= fAST.newSingleVariableDeclaration();
@@ -1310,6 +1330,7 @@ private Block createMethodBody(ASTNode[] selectedNodes, TextEditGroup substitute
 				}
 			}
 		}
+
 		for (ParameterInfo parameter : fParameterInfos) {
 			if (parameter.isRenamed()) {
 				for (ASTNode selectedNode : selectedNodes) {
@@ -1320,6 +1341,14 @@ private Block createMethodBody(ASTNode[] selectedNodes, TextEditGroup substitute
 			}
 		}
 
+		// Replace all old field accesses with new ones referencing passed in type variable
+		for (SimpleName fieldAccess : fFieldAccesses) {
+			FieldAccess newFieldAccess= fAST.newFieldAccess();
+			newFieldAccess.setExpression(fAST.newSimpleName(fPassedTypeName));
+			newFieldAccess.setName((SimpleName)fRewriter.createCopyTarget(fieldAccess));
+			fRewriter.replace(fieldAccess, newFieldAccess, null);
+		}
+
 		boolean extractsExpression= fAnalyzer.isExpressionSelected();
 		ASTNode[] callNodes= createCallNodes(null, modifiers);
 		ASTNode replacementNode;

diff --git a/....refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/nested_in/A_test655.java b/....refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/nested_in/A_test655.java
@@ -0,0 +1,11 @@
+package nested_in;
+
+public class A_test655 {
+	public static class NestedStatic {
+		private int i= 0;
+
+		public void foo() {
+			/*[*/System.out.println("Greetings" + ++i);/*]*/
+		}
+	}
+}
diff --git a/...refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/nested_out/A_test655.java b/...refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/nested_out/A_test655.java
@@ -0,0 +1,15 @@
+package nested_in;
+
+public class A_test655 {
+	public static class NestedStatic {
+		private int i= 0;
+
+		public void foo() {
+			extracted(this);
+		}
+	}
+
+	protected static void extracted(NestedStatic passedNestedStatic) {
+		/*[*/System.out.println("Greetings" + ++passedNestedStatic.i);/*]*/
+	}
+}
diff --git a/...tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java b/...tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java
@@ -181,6 +181,11 @@ protected void nestedTest() throws Exception {
 		performTest(fgTestSetup.getNestedPackage(), "A", COMPARE_WITH_OUTPUT, "nested_out");
 	}
 
+	protected void nestedTest1() throws Exception {
+		performTest(fgTestSetup.getNestedPackage(), "A", COMPARE_WITH_OUTPUT, "nested_out", null, null, 1);
+
+	}
+
 	protected void returnTest() throws Exception {
 		performTest(fgTestSetup.getReturnPackage(), "A", COMPARE_WITH_OUTPUT, "return_out");
 	}
@@ -1839,6 +1844,11 @@ public void test654() throws Exception {
 		nestedTest();
 	}
 
+	@Test
+	public void test655() throws Exception {
+		nestedTest1();
+	}
+
 	//---- Extracting method containing a return statement.
 
 	@Test