From d6f3b1ec85130e12c11b464ced8eaf453ad6fb98 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Mon, 6 Dec 2021 21:17:22 +0100 Subject: [PATCH] [java] format javadoc after change signature fixes IDEA-281568, IDEA-139879, IDEA-55288 GitOrigin-RevId: c0a83cf3288ee5c78ed8c53dd7bae73fc3de59dd --- .../JavaChangeSignatureUsageProcessor.java | 3 ++ .../refactoring/util/RefactoringUtil.java | 8 ++++- .../source/javadoc/PsiDocCommentImpl.java | 9 +++-- .../recordCanBeClass/afterNormal.java | 2 +- .../JavadocNotBrokenAfterDelete.java | 11 +++++++ .../JavadocNotBrokenAfterDelete_after.java | 10 ++++++ .../JavadocOfDeleted_after.java | 1 + ...mentAfterMethodReturnTypeChange_after.java | 2 +- .../changeSignature/MultilineJavadoc.java | 15 +++++++++ .../MultilineJavadocWithoutFormatting.java | 15 +++++++++ ...ltilineJavadocWithoutFormatting_after.java | 14 ++++++++ .../MultilineJavadoc_after.java | 12 +++++++ .../NoGapsInParameterTags.java | 23 +++++++++++++ .../NoGapsInParameterTags_after.java | 22 +++++++++++++ .../changeSignature/ParamJavadoc0_after.java | 4 +-- .../changeSignature/ParamJavadoc1_after.java | 2 +- .../ParamJavadocRenamedReordered_after.java | 2 +- .../changeSignature/ParamJavadoc_after.java | 2 +- ...anonicalConstructorAddParameter_after.java | 3 +- ...cordCanonicalConstructorReorder_after.java | 6 ++-- .../RecordHeaderDeleteRename_after.java | 1 - .../ReturnJavadocAdded_after.java | 3 +- .../java/refactoring/ChangeSignatureTest.java | 33 +++++++++++++++++++ 23 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete_after.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting_after.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc_after.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags.java create mode 100644 java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/changeSignature/JavaChangeSignatureUsageProcessor.java b/java/java-impl/src/com/intellij/refactoring/changeSignature/JavaChangeSignatureUsageProcessor.java index 296f733d8eaa..ba77889552fb 100644 --- a/java/java-impl/src/com/intellij/refactoring/changeSignature/JavaChangeSignatureUsageProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/changeSignature/JavaChangeSignatureUsageProcessor.java @@ -23,6 +23,7 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleSettings; import com.intellij.psi.codeStyle.VariableKind; +import com.intellij.psi.impl.source.codeStyle.javadoc.CommentFormatter; import com.intellij.psi.impl.source.resolve.JavaResolveUtil; import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.javadoc.PsiDocTag; @@ -1078,6 +1079,8 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr methodDocComment.add(JavaPsiFacade.getElementFactory(method.getProject()).createDocTagFromText("@return")); } } + CommentFormatter formatter = new CommentFormatter(method.getContainingFile()); + formatter.processComment(methodDocComment.getNode()); } } diff --git a/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java b/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java index fcf4dbd656a8..1d135d03d33e 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java +++ b/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java @@ -9,6 +9,7 @@ import com.intellij.codeInsight.highlighting.HighlightManager; import com.intellij.java.refactoring.JavaRefactoringBundle; import com.intellij.lang.java.JavaLanguage; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.editor.RangeMarker; import com.intellij.openapi.editor.colors.EditorColors; @@ -24,6 +25,7 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.impl.PsiImplUtil; +import com.intellij.psi.impl.source.codeStyle.javadoc.CommentFormatter; import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.javadoc.PsiDocTag; import com.intellij.psi.javadoc.PsiDocTagValue; @@ -1177,8 +1179,12 @@ public final class RefactoringUtil { paramTag.delete(); } for (PsiDocTag psiDocTag : newTags) { - anchor = anchor != null && anchor.isValid() ? docComment.addAfter(psiDocTag, anchor) : docComment.add(psiDocTag); + anchor = anchor != null && anchor.isValid() + ? docComment.addAfter(psiDocTag, anchor) + : docComment.add(psiDocTag); } + CommentFormatter formatter = new CommentFormatter(method.getContainingFile()); + formatter.processComment(docComment.getNode()); } @NotNull diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/javadoc/PsiDocCommentImpl.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/javadoc/PsiDocCommentImpl.java index 6683de61c736..76c4c4c71475 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/javadoc/PsiDocCommentImpl.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/javadoc/PsiDocCommentImpl.java @@ -11,6 +11,7 @@ import com.intellij.psi.impl.source.SourceTreeToPsiMap; import com.intellij.psi.impl.source.tree.*; import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.javadoc.PsiDocTag; +import com.intellij.psi.javadoc.PsiDocToken; import com.intellij.psi.tree.ChildRoleBase; import com.intellij.psi.tree.IElementType; import com.intellij.psi.tree.TokenSet; @@ -195,16 +196,19 @@ public class PsiDocCommentImpl extends LazyParseablePsiElement implements PsiDoc addNewLineToTag((CompositeElement)first, getContainingFile(), getManager()); } else { - removeEndingAsterisksFromTag((CompositeElement)first); + removeEndingAsterisksFromTagIfNeeded((CompositeElement)first); } } return first; } - private static void removeEndingAsterisksFromTag(CompositeElement tag) { + private static void removeEndingAsterisksFromTagIfNeeded(CompositeElement tag) { ASTNode current = tag.getLastChildNode(); while (current != null && current.getElementType() == DOC_COMMENT_DATA) { + if (current instanceof PsiDocToken) { + return; + } current = current.getTreePrev(); } if (current != null && current.getElementType() == DOC_COMMENT_LEADING_ASTERISKS) { @@ -219,6 +223,7 @@ 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; diff --git a/java/java-tests/testData/inspection/recordCanBeClass/afterNormal.java b/java/java-tests/testData/inspection/recordCanBeClass/afterNormal.java index f0e88388ed67..c25530cfa0d9 100644 --- a/java/java-tests/testData/inspection/recordCanBeClass/afterNormal.java +++ b/java/java-tests/testData/inspection/recordCanBeClass/afterNormal.java @@ -18,7 +18,7 @@ final class R { * @param b b value * @param c c value * @param d d value - * @param s s value + * @param s s value */ R(int a, boolean b, float c, double d, String s) { this.a = a; diff --git a/java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete.java b/java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete.java new file mode 100644 index 000000000000..63067da1dcef --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete.java @@ -0,0 +1,11 @@ +class A { + /** + * Foo + * + * @param i1 an int + * @param i2 another int + */ + void foo(int i1, int i2) { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete_after.java b/java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete_after.java new file mode 100644 index 000000000000..f6a386e0a77a --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/JavadocNotBrokenAfterDelete_after.java @@ -0,0 +1,10 @@ +class A { + /** + * Foo + * + * @param i1 an int + */ + void foo(int i1) { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/JavadocOfDeleted_after.java b/java/java-tests/testData/refactoring/changeSignature/JavadocOfDeleted_after.java index af54692fc2d8..6fedda000d4c 100644 --- a/java/java-tests/testData/refactoring/changeSignature/JavadocOfDeleted_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/JavadocOfDeleted_after.java @@ -16,6 +16,7 @@ class C { /** * This is the role - + * * @param role another desc * @return return description */ diff --git a/java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodReturnTypeChange_after.java b/java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodReturnTypeChange_after.java index 470572e3a70f..467b7aaf7e68 100644 --- a/java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodReturnTypeChange_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/MethodParametersAlignmentAfterMethodReturnTypeChange_after.java @@ -1,6 +1,6 @@ public class Test { /** - * @param i + * @param i * @param j * @return */ diff --git a/java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc.java b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc.java new file mode 100644 index 000000000000..980fedc503d0 --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc.java @@ -0,0 +1,15 @@ +class A { + /** + * Demo. + * + * @param a + * a. + * @param b + * b. + * @param c + * c. + */ + public void demo(int a, int b, int c) { + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting.java b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting.java new file mode 100644 index 000000000000..980fedc503d0 --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting.java @@ -0,0 +1,15 @@ +class A { + /** + * Demo. + * + * @param a + * a. + * @param b + * b. + * @param c + * c. + */ + public void demo(int a, int b, int c) { + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting_after.java b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting_after.java new file mode 100644 index 000000000000..62d40b1808cc --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadocWithoutFormatting_after.java @@ -0,0 +1,14 @@ +class A { + /** + * Demo. + * @param b + * b. + * @param a + * a. + * @param c + * c. + */ + public void demo(int b, int a, int c) { + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc_after.java b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc_after.java new file mode 100644 index 000000000000..5c6694f19511 --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/MultilineJavadoc_after.java @@ -0,0 +1,12 @@ +class A { + /** + * Demo. + * + * @param b b. + * @param a a. + * @param c c. + */ + public void demo(int b, int a, int c) { + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags.java b/java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags.java new file mode 100644 index 000000000000..37bab9e3af24 --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags.java @@ -0,0 +1,23 @@ +public class MyClass3 +{ + /** + * This method does amazing things. + * + * @param a First parameter. + * @param b Second parameter. + * @param c Third parameter. + * + * @return A magic string. + * + * @since Blabla 1.2. + */ + public String myMethod(int a, long b, boolean c) + { + return "Hi there!"; + } + + public static void main(String[] args) + { + System.out.println(new MyClass3().myMethod(1, "2", true)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags_after.java b/java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags_after.java new file mode 100644 index 000000000000..590caccdb497 --- /dev/null +++ b/java/java-tests/testData/refactoring/changeSignature/NoGapsInParameterTags_after.java @@ -0,0 +1,22 @@ +public class MyClass3 +{ + /** + * This method does amazing things. + * + * @param b Second parameter. + * @param a First parameter. + * @param c Third parameter. + * @param d + * @return A magic string. + * @since Blabla 1.2. + */ + public String myMethod(int b, long a, boolean c, short d) + { + return "Hi there!"; + } + + public static void main(String[] args) + { + System.out.println(new MyClass3().myMethod(1, "2", true, )); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc0_after.java b/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc0_after.java index 9b5380692526..c41e12b522ac 100644 --- a/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc0_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc0_after.java @@ -5,9 +5,9 @@ class X { */ public class TestRefactorLink { /** - @return nothing - * @param z zparam + * @param z zparam * @param y yparam + * @return nothing */ public void mymethod(int z, int y) { } } diff --git a/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc1_after.java b/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc1_after.java index 314ffd6e42f0..ceb96b590566 100644 --- a/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc1_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc1_after.java @@ -5,8 +5,8 @@ class X { */ public class TestRefactorLink { /** - * @return nothing * @param z yparam + * @return nothing */ public void mymethod(boolean z) { } } diff --git a/java/java-tests/testData/refactoring/changeSignature/ParamJavadocRenamedReordered_after.java b/java/java-tests/testData/refactoring/changeSignature/ParamJavadocRenamedReordered_after.java index dc9349153919..6e670d295289 100644 --- a/java/java-tests/testData/refactoring/changeSignature/ParamJavadocRenamedReordered_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/ParamJavadocRenamedReordered_after.java @@ -1,7 +1,7 @@ class X { /** - * @param a aparam + * @param a aparam * @param c * @param b1 bparam */ diff --git a/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc_after.java b/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc_after.java index 6e97d8010abf..c41e12b522ac 100644 --- a/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/ParamJavadoc_after.java @@ -5,9 +5,9 @@ class X { */ public class TestRefactorLink { /** - * @return nothing * @param z zparam * @param y yparam + * @return nothing */ public void mymethod(int z, int y) { } } diff --git a/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorAddParameter_after.java b/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorAddParameter_after.java index 58afb7a5b7cf..9f2c234a8e2b 100644 --- a/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorAddParameter_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorAddParameter_after.java @@ -1,5 +1,6 @@ /** - * Record javadoc + * Record javadoc + * * @param x x * @param y */ diff --git a/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorReorder_after.java b/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorReorder_after.java index db512d089008..e09d11475e51 100644 --- a/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorReorder_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/RecordCanonicalConstructorReorder_after.java @@ -1,12 +1,14 @@ /** - * Record javadoc + * Record javadoc + * * @param y y * @param z z * @param x x */ record Rec(int y, int z, int x) { /** - * Constructor javadoc + * Constructor javadoc + * * @param y y * @param z z * @param x x diff --git a/java/java-tests/testData/refactoring/changeSignature/RecordHeaderDeleteRename_after.java b/java/java-tests/testData/refactoring/changeSignature/RecordHeaderDeleteRename_after.java index 8745cf71ecba..36181b43e7e7 100644 --- a/java/java-tests/testData/refactoring/changeSignature/RecordHeaderDeleteRename_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/RecordHeaderDeleteRename_after.java @@ -1,6 +1,5 @@ /** * @param yyy y - * */ record Rec(long yyy) { public long yyy() { diff --git a/java/java-tests/testData/refactoring/changeSignature/ReturnJavadocAdded_after.java b/java/java-tests/testData/refactoring/changeSignature/ReturnJavadocAdded_after.java index f67c59429a2b..2a8f88a30d04 100644 --- a/java/java-tests/testData/refactoring/changeSignature/ReturnJavadocAdded_after.java +++ b/java/java-tests/testData/refactoring/changeSignature/ReturnJavadocAdded_after.java @@ -2,7 +2,8 @@ class X { /** * documentation + * * @return */ - public int mymethod() { } + public int mymethod() { } } diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ChangeSignatureTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ChangeSignatureTest.java index 358d656f2255..497f40dba8ed 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ChangeSignatureTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ChangeSignatureTest.java @@ -5,6 +5,7 @@ import com.intellij.codeInsight.TargetElementUtil; import com.intellij.lang.java.JavaLanguage; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CommonCodeStyleSettings; +import com.intellij.psi.codeStyle.JavaCodeStyleSettings; import com.intellij.refactoring.BaseRefactoringProcessor; import com.intellij.refactoring.changeSignature.ChangeSignatureProcessor; import com.intellij.refactoring.changeSignature.JavaThrownExceptionInfo; @@ -589,5 +590,37 @@ public class ChangeSignatureTest extends ChangeSignatureBaseTest { doTest(null, null, "@org.jetbrains.annotations.NotNull java.lang.String", method -> new ParameterInfoImpl[0], false); } + public void testMultilineJavadoc() { // IDEA-281568 + doTest(null, null, null, method -> new ParameterInfoImpl[]{ + ParameterInfoImpl.create(1).withType(PsiType.INT).withName("b"), + ParameterInfoImpl.create(0).withType(PsiType.INT).withName("a"), + ParameterInfoImpl.create(2).withType(PsiType.INT).withName("c"), + }, false); + } + + public void testMultilineJavadocWithoutFormatting() { // IDEA-281568 + JavaCodeStyleSettings.getInstance(getProject()).ENABLE_JAVADOC_FORMATTING = false; + doTest(null, null, null, method -> new ParameterInfoImpl[]{ + ParameterInfoImpl.create(1).withType(PsiType.INT).withName("b"), + ParameterInfoImpl.create(0).withType(PsiType.INT).withName("a"), + ParameterInfoImpl.create(2).withType(PsiType.INT).withName("c"), + }, false); + } + + public void testJavadocNotBrokenAfterDelete() { // IDEA-139879 + doTest(null, null, null, method -> new ParameterInfoImpl[]{ + ParameterInfoImpl.create(0).withType(PsiType.INT).withName("i1") + }, false); + } + + public void testNoGapsInParameterTags() { // IDEA-139879 + doTest(null, null, null, method -> new ParameterInfoImpl[]{ + ParameterInfoImpl.create(0).withType(PsiType.INT).withName("b"), + ParameterInfoImpl.create(1).withType(PsiType.LONG).withName("a"), + ParameterInfoImpl.create(2).withType(PsiType.BOOLEAN).withName("c"), + ParameterInfoImpl.createNew().withType(PsiType.SHORT).withName("d"), + }, false); + } + /* workers */ }