IDEA-4384 Refactorings should obey "align when multiline"

1. Method parameters/method call arguments are reformatted during method return type/name/modifiers change now (corresponding logic is added at PSI level);
2. Corresponding tests are added;

java/java-impl/src/com/intellij/psi/impl/source/PsiModifierListImpl.java

@@ -24,6 +24,7 @@ import com.intellij.psi.impl.PsiImplUtil;
 import com.intellij.psi.impl.cache.ModifierFlags;
 import com.intellij.psi.impl.java.stubs.JavaStubElementTypes;
 import com.intellij.psi.impl.java.stubs.PsiModifierListStub;
+import com.intellij.psi.impl.source.codeStyle.CodeEditUtil;
 import com.intellij.psi.impl.source.tree.CompositeElement;
 import com.intellij.psi.impl.source.tree.Factory;
 import com.intellij.psi.impl.source.tree.JavaElementType;
@@ -190,6 +191,14 @@ public class PsiModifierListImpl extends JavaStubPsiElement<PsiModifierListStub>
   public void setModifierProperty(@NotNull String name, boolean value) throws IncorrectOperationException{
     checkSetModifierProperty(name, value);
 
+    // There is a possible case that parameters list occupies more than one line and its elements are aligned. Modifiers list change
+    // changes horizontal position of parameters list start, hence, we need to reformat them in order to preserve alignment.
+    PsiElement methodCandidate = getParent();
+    if (methodCandidate instanceof PsiMethod) {
+      PsiMethod method = (PsiMethod)methodCandidate;
+      CodeEditUtil.markToReformat(method.getParameterList().getNode(), true);
+    }
+
     IElementType type = NAME_TO_KEYWORD_TYPE_MAP.get(name);
 
     CompositeElement treeElement = (CompositeElement)getNode();

java/java-impl/src/com/intellij/psi/impl/source/PsiTypeElementImpl.java

@@ -23,6 +23,7 @@ import com.intellij.pom.java.LanguageLevel;
 import com.intellij.psi.*;
 import com.intellij.psi.impl.DebugUtil;
 import com.intellij.psi.impl.PsiImplUtil;
+import com.intellij.psi.impl.source.codeStyle.CodeEditUtil;
 import com.intellij.psi.impl.source.tree.*;
 import com.intellij.psi.scope.PsiScopeProcessor;
 import com.intellij.psi.tree.IElementType;
@@ -290,5 +291,45 @@ public class PsiTypeElementImpl extends CompositePsiElement implements PsiTypeEl
   public PsiAnnotation addAnnotation(@NotNull @NonNls String qualifiedName) {
     throw new UnsupportedOperationException();//todo
   }
+
+  @Override
+  public PsiElement replace(@NotNull PsiElement newElement) throws IncorrectOperationException {
+    PsiElement result = super.replace(newElement);
+
+    // We want to reformat method call arguments on method return type change because there is a possible situation that they are aligned
+    // and the change breaks the alignment.
+    // Example:
+    //     Object test(1,
+    //                 2) {}
+    // Suppose we're changing return type to 'MyCustomClass'. We get the following if parameter list is not reformatted:
+    //     MyCustomClass test(1,
+    //                 2) {}
+    PsiElement parent = result.getParent();
+    if (parent instanceof PsiMethod) {
+      PsiMethod method = (PsiMethod)parent;
+      CodeEditUtil.markToReformat(method.getParameterList().getNode(), true);
+    }
+
+    // We cover situation like below here:
+    //     int test(int i, int j) {}
+    //     ...
+    //     int i = test(1,
+    //                  2);
+    // I.e. the point is to avoid code like below during changing 'test()' return type from 'int' to 'long':
+    //     long i = test(1,
+    //                  2);
+    else if (parent instanceof PsiVariable) {
+      PsiVariable variable = (PsiVariable)parent;
+      if (variable.hasInitializer()) {
+        PsiExpression methodCallCandidate = variable.getInitializer();
+        if (methodCallCandidate instanceof PsiMethodCallExpression) {
+          PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)methodCallCandidate;
+          CodeEditUtil.markToReformat(methodCallExpression.getArgumentList().getNode(), true);
+        }
+      }
+    }
+
+    return result;
+  }
 }
 

