IDEA-110654 Change Signature inserts unnecessary new line in comment block

java/java-psi-impl/src/com/intellij/psi/impl/source/javadoc/PsiDocCommentImpl.java

@@ -192,19 +192,27 @@ public class PsiDocCommentImpl extends LazyParseablePsiElement implements PsiDoc
         }
         needToAddNewline = true;
       }
-      if (anchor.getElementType() != DOC_TAG) {
-        final CharTable charTable = SharedImplUtil.findCharTableByTree(this);
-        final TreeElement newLine = Factory.createSingleLeafElement(DOC_COMMENT_DATA, "\n", 0, 1, charTable, getManager());
-        final TreeElement leadingAsterisk = Factory.createSingleLeafElement(DOC_COMMENT_LEADING_ASTERISKS, "*", 0, 1, charTable, getManager());
-        final TreeElement commentData = Factory.createSingleLeafElement(DOC_COMMENT_DATA, " ", 0, 1, charTable, getManager());
-        final TreeElement indentWS = Factory.createSingleLeafElement(DOC_COMMENT_DATA, " ", 0, 1, charTable, getManager());
-        newLine.getTreeParent().addChild(indentWS);
-        newLine.getTreeParent().addChild(leadingAsterisk);
-        newLine.getTreeParent().addChild(commentData);
-        super.addInternal(newLine, commentData, anchor, Boolean.FALSE);
 
-        anchor = commentData;
-        before = Boolean.FALSE;
+      if (anchor.getElementType() != DOC_TAG) {
+        if (nodeOnSameLineWithCommentStartBlock(anchor)
+            || !nodeIsNextAfterAsterisks(anchor)
+            || !docTagEndsWithLineFeedAndAsterisks(first))
+        {
+          final CharTable charTable = SharedImplUtil.findCharTableByTree(this);
+          final TreeElement newLine = Factory.createSingleLeafElement(DOC_COMMENT_DATA, "\n", 0, 1, charTable, getManager());
+          final TreeElement leadingAsterisk = Factory.createSingleLeafElement(DOC_COMMENT_LEADING_ASTERISKS, "*", 0, 1, charTable, getManager());
+          final TreeElement commentData = Factory.createSingleLeafElement(DOC_COMMENT_DATA, " ", 0, 1, charTable, getManager());
+          final TreeElement indentWS = Factory.createSingleLeafElement(DOC_COMMENT_DATA, " ", 0, 1, charTable, getManager());
+
+          newLine.getTreeParent().addChild(indentWS);
+          newLine.getTreeParent().addChild(leadingAsterisk);
+          newLine.getTreeParent().addChild(commentData);
+
+          super.addInternal(newLine, commentData, anchor, Boolean.FALSE);
+
+          anchor = commentData;
+          before = Boolean.FALSE;
+        }
       }
       else {
         needToAddNewline = true;
@@ -243,6 +251,45 @@ public class PsiDocCommentImpl extends LazyParseablePsiElement implements PsiDoc
     }
   }
 
+  private static boolean nodeIsNextAfterAsterisks(@NotNull ASTNode node) {
+    ASTNode current = TreeUtil.findSiblingBackward(node, DOC_COMMENT_LEADING_ASTERISKS);
+    if (current == null || current == node) return false;
+    while (current.getTreeNext() != node) {
+      current = current.getTreeNext();
+      CharSequence currentText = current.getChars();
+      if (CharArrayUtil.shiftForward(currentText, 0, " \t") != currentText.length()) return false;
+    }
+    return true;
+  }
+
+  private static boolean docTagEndsWithLineFeedAndAsterisks(@NotNull ASTNode node) {
+    assert (node.getElementType() == DOC_TAG);
+    ASTNode lastAsterisks = TreeUtil.findChildBackward(node, DOC_COMMENT_LEADING_ASTERISKS);
+    if (lastAsterisks == null || !lastAsterisks.getTreePrev().textContains('\n')) {
+      return false;
+    }
+    //So last asterisk is placed on new line, checking if after it there are no non-whitespace symbols
+    ASTNode last = node.getLastChildNode();
+    ASTNode current = lastAsterisks;
+    while (current != last) {
+      current = current.getTreeNext();
+      CharSequence currentText = current.getChars();
+      if (CharArrayUtil.shiftForward(currentText, 0, " \t") != currentText.length()) return false;
+    }
+    return true;
+  }
+
+  private static boolean nodeOnSameLineWithCommentStartBlock(@NotNull ASTNode node) {
+    ASTNode current = TreeUtil.findSiblingBackward(node, DOC_COMMENT_START);
+    if (current == null) return false;
+    if (current == node) return true;
+    while (current.getTreeNext() != node) {
+      current = current.getTreeNext();
+      if (current.textContains('\n')) return false;
+    }
+    return true;
+  }
+
   @Override
   public void deleteChildInternal(@NotNull ASTNode child) {
     if (child.getElementType() == DOC_TAG) {

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

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2000-2013 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+class Test {
+
+  /**
+   * Blah blah
+   *
+   * @param arg too many
+   * @return zero
+   */
+  public static int <caret>get(int arg) {
+    return 0;
+  }
+
+}

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

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2000-2013 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+class Test {
+
+  /**
+   * Blah blah
+   *
+   * @param newArgs too many
+   * @return zero
+   */
+  public static int get(double newArgs) {
+    return 0;
+  }
+
+}

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

@@ -182,6 +182,12 @@ public class ChangeSignatureTest extends LightRefactoringTestCase {
     }, false);
   }
 
+  public void testJavadocNoNewLineInserted() throws Exception {
+    doTest(null, new ParameterInfoImpl[]{
+      new ParameterInfoImpl(0, "newArgs", PsiType.DOUBLE),
+    }, false);
+  }
+
   public void testSuperCallFromOtherMethod() throws Exception {
     doTest(null, new ParameterInfoImpl[] {
       new ParameterInfoImpl(-1, "nnn", PsiType.INT, "-222"),