Fix push down to replace super references where appropriate (#1694)

* Fix push down to replace super references where appropriate - modify PushDownRefactoringProcessor to replace any super method invocations and super field accesses to the members being pushed down if they are in a destination of the moved members - add new test to PushDownTests - fixes #1692
eclipse-jdt · Oct 4, 2024 · b46e6a2 · b46e6a2
1 parent 5aad011
commit b46e6a2
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 3 deletions.
diff --git a/...g/org/eclipse/jdt/internal/corext/refactoring/structure/PushDownRefactoringProcessor.java b/...g/org/eclipse/jdt/internal/corext/refactoring/structure/PushDownRefactoringProcessor.java
@@ -32,6 +32,7 @@
 
 import org.eclipse.text.edits.MalformedTreeException;
 import org.eclipse.text.edits.TextEdit;
+import org.eclipse.text.edits.TextEditGroup;
 
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.Document;
@@ -863,15 +864,19 @@ private void copyMembers(Collection<MemberVisibilityAdjustor> adjustors, Map<IMe
 							final VariableDeclarationFragment oldField= ASTNodeSearchUtil.getFieldDeclarationFragmentNode((IField) infos[offset].getMember(), sourceRewriter.getRoot());
 							if (oldField != null) {
 								FieldDeclaration newField= createNewFieldDeclarationNode(infos[offset], sourceRewriter.getRoot(), mapping, unitRewriter.getASTRewrite(), oldField);
-								unitRewriter.getASTRewrite().getListRewrite(declaration, declaration.getBodyDeclarationsProperty()).insertAt(newField, BodyDeclarationRewrite.getInsertionIndex(newField, declaration.bodyDeclarations()), unitRewriter.createCategorizedGroupDescription(RefactoringCoreMessages.HierarchyRefactoring_add_member, SET_PUSH_DOWN));
+								TextEditGroup group= unitRewriter.createCategorizedGroupDescription(RefactoringCoreMessages.HierarchyRefactoring_add_member, SET_PUSH_DOWN);
+								unitRewriter.getASTRewrite().getListRewrite(declaration, declaration.getBodyDeclarationsProperty()).insertAt(newField, BodyDeclarationRewrite.getInsertionIndex(newField, declaration.bodyDeclarations()), group);
 								ImportRewriteUtil.addImports(unitRewriter, context, oldField.getParent(), new HashMap<>(), new HashMap<>(), false);
+								replaceTargetSuperFieldReferences(declaration, member, unitRewriter.getASTRewrite(), group);
 							}
 						} else {
 							final MethodDeclaration oldMethod= ASTNodeSearchUtil.getMethodDeclarationNode((IMethod) infos[offset].getMember(), sourceRewriter.getRoot());
 							if (oldMethod != null) {
 								MethodDeclaration newMethod= createNewMethodDeclarationNode(infos[offset], mapping, unitRewriter, oldMethod);
-								unitRewriter.getASTRewrite().getListRewrite(declaration, declaration.getBodyDeclarationsProperty()).insertAt(newMethod, BodyDeclarationRewrite.getInsertionIndex(newMethod, declaration.bodyDeclarations()), unitRewriter.createCategorizedGroupDescription(RefactoringCoreMessages.HierarchyRefactoring_add_member, SET_PUSH_DOWN));
+								TextEditGroup group= unitRewriter.createCategorizedGroupDescription(RefactoringCoreMessages.HierarchyRefactoring_add_member, SET_PUSH_DOWN);
+								unitRewriter.getASTRewrite().getListRewrite(declaration, declaration.getBodyDeclarationsProperty()).insertAt(newMethod, BodyDeclarationRewrite.getInsertionIndex(newMethod, declaration.bodyDeclarations()), group);
 								ImportRewriteUtil.addImports(unitRewriter, context, oldMethod, new HashMap<>(), new HashMap<>(), false);
+								replaceTargetSuperMethodInvocations(declaration, member, unitRewriter.getASTRewrite(), group);
 							}
 						}
 					}
@@ -882,6 +887,59 @@ private void copyMembers(Collection<MemberVisibilityAdjustor> adjustors, Map<IMe
 		}
 	}
 