java/java-impl/src/com/intellij/psi/impl/source/tree/java/PsiIdentifierImpl.java

@@ -17,8 +17,10 @@ package com.intellij.psi.impl.source.tree.java;
 
 import com.intellij.psi.*;
 import com.intellij.psi.impl.source.Constants;
+import com.intellij.psi.impl.source.codeStyle.CodeEditUtil;
 import com.intellij.psi.impl.source.tree.LeafPsiElement;
 import com.intellij.psi.tree.IElementType;
+import com.intellij.util.IncorrectOperationException;
 import org.jetbrains.annotations.NotNull;
 
 public class PsiIdentifierImpl extends LeafPsiElement implements PsiIdentifier, PsiJavaToken {
@@ -39,6 +41,27 @@ public class PsiIdentifierImpl extends LeafPsiElement implements PsiIdentifier,
     }
   }
 
+  @Override
+  public PsiElement replace(@NotNull PsiElement newElement) throws IncorrectOperationException {
+    PsiElement result = super.replace(newElement);
+
+    // We want to reformat method parameters on method name change as well because there is a possible situation that they are aligned
+    // and method name change breaks the alignment.
+    // Example:
+    //     public void test(int i,
+    //                      int j) {}
+    // Suppose we're renaming the method to test123. We get the following if parameter list is not reformatted:
+    //     public void test123(int i,
+    //                     int j) {}
+    PsiElement methodCandidate = result.getParent();
+    if (methodCandidate instanceof PsiMethod) {
+      PsiMethod method = (PsiMethod)methodCandidate;
+      CodeEditUtil.markToReformat(method.getParameterList().getNode(), true);
+    }
+
+    return result;
+  }
+
   public String toString(){
     return "PsiIdentifier:" + getText();
   }

java/java-impl/src/com/intellij/psi/impl/source/tree/java/PsiReferenceExpressionImpl.java

@@ -31,6 +31,7 @@ import com.intellij.psi.impl.PsiImplUtil;
 import com.intellij.psi.impl.PsiManagerEx;
 import com.intellij.psi.impl.source.SourceJavaCodeReference;
 import com.intellij.psi.impl.source.SourceTreeToPsiMap;
+import com.intellij.psi.impl.source.codeStyle.CodeEditUtil;
 import com.intellij.psi.impl.source.parsing.ExpressionParsing;
 import com.intellij.psi.impl.source.resolve.ClassResolverProcessor;
 import com.intellij.psi.impl.source.resolve.JavaResolveCache;
@@ -626,6 +627,25 @@ public class PsiReferenceExpressionImpl extends ExpressionPsiElement implements
     return getChildRole(getFirstChildNode()) == ChildRole.QUALIFIER;
   }
 
+  @Override
+  public void subtreeChanged() {
+    super.subtreeChanged();
+
+    // We want to reformat method call arguments on method name change because there is a possible situation that they are aligned
+    // and method change breaks the alignment.
+    // Example:
+    //     test(1,
+    //          2);
+    // Suppose we're renaming the method to test123. We get the following if parameter list is not reformatted:
+    //     test123(1,
+    //          2);
+    PsiElement methodCallCandidate = getParent();
+    if (methodCallCandidate instanceof PsiMethodCallExpression) {
+      PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)methodCallCandidate;
+      CodeEditUtil.markToReformat(methodCallExpression.getArgumentList().getNode(), true);
+    }
+  }
+
   private String getCachedTextSkipWhiteSpaceAndComments() {
     String whiteSpaceAndComments = myCachedTextSkipWhiteSpaceAndComments;
     if (whiteSpaceAndComments == null) {

java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodNameChange.java

@@ -0,0 +1,10 @@
+public class Test {
+  public void <caret>test123(int i,
+                      int j) {
+  }
+
+  public void foo() {
+    test123(1,
+            2);
+  }
+}

java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodNameChange_after.java

@@ -0,0 +1,10 @@
+public class Test {
+  public void test123asd(int i,
+                         int j) {
+  }
+
+  public void foo() {
+    test123asd(1,
+               2);
+  }
+}

java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodReturnTypeChange.java

@@ -0,0 +1,6 @@
+public class Test {
+  public String <caret>test123(int i,
+                               int j) {
+    return null;
+  }
+}

java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodReturnTypeChange_after.java

@@ -0,0 +1,6 @@
+public class Test {
+  public Exception test123(int i,
+                           int j) {
+    return null;
+  }
+}

java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodVisibilityChange.java

@@ -0,0 +1,5 @@
+public class Test {
+  public void <caret>test123(int i,
+                             int j) {
+  }
+}

java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodVisibilityChange_after.java

@@ -0,0 +1,5 @@
+public class Test {
+  protected void test123(int i,
+                         int j) {
+  }
+}

java/java-tests/testData/refactoring/makeMethodStatic/afterPreserveParametersAlignment.java

@@ -0,0 +1,5 @@
+public class Test {
+    public static void test123(int i,
+                               int j) {
+    }
+}

java/java-tests/testData/refactoring/makeMethodStatic/beforePreserveParametersAlignment.java

@@ -0,0 +1,5 @@
+public class Test {
+    public void <caret>test123(int i,
+                        int j) {
+    }
+}

java/java-tests/testData/refactoring/renameMethod/multi/alignedMultilineParameters/after/AlignedMultilineParameters.java

@@ -0,0 +1,11 @@
+public class AlignedMultilineParameters {
+
+  public void test123asd(int i,
+                         int j) {
+  }
+
+  public void foo() {
+    test123asd(1,
+               2);
+  }
+}

java/java-tests/testData/refactoring/renameMethod/multi/alignedMultilineParameters/before/AlignedMultilineParameters.java

@@ -0,0 +1,11 @@
+public class AlignedMultilineParameters {
+
+  public void test123(int i,
+                      int j) {
+  }
+
+  public void foo() {
+    test123(1,
+            2);
+  }
+}

java/java-tests/testSrc/com/intellij/refactoring/ChangeSignatureTest.java

@@ -230,6 +230,24 @@ public class ChangeSignatureTest extends LightCodeInsightTestCase {
     }, false);
   }
 
+  public void testMethodParametersAlignmentAfterMethodNameChange() throws Exception {
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS = true;
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS_IN_CALLS = true;
+    doTest(null, "test123asd", null, new SimpleParameterGen(), new SimpleExceptionsGen(), false);
+  }
+
+  public void testMethodParametersAlignmentAfterMethodVisibilityChange() throws Exception {
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS = true;
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS_IN_CALLS = true;
+    doTest("protected", null, null, new SimpleParameterGen(), new SimpleExceptionsGen(), false);
+  }
+
+  public void testMethodParametersAlignmentAfterMethodReturnTypeChange() throws Exception {
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS = true;
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS_IN_CALLS = true;
+    doTest(null, null, "Exception", new SimpleParameterGen(), new SimpleExceptionsGen(), false);
+  }
+
   private void doTest(String newReturnType, ParameterInfoImpl[] parameterInfos, final boolean generateDelegate) throws Exception {
     doTest(null, null, newReturnType, parameterInfos, new ThrownExceptionInfo[0], generateDelegate);
   }
@@ -268,7 +286,10 @@ public class ChangeSignatureTest extends LightCodeInsightTestCase {
   }
 
   private static class SimpleParameterGen implements GenParams {
-    private final ParameterInfoImpl[] myInfos;
+    private ParameterInfoImpl[] myInfos;
+
+    private SimpleParameterGen() {
+    }
 
     private SimpleParameterGen(ParameterInfoImpl[] infos) {
       myInfos = infos;
@@ -276,6 +297,12 @@ public class ChangeSignatureTest extends LightCodeInsightTestCase {
 
     @Override
     public ParameterInfoImpl[] genParams(PsiMethod method) {
+      if (myInfos == null) {
+        myInfos = new ParameterInfoImpl[method.getParameterList().getParametersCount()];
+        for (int i = 0; i < myInfos.length; i++) {
+          myInfos[i] = new ParameterInfoImpl(i);
+        }
+      }
       for (ParameterInfoImpl info : myInfos) {
         info.updateFromMethod(method);
       }

java/java-tests/testSrc/com/intellij/refactoring/MakeMethodStaticTest.java

@@ -170,6 +170,16 @@ public class MakeMethodStaticTest extends LightCodeInsightTestCase {
     checkResultByFile("/refactoring/makeMethodStatic/afterPreserveTypeParams.java");
   }
 
+  public void testPreserveParametersAlignment() throws Exception {
+    doTest();
+  }
+
+  private void doTest() throws Exception {
+    configureByFile("/refactoring/makeMethodStatic/before" + getTestName(false) + ".java");
+    perform(false);
+    checkResultByFile("/refactoring/makeMethodStatic/after" + getTestName(false) + ".java");
+  }
+
   private void perform(boolean addClassParameter) {
     PsiElement element = TargetElementUtilBase.findTargetElement(myEditor, TargetElementUtilBase.ELEMENT_NAME_ACCEPTED);
     assertTrue(element instanceof PsiMethod);

java/java-tests/testSrc/com/intellij/refactoring/RenameMethodMultiTest.java

@@ -39,7 +39,15 @@ public class RenameMethodMultiTest extends MultiFileTestCase {
     doTest("pack1.A", "void staticMethod(int i)", "renamedStaticMethod");
   }
 
+  public void testAlignedMultilineParameters() throws Exception {
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS = true;
+    getCurrentCodeStyleSettings().ALIGN_MULTILINE_PARAMETERS_IN_CALLS = true;
+    doTest("void test123(int i, int j)", "test123asd");
+  }
 
+  private void doTest(final String methodSignature, final String newName) throws Exception {
+    doTest(getTestName(false), methodSignature, newName);
+  }
 
   private void doTest(final String className, final String methodSignature, final String newName) throws Exception {
     doTest(new PerformAction() {

platform/lang-impl/src/com/intellij/psi/impl/source/PostprocessReformattingAspect.java

@@ -512,12 +512,23 @@ public class PostprocessReformattingAspect implements PomModelAspect, Disposable
 
   private static void handleReformatMarkers(final FileViewProvider key, final TreeSet<PostprocessFormattingTask> rangesToProcess) {
     final Document document = key.getDocument();
+    if (document == null) {
+      return;
+    }
     for (final FileElement fileElement : ((SingleRootFileViewProvider)key).getKnownTreeRoots()) {
       fileElement.acceptTree(new RecursiveTreeElementWalkingVisitor() {
         protected void visitNode(TreeElement element) {
           if (CodeEditUtil.isMarkedToReformatBefore(element)) {
             CodeEditUtil.markToReformatBefore(element, false);
-            rangesToProcess.add(new ReformatWithHeadingWhitespaceTask(document.createRangeMarker(element.getStartOffset(), element.getStartOffset())));
+            rangesToProcess.add(new ReformatWithHeadingWhitespaceTask(
+              document.createRangeMarker(element.getStartOffset(), element.getStartOffset()))
+            );
+          }
+          else if (CodeEditUtil.isMarkedToReformat(element)) {
+            CodeEditUtil.markToReformat(element, false);
+            rangesToProcess.add(new ReformatWithHeadingWhitespaceTask(
+              document.createRangeMarker(element.getStartOffset(), element.getStartOffset() + element.getTextLength()))
+            );
           }
           super.visitNode(element);
         }

platform/lang-impl/src/com/intellij/psi/impl/source/codeStyle/CodeEditUtil.java

@@ -37,6 +37,7 @@ import com.intellij.psi.impl.source.tree.TreeElement;
 import com.intellij.psi.impl.source.tree.TreeUtil;
 import com.intellij.psi.templateLanguages.OuterLanguageElement;
 import com.intellij.psi.tree.IElementType;
+import com.intellij.psi.tree.TokenSet;
 import com.intellij.psi.util.PsiUtilBase;
 import com.intellij.util.text.CharArrayUtil;
 import org.jetbrains.annotations.NotNull;
@@ -45,6 +46,7 @@ import org.jetbrains.annotations.Nullable;
 public class CodeEditUtil {
   private static final Key<Boolean> GENERATED_FLAG = new Key<Boolean>("GENERATED_FLAG");
   private static final Key<Integer> INDENT_INFO = new Key<Integer>("INDENT_INFO");
+  private static final Key<Boolean> REFORMAT_BEFORE_KEY = new Key<Boolean>("REFORMAT_BEFORE_KEY");
   private static final Key<Boolean> REFORMAT_KEY = new Key<Boolean>("REFORMAT_KEY");
 
   public static final Key<Boolean> OUTER_OK = new Key<Boolean>("OUTER_OK");
@@ -128,7 +130,11 @@ public class CodeEditUtil {
 
   public static void saveWhitespacesInfo(final ASTNode first) {
     if(first == null || isNodeGenerated(first) || getOldIndentation(first) >= 0) return;
-    final PsiFile containingFile = first.getPsi().getContainingFile();
+    PsiElement psiElement = first.getPsi();
+    if (psiElement == null) {
+      return;
+    }
+    final PsiFile containingFile = psiElement.getContainingFile();
     final Helper helper = HelperFactory.createHelper(containingFile.getFileType(), containingFile.getProject());
     setOldIndentation((TreeElement)first, helper.getIndent(first));
   }
@@ -155,7 +161,7 @@ public class CodeEditUtil {
     final ASTNode nextLeaf = TreeUtil.nextLeaf(first);
     parent.removeRange(first, last.getTreeNext());
     ASTNode nextLeafToAdjust = nextLeaf;
-    if (nextLeafToAdjust != null && nextLeafToAdjust.getTreeParent() == null) {
+    if (nextLeafToAdjust != null && prevLeaf != null && nextLeafToAdjust.getTreeParent() == null) {
       //next element has invalidated
       nextLeafToAdjust = prevLeaf.getTreeNext();
     }
@@ -231,6 +237,7 @@ public class CodeEditUtil {
     return null;
   }
 
+  @Nullable
   private static ASTNode makePlaceHolderBetweenTokens(ASTNode left, final ASTNode right, boolean forceReformat, final boolean normalizeTailingWhitespace) {
     if(right == null) return left;
 
@@ -315,10 +322,10 @@ public class CodeEditUtil {
 
   public static void markToReformatBefore(final ASTNode right, boolean value) {
     if (value) {
-      right.putCopyableUserData(REFORMAT_KEY, true);
+      right.putCopyableUserData(REFORMAT_BEFORE_KEY, true);
     }
     else {
-      right.putCopyableUserData(REFORMAT_KEY, null);
+      right.putCopyableUserData(REFORMAT_BEFORE_KEY, null);
     }
   }
 
@@ -329,6 +336,7 @@ public class CodeEditUtil {
     return result;
   }
 
+  @Nullable
   public static String getStringWhiteSpaceBetweenTokens(ASTNode first, ASTNode second, PsiFile file) {
     final FormattingModelBuilder modelBuilder = LanguageFormatting.INSTANCE.forContext(file);
     if (modelBuilder == null) {
@@ -398,10 +406,31 @@ public class CodeEditUtil {
   }
 
   public static boolean isMarkedToReformatBefore(final TreeElement element) {
-    return element.getCopyableUserData(REFORMAT_KEY) != null;
+    return element.getCopyableUserData(REFORMAT_BEFORE_KEY) != null;
   }
 
+  @Nullable
   public static PsiElement createLineFeed(final PsiManager manager) {
     return Factory.createSingleLeafElement(TokenType.WHITE_SPACE, "\n", 0, 1, null, manager).getPsi();
   }
+
+  /**
+   * Allows to answer if given node is configured to be reformatted.
+   *
+   * @param node    node to check
+   * @return        <code>true</code> if given node is configured to be reformatted; <code>false</code> otherwise
+   */
+  public static boolean isMarkedToReformat(final ASTNode node) {
+    return node.getCopyableUserData(REFORMAT_KEY) != null;
+  }
+
+  /**
+   * Allows to define if given element should be reformatted later.
+   *
+   * @param node      target element which <code>'reformat'</code> status should be changed
+   * @param value     <code>true</code> if the element should be reformatted; <code>false</code> otherwise
+   */
+  public static void markToReformat(final ASTNode node, boolean value) {
+    node.putCopyableUserData(REFORMAT_KEY, value ? true : null);
+  }
 }