Fix move instance method when inner class is referenced (#1412)

* Fix move instance method when inner class is referenced

- fix MoveInstanceMethodProcessor to handle the cases where the
  method to move references a member class
- add new tests to MoveInstanceMethodTests
- fixes #1301
- verifies #1303 is fixed

org.eclipse.jdt.core.manipulation/META-INF/MANIFEST.MF

@@ -3,7 +3,7 @@ Automatic-Module-Name: org.eclipse.jdt.core.manipulation
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jdt.core.manipulation; singleton:=true
-Bundle-Version: 1.21.100.qualifier
+Bundle-Version: 1.21.200.qualifier
 Bundle-Vendor: %providerName
 Bundle-Activator: org.eclipse.jdt.internal.core.manipulation.JavaManipulationPlugin
 Bundle-Localization: plugin

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/MoveInstanceMethodProcessor.java

@@ -67,6 +67,7 @@
 import org.eclipse.jdt.core.IJavaProject;
 import org.eclipse.jdt.core.IMember;
 import org.eclipse.jdt.core.IMethod;
+import org.eclipse.jdt.core.IPackageDeclaration;
 import org.eclipse.jdt.core.IType;
 import org.eclipse.jdt.core.ITypeHierarchy;
 import org.eclipse.jdt.core.JavaCore;
@@ -98,12 +99,14 @@
 import org.eclipse.jdt.core.dom.MethodRefParameter;
 import org.eclipse.jdt.core.dom.Modifier;
 import org.eclipse.jdt.core.dom.Name;
+import org.eclipse.jdt.core.dom.NameQualifiedType;
 import org.eclipse.jdt.core.dom.NullLiteral;
 import org.eclipse.jdt.core.dom.PostfixExpression;
 import org.eclipse.jdt.core.dom.PrefixExpression;
 import org.eclipse.jdt.core.dom.PrimitiveType;
 import org.eclipse.jdt.core.dom.QualifiedName;
 import org.eclipse.jdt.core.dom.SimpleName;
+import org.eclipse.jdt.core.dom.SimpleType;
 import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
 import org.eclipse.jdt.core.dom.SuperFieldAccess;
 import org.eclipse.jdt.core.dom.SuperMethodInvocation;
@@ -564,6 +567,56 @@ public boolean visit(final AnonymousClassDeclaration node) {
 			return super.visit(node);
 		}
 
+		@Override
+		public boolean visit(SimpleType node) {
+			ITypeBinding nodeTypeBinding= ASTNodes.getEnclosingType(node);
+			ITypeBinding simpleTypeBinding= node.resolveBinding();
+			if (nodeTypeBinding != null && !nodeTypeBinding.isEqualTo(simpleTypeBinding) && simpleTypeBinding.isMember()) {
+				AST ast= node.getAST();
+				if (node.getParent() instanceof ClassInstanceCreation parent && parent.getExpression() == null && !Modifier.isStatic(simpleTypeBinding.getModifiers())) {
+					ClassInstanceCreation newCreation= ast.newClassInstanceCreation();
+					newCreation.setType((Type) fRewrite.createCopyTarget(node));
+					newCreation.setExpression(ASTNodeFactory.newName(node.getAST(), fTargetName));
+					List<Expression> args= parent.arguments();
+					for (Expression arg : args) {
+						newCreation.arguments().add(fRewrite.createCopyTarget(arg));
+					}
+					List<Type> typeArgs= parent.typeArguments();
+					for (Type typeArg : typeArgs) {
+						newCreation.typeArguments().add(fRewrite.createCopyTarget(typeArg));
+					}
+					if (parent.getAnonymousClassDeclaration() != null) {
+						newCreation.setAnonymousClassDeclaration((AnonymousClassDeclaration) fRewrite.createCopyTarget(parent.getAnonymousClassDeclaration()));
+					}
+					fRewrite.replace(node.getParent(), newCreation, null);
+				} else {
+					try {
+						if (fMethod.getCompilationUnit().equals(getTargetType().getCompilationUnit())) {
+							String qualifiedTypeName= Bindings.getFullyQualifiedName(simpleTypeBinding);
+							int index= qualifiedTypeName.lastIndexOf("."); //$NON-NLS-1$
+							int startIndex= 0;
+							IPackageDeclaration[] packages= fMethod.getCompilationUnit().getPackageDeclarations();
+							if (packages.length > 0) {
+								String packageName= fMethod.getCompilationUnit().getPackageDeclarations()[0].getElementName();
+								if (packageName.length() > 0 && qualifiedTypeName.startsWith(packageName)) {
+									startIndex= packageName.length() + 1;
+								}
+								String qualifier= qualifiedTypeName.substring(startIndex, index);
+								QualifiedName qualifiedName= (QualifiedName) fRewrite.createStringPlaceholder(qualifier, ASTNode.QUALIFIED_NAME);
+								NameQualifiedType newQualifiedType= ast.newNameQualifiedType(qualifiedName, (SimpleName) fRewrite.createCopyTarget(node.getName()));
+								fRewrite.replace(node, newQualifiedType, null);
+							}
+						}
+
+					} catch (JavaModelException e) {
+						// TODO Auto-generated catch block
+						e.printStackTrace();
+					}
+				}
+			}
+			return super.visit(node);
+		}
+
 		@Override
 		public boolean visit(final ClassInstanceCreation node) {
 			Assert.isNotNull(node);
@@ -572,6 +625,31 @@ public boolean visit(final ClassInstanceCreation node) {
 				if (declaration != null)
 					visit(declaration);
 				return false;
+			} else {
+				Type type= node.getType();
+				if (node.getExpression() == null && type.isSimpleType() && type.getRoot() == node.getRoot()) {
+					ITypeBinding nodeTypeBinding= ASTNodes.getEnclosingType(node);
+					ITypeBinding newTypeBinding= type.resolveBinding();
+					if (nodeTypeBinding != null && newTypeBinding.isMember() && !nodeTypeBinding.isEqualTo(newTypeBinding) && !Modifier.isStatic(newTypeBinding.getModifiers())) {
+						AST ast= node.getAST();
+						ClassInstanceCreation newCreation= ast.newClassInstanceCreation();
+						newCreation.setType((Type) fRewrite.createCopyTarget(type));
+						newCreation.setExpression(ASTNodeFactory.newName(node.getAST(), fTargetName));
+						List<Expression> args= node.arguments();
+						for (Expression arg : args) {
+							newCreation.arguments().add(fRewrite.createCopyTarget(arg));
+						}
+						List<Type> typeArgs= node.typeArguments();
+						for (Type typeArg : typeArgs) {
+							newCreation.typeArguments().add(fRewrite.createCopyTarget(typeArg));
+						}
+						if (node.getAnonymousClassDeclaration() != null) {
+							newCreation.setAnonymousClassDeclaration((AnonymousClassDeclaration) fRewrite.createCopyTarget(node.getAnonymousClassDeclaration()));
+						}
+						fRewrite.replace(node, newCreation, null);
+					}
+				}
+
 			}
 			return super.visit(node);
 		}
@@ -1021,6 +1099,19 @@ public boolean visit(final SimpleName node) {
 			if (isFieldAccess(node) && !isTargetAccess(node)) {
 				fResult.add(node);
 				fStatus.merge(RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.MoveInstanceMethodProcessor_this_reference, JavaStatusContext.create(fMethod.getCompilationUnit(), node)));
+			} else if (node.getParent() instanceof ClassInstanceCreation constructor && constructor.getExpression() == null) {
+				Type type= constructor.getType();
+				ITypeBinding binding= type.resolveBinding();
+				if (binding != null && binding.isMember() && !Modifier.isPublic(binding.getModifiers()) && !Modifier.isStatic(binding.getModifiers())) {
+					fResult.add(node);
+					fStatus.merge(RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.MoveInstanceMethodProcessor_this_reference, JavaStatusContext.create(fMethod.getCompilationUnit(), node)));
+				}
+			} else if (node.getParent() instanceof SimpleType simpleType) {
+				ITypeBinding binding= simpleType.resolveBinding();
+				if (binding != null && binding.isMember() && !Modifier.isPublic(binding.getModifiers()) && !Modifier.isStatic(binding.getModifiers())) {
+					fResult.add(node);
+					fStatus.merge(RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.MoveInstanceMethodProcessor_this_reference, JavaStatusContext.create(fMethod.getCompilationUnit(), node)));
+				}
 			}
 			return false;
 		}

org.eclipse.jdt.ui.tests.refactoring/META-INF/MANIFEST.MF

@@ -3,7 +3,7 @@ Automatic-Module-Name: org.eclipse.jdt.ui.tests.refactoring
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.jdt.ui.tests.refactoring; singleton:=true
-Bundle-Version: 3.15.400.qualifier
+Bundle-Version: 3.15.500.qualifier
 Bundle-Activator: org.eclipse.jdt.ui.tests.refactoring.infra.RefactoringTestPlugin
 Bundle-ActivationPolicy: lazy
 Bundle-Vendor: %Plugin.providerName

org.eclipse.jdt.ui.tests.refactoring/pom.xml

@@ -19,7 +19,7 @@
   </parent>
   <groupId>org.eclipse.jdt</groupId>
   <artifactId>org.eclipse.jdt.ui.tests.refactoring</artifactId>
-  <version>3.15.400-SNAPSHOT</version>
+  <version>3.15.500-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
   <properties>
     <testSuite>${project.artifactId}</testSuite>

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test71/in/A.java

@@ -0,0 +1,21 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    private class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+
+    public void m() {
+        InnerInterface inner = new InnerInterface();
+        inner.innerMethod();
+    }
+}
+
+class B {
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test71/out/A.java

@@ -0,0 +1,21 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+}
+
+class B {
+
+	public void m(A a) {
+	    A.InnerInterface inner = a.new InnerInterface();
+	    inner.innerMethod();
+	}
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test72/in/A.java

@@ -0,0 +1,21 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    private static class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+
+    public void m() {
+        InnerInterface inner = new InnerInterface();
+        inner.innerMethod();
+    }
+}
+
+class B {
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test72/out/A.java

@@ -0,0 +1,21 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    static class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+}
+
+class B {
+
+	public void m() {
+	    A.InnerInterface inner = new A.InnerInterface();
+	    inner.innerMethod();
+	}
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test73/in/A.java

@@ -0,0 +1,17 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    private class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+
+    public void m() {
+        InnerInterface inner = new InnerInterface();
+        inner.innerMethod();
+    }
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test73/in/B.java

@@ -0,0 +1,5 @@
+package p1;
+
+class B {
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test73/out/A.java

@@ -0,0 +1,12 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test73/out/B.java

@@ -0,0 +1,12 @@
+package p1;
+
+import p1.A.InnerInterface;
+
+class B {
+
+	public void m(A a) {
+	    InnerInterface inner = a.new InnerInterface();
+	    inner.innerMethod();
+	}
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test74/in/A.java

@@ -0,0 +1,17 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    private static class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+
+    public void m() {
+        InnerInterface inner = new InnerInterface();
+        inner.innerMethod();
+    }
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test74/in/B.java

@@ -0,0 +1,5 @@
+package p1;
+
+class B {
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test74/out/A.java

@@ -0,0 +1,12 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    static class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test74/out/B.java

@@ -0,0 +1,12 @@
+package p1;
+
+import p1.A.InnerInterface;
+
+class B {
+
+	public void m() {
+	    InnerInterface inner = new InnerInterface();
+	    inner.innerMethod();
+	}
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test75/in/A.java

@@ -0,0 +1,21 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    protected class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+
+    public void m() {
+        InnerInterface inner = new InnerInterface();
+        inner.innerMethod();
+    }
+}
+
+class B {
+
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/canMove/test75/out/A.java

@@ -0,0 +1,21 @@
+package p1;
+
+public class A {
+
+    B b;
+
+    protected class InnerInterface {
+        void innerMethod() {
+
+        }
+    }
+}
+
+class B {
+
+	public void m(A a) {
+	    A.InnerInterface inner = a.new InnerInterface();
+	    inner.innerMethod();
+	}
+
+}