+	private void replaceTargetSuperMethodInvocations(AbstractTypeDeclaration declaration, IMember member, ASTRewrite astRewrite, TextEditGroup group) {
+		ASTVisitor removeSuperMethodInvocationVisitor= new ASTVisitor() {
+			@Override
+			public boolean visit(SuperMethodInvocation node) {
+				IMethodBinding binding= node.resolveMethodBinding();
+				if (binding != null) {
+					if (binding.getJavaElement() != null && binding.getJavaElement().equals(member)) {
+						AST ast= astRewrite.getAST();
+						MethodInvocation newMethodInvocation= ast.newMethodInvocation();
+						newMethodInvocation.setName(ast.newSimpleName(node.getName().getFullyQualifiedName()));
+						ListRewrite typeArgs= astRewrite.getListRewrite(node, SuperMethodInvocation.TYPE_ARGUMENTS_PROPERTY);
+						List<Type> originalTypeList= typeArgs.getOriginalList();
+						if (originalTypeList.size() > 0) {
+							ASTNode typeArgsCopy= typeArgs.createCopyTarget(originalTypeList.get(0), originalTypeList.get(originalTypeList.size() - 1));
+							newMethodInvocation.typeArguments().add(typeArgsCopy);
+						}
+						ListRewrite args= astRewrite.getListRewrite(node, SuperMethodInvocation.ARGUMENTS_PROPERTY);
+						List<Type> originalArgsList= args.getOriginalList();
+						if (originalArgsList.size() > 0) {
+							ASTNode argsCopy= typeArgs.createCopyTarget(originalArgsList.get(0), originalArgsList.get(originalTypeList.size() - 1));
+							newMethodInvocation.arguments().add(argsCopy);
+						}
+						astRewrite.replace(node, newMethodInvocation, group);
+						return false;
+					}
+				}
+				return true;
+			}
+		};
+		declaration.accept(removeSuperMethodInvocationVisitor);
+	}
+
+	private void replaceTargetSuperFieldReferences(AbstractTypeDeclaration declaration, IMember member, ASTRewrite astRewrite, TextEditGroup group) {
+		ASTVisitor removeSuperFieldVisitor= new ASTVisitor() {
+			@Override
+			public boolean visit(SuperFieldAccess node) {
+				IVariableBinding binding= node.resolveFieldBinding();
+				if (binding != null) {
+					if (binding.isField() && binding.getJavaElement() != null && binding.getJavaElement().equals(member)) {
+						AST ast= astRewrite.getAST();
+						FieldAccess newFieldAccess= ast.newFieldAccess();
+						newFieldAccess.setName(ast.newSimpleName(node.getName().getFullyQualifiedName()));
+						newFieldAccess.setExpression(ast.newThisExpression());
+						astRewrite.replace(node, newFieldAccess, group);
+						return false;
+					}
+				}
+				return true;
+			}
+		};
+		declaration.accept(removeSuperFieldVisitor);
+	}
+
 	@Override
 	public Change createChange(IProgressMonitor pm) throws CoreException, OperationCanceledException {
 		try {

diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/PushDown/test41/in/A.java b/org.eclipse.jdt.ui.tests.refactoring/resources/PushDown/test41/in/A.java
@@ -0,0 +1,17 @@
+package p;
+
+class A {
+	int f;
+
+	void m() {
+	}
+
+	void mA() {
+		class B extends A {
+			public void mB() {
+				super.m();
+				super.f = 0;
+			}
+		}
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/PushDown/test41/out/A.java b/org.eclipse.jdt.ui.tests.refactoring/resources/PushDown/test41/out/A.java
@@ -0,0 +1,17 @@
+package p;
+
+class A {
+	void mA() {
+		class B extends A {
+			int f;
+
+			public void mB() {
+				m();
+				this.f = 0;
+			}
+
+			void m() {
+			}
+		}
+	}
+}
diff --git a/...t.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/PushDownTests.java b/...t.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/PushDownTests.java
@@ -964,7 +964,7 @@ public void test39() throws Exception { //https://github.com/eclipse-jdt/eclipse
 	}
 
 	@Test
-	public void test40() throws Exception { //https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1679
+	public void test40() throws Exception { //https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1691
 		String[] selectedMethodNames= { };
 		String[][] selectedMethodSignatures= { new String[0] };
 		String[] selectedFieldNames= { "f" };
@@ -981,6 +981,24 @@ public void test40() throws Exception { //https://github.com/eclipse-jdt/eclipse
 				namesOfMethodsToDeclareAbstract, signaturesOfMethodsToDeclareAbstract, null, null);
 	}
 
+	@Test
+	public void test41() throws Exception { //https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1692
+		String[] selectedMethodNames= { "m" };
+		String[][] selectedMethodSignatures= { new String[0] };
+		String[] selectedFieldNames= { "f" };
+		String[] namesOfMethodsToPushDown= selectedMethodNames;
+		String[][] signaturesOfMethodsToPushDown= selectedMethodSignatures;
+		String[] namesOfFieldsToPushDown= {};
+		String[] namesOfMethodsToDeclareAbstract= {};
+		String[][] signaturesOfMethodsToDeclareAbstract= {};
+
+		helper(selectedMethodNames, selectedMethodSignatures,
+				selectedFieldNames,
+				namesOfMethodsToPushDown, signaturesOfMethodsToPushDown,
+				namesOfFieldsToPushDown,
+				namesOfMethodsToDeclareAbstract, signaturesOfMethodsToDeclareAbstract, null, null);
+	}
+
 	@Test
 	public void testFail0() throws Exception {
 		String[] selectedMethodNames= {"f"};