Fix move method to handle field access modifier adjustment (#1427)

* Fix move method to handle field access modifier adjustment

- fix MoveInstanceMethodProcessor.createMethodBody() to perform any
  needed adjustments to field modifiers if required by the move
- add new test to MoveInstanceMethodTests
- fixes #1297

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

@@ -521,7 +521,7 @@ public final class MethodBodyRewriter extends ASTVisitor {
 		protected final Set<IBinding> fStaticImports= new HashSet<>();
 
 		/** The refactoring status */
-		protected final RefactoringStatus fStatus= new RefactoringStatus();
+		protected final RefactoringStatus fStatus;
 
 		/** The target compilation unit rewrite to use */
 		protected final CompilationUnitRewrite fTargetRewrite;
@@ -539,17 +539,20 @@ public final class MethodBodyRewriter extends ASTVisitor {
 		 *            the source method declaration
 		 * @param adjustments
 		 *            the map of elements to visibility adjustments
+		 * @param status
+		 *            refactoring status
 		 */
-		public MethodBodyRewriter(final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final CompilationUnitRewrite targetRewrite, final ASTRewrite rewrite, final MethodDeclaration sourceDeclaration, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments) {
+		public MethodBodyRewriter(final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final CompilationUnitRewrite targetRewrite, final ASTRewrite rewrite, final MethodDeclaration sourceDeclaration, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final RefactoringStatus status) {
 			Assert.isNotNull(targetRewrite);
 			Assert.isNotNull(rewrite);
 			Assert.isNotNull(sourceDeclaration);
 			fTargetRewrite= targetRewrite;
 			fRewrite= rewrite;
 			fRewrites= rewrites;
 			fDeclaration= sourceDeclaration;
-			fAdjustments= adjustments;
 			fStaticImports.clear();
+			fAdjustments= adjustments;
+			fStatus= status;
 			ImportRewriteUtil.collectImports(fMethod.getJavaProject(), sourceDeclaration, new HashSet<>(), fStaticImports, false);
 		}
 
@@ -843,6 +846,30 @@ else if (binding instanceof IVariableBinding) {
 							targetType= enclosingType;
 						}
 					}
+					final IField field= (IField) variable.getJavaElement();
+					try {
+						if (field != null && !Modifier.isPublic(field.getFlags())) {
+							boolean checkRequired= true;
+							ITypeBinding pClass= fTarget.getType();
+							while (pClass != null && pClass.isMember()) {
+								pClass= pClass.getDeclaringClass();
+								if (pClass != null && pClass.isEqualTo(targetType)) {
+									checkRequired= false;
+									break;
+								}
+							}
+							if (checkRequired) {
+								boolean same= field.getAncestor(IJavaElement.PACKAGE_FRAGMENT).equals(fTarget.getJavaElement().getAncestor(IJavaElement.PACKAGE_FRAGMENT));
+								final Modifier.ModifierKeyword keyword= same ? null : Modifier.ModifierKeyword.PUBLIC_KEYWORD;
+								final String modifier= same ? RefactoringCoreMessages.MemberVisibilityAdjustor_change_visibility_default : RefactoringCoreMessages.MemberVisibilityAdjustor_change_visibility_public;
+								if (MemberVisibilityAdjustor.hasLowerVisibility(field.getFlags(), (keyword == null ? Modifier.NONE : keyword.toFlagValue())) && MemberVisibilityAdjustor.needsVisibilityAdjustments(field, keyword, fAdjustments)) {
+									fAdjustments.put(field, new MemberVisibilityAdjustor.OutgoingMemberVisibilityAdjustment(field, keyword, RefactoringStatus.createWarningStatus(Messages.format(RefactoringCoreMessages.MemberVisibilityAdjustor_change_visibility_field_warning, new String[] { BindingLabelProviderCore.getBindingLabel(variable, JavaElementLabelsCore.ALL_FULLY_QUALIFIED), modifier }), JavaStatusContext.create(field))));
+								}
+							}
+						}
+					} catch (JavaModelException e) {
+						// ignore as this should not happen
+					}
 				}
 				if (method != null && targetType != null) {
 					targetType= targetType.getTypeDeclaration();
@@ -2376,11 +2403,13 @@ public final ASTNode getTargetNode() throws JavaModelException {
 	 *            the source method declaration
 	 * @param adjustments
 	 *            the map of elements to visibility adjustments
+	 * @param status
+	 *            refactoring status
 	 */
 	protected void createMethodBody(final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final CompilationUnitRewrite rewriter,
-			final ASTRewrite rewrite, final MethodDeclaration declaration, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments) {
+			final ASTRewrite rewrite, final MethodDeclaration declaration, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final RefactoringStatus status) {
 		Assert.isNotNull(declaration);
-		declaration.getBody().accept(new MethodBodyRewriter(rewrites, rewriter, rewrite, declaration, adjustments));
+		declaration.getBody().accept(new MethodBodyRewriter(rewrites, rewriter, rewrite, declaration, adjustments, status));
 	}
 
 	/**
@@ -2570,7 +2599,7 @@ protected void createMethodCopy(IDocument document, MethodDeclaration declaratio
 			createMethodArguments(rewrites, rewrite, declaration, adjustments, status);
 			createMethodTypeParameters(rewrite, declaration, status);
 			createMethodComment(rewrite, declaration);
-			createMethodBody(rewrites, rewriter, rewrite, declaration, adjustments);
+			createMethodBody(rewrites, rewriter, rewrite, declaration, adjustments, status);
 		} finally {
 			if (fMethod.getCompilationUnit().equals(getTargetType().getCompilationUnit()))
 				rewriter.clearImportRewrites();

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

@@ -0,0 +1,18 @@
+class B {
+    public void f() {
+
+    }
+}
+
+class A {
+    B b;
+    private int c;
+
+    public void m() {
+        n(c);
+    }
+
+    private void n(int x) {
+
+    }
+}

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

@@ -0,0 +1,18 @@
+class B {
+    public void f() {
+
+    }
+
+	public void m(A a) {
+	    a.n(a.c);
+	}
+}
+
+class A {
+    B b;
+    int c;
+
+    void n(int x) {
+
+    }
+}

org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/MoveInstanceMethodTests.java

@@ -704,6 +704,10 @@ public void test78() throws Exception {
 		helper1(new String[] { "p1.A", "p1.B" }, "p1.A", 12, 17, 12, 18, FIELD, "b", true, true);
 	}
 
+	@Test
+	public void test79() throws Exception {
+		helper1(new String[] { "A" }, "A", 11, 17, 11, 18, FIELD, "b", true, true);
+	}
 	// Move mA1 to field fB, do not inline delegator
 	@Test
 	public void test3() throws Exception {