From 29d17b55c3bbe7445a0f7db0b36ac8ac4d308092 Mon Sep 17 00:00:00 2001 From: "Denis.Zhdanov" Date: Thu, 11 Jul 2013 16:05:02 +0400 Subject: [PATCH] IDEA-109631 Javadocs formatting depends on current method indentation --- .../codeStyle/javadoc/CommentFormatter.java | 77 ++++---- .../psi/formatter/java/SCR11296_after.java | 7 +- .../java/AbstractJavaFormatterTest.java | 71 ++++---- .../formatter/java/JavadocFormatterTest.java | 164 ++++++++++++++++++ 4 files changed, 247 insertions(+), 72 deletions(-) diff --git a/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/javadoc/CommentFormatter.java b/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/javadoc/CommentFormatter.java index 9f900c0f6573..983e67e3e03f 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/javadoc/CommentFormatter.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/javadoc/CommentFormatter.java @@ -15,16 +15,21 @@ */ package com.intellij.psi.impl.source.codeStyle.javadoc; +import com.intellij.ide.highlighter.JavaFileType; import com.intellij.lang.ASTNode; +import com.intellij.lang.java.JavaLanguage; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.LineTokenizer; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleSettings; import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.psi.impl.source.SourceTreeToPsiMap; import com.intellij.psi.javadoc.PsiDocComment; +import com.intellij.psi.util.PsiUtil; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** @@ -59,7 +64,7 @@ public class CommentFormatter { processElementComment(psiElement); } - private void processElementComment(PsiElement psiElement) { + private void processElementComment(@Nullable PsiElement psiElement) { if (psiElement instanceof PsiClass) { String newCommentText = formatClassComment((PsiClass)psiElement); replaceDocComment(newCommentText, (PsiDocCommentOwner)psiElement); @@ -77,7 +82,7 @@ public class CommentFormatter { } } - private void replaceDocComment(String newCommentText, final PsiDocCommentOwner psiDocCommentOwner) { + private void replaceDocComment(@Nullable String newCommentText, @NotNull final PsiDocCommentOwner psiDocCommentOwner) { final PsiDocComment oldComment = psiDocCommentOwner.getDocComment(); if (newCommentText != null) newCommentText = stripSpaces(newCommentText); if (newCommentText == null || oldComment == null || newCommentText.equals(oldComment.getText())) { @@ -99,7 +104,7 @@ public class CommentFormatter { private static String stripSpaces(String text) { String[] lines = LineTokenizer.tokenize(text.toCharArray(), false); - StringBuffer buf = new StringBuffer(text.length()); + StringBuilder buf = new StringBuilder(text.length()); for (int i = 0; i < lines.length; i++) { if (i > 0) buf.append('\n'); buf.append(rTrim(lines[i])); @@ -117,7 +122,7 @@ public class CommentFormatter { } @Nullable - private String formatClassComment(PsiClass psiClass) { + private String formatClassComment(@NotNull PsiClass psiClass) { final String info = getOrigCommentInfo(psiClass); if (info == null) return null; @@ -126,7 +131,7 @@ public class CommentFormatter { } @Nullable - private String formatMethodComment(PsiMethod psiMethod) { + private String formatMethodComment(@NotNull PsiMethod psiMethod) { final String info = getOrigCommentInfo(psiMethod); if (info == null) return null; @@ -135,7 +140,7 @@ public class CommentFormatter { } @Nullable - private String formatFieldComment(PsiField psiField) { + private String formatFieldComment(@NotNull PsiField psiField) { final String info = getOrigCommentInfo(psiField); if (info == null) return null; @@ -149,8 +154,9 @@ public class CommentFormatter { * @param element the specified element * @return text chunk */ + @Nullable private static String getOrigCommentInfo(PsiDocCommentOwner element) { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); PsiElement e = element.getFirstChild(); if (!(e instanceof PsiComment)) { // no comments for this element @@ -188,39 +194,38 @@ public class CommentFormatter { } /** - * For the specified element returns its indentation - * - * @param element the specified element - * @return indentation as string + * Computes indentation of PsiClass, PsiMethod and PsiField elements after formatting + * @param element PsiClass or PsiMethod or PsiField + * @return indentation size */ - private static String getIndent(PsiElement element) { - PsiElement e = element.getFirstChild(); - PsiWhiteSpace lastWS = null; - for (; ; e = e.getNextSibling()) { - if (e instanceof PsiWhiteSpace) { - lastWS = (PsiWhiteSpace)e; - } - else if (e instanceof PsiComment) { - lastWS = null; - } - else { + private int getIndentSpecial(@NotNull PsiElement element) { + assert(element instanceof PsiClass || + element instanceof PsiField || + element instanceof PsiMethod); + + int indentSize = mySettings.getIndentSize(JavaFileType.INSTANCE); + boolean doNotIndentTopLevelClassMembers = mySettings.getCommonSettings(JavaLanguage.INSTANCE).DO_NOT_INDENT_TOP_LEVEL_CLASS_MEMBERS; + + int indent = 0; + PsiClass top = PsiUtil.getTopLevelClass(element); + while (top != null && !element.isEquivalentTo(top)) { + if (doNotIndentTopLevelClassMembers && element.getParent().isEquivalentTo(top)) { break; } + element = element.getParent(); + indent += indentSize; } - e = lastWS == null ? element.getPrevSibling() : lastWS; - if (!(e instanceof PsiWhiteSpace)) return ""; - PsiWhiteSpace ws = (PsiWhiteSpace)e; - String t = ws.getText(); - int l = t.length(); - int i = l; - while (--i >= 0) { - char ch = t.charAt(i); - if (ch == '\n' || ch == '\r') break; - } - if (i < 0) return t; - i++; - if (i == l) return ""; - return t.substring(i); + return indent; + } + + /** + * Used while formatting javadocs. We need precise element indentation after formatting to wrap comments correctly. + * Used only for PsiClass, PsiMethod and PsiFields. + * @return indent which would be used for the given element when it's formatted according to the current code style settings + */ + @NotNull + private String getIndent(@NotNull PsiElement element) { + return StringUtil.repeatSymbol(' ', getIndentSpecial(element)); } } diff --git a/java/java-tests/testData/psi/formatter/java/SCR11296_after.java b/java/java-tests/testData/psi/formatter/java/SCR11296_after.java index 502dc50d6e03..26c117dadfb3 100644 --- a/java/java-tests/testData/psi/formatter/java/SCR11296_after.java +++ b/java/java-tests/testData/psi/formatter/java/SCR11296_after.java @@ -1,9 +1,10 @@ class A { /** * The String class represents - * character strings. All string literals in Java - * programs, such as "abc", are - * implemented as instances of this class. + * character strings. All string literals in + * Java programs, such as "abc", + * are implemented as instances of this + * class. * Some text after empty line * * @author Lee Boynton diff --git a/java/java-tests/testSrc/com/intellij/psi/formatter/java/AbstractJavaFormatterTest.java b/java/java-tests/testSrc/com/intellij/psi/formatter/java/AbstractJavaFormatterTest.java index bd2f80cf45cc..df732703eda2 100644 --- a/java/java-tests/testSrc/com/intellij/psi/formatter/java/AbstractJavaFormatterTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/formatter/java/AbstractJavaFormatterTest.java @@ -39,6 +39,7 @@ import com.intellij.util.IncorrectOperationException; import com.intellij.util.text.LineReader; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.io.ByteArrayInputStream; import java.io.File; @@ -129,35 +130,47 @@ public abstract class AbstractJavaFormatterTest extends LightIdeaTestCase { doTextTest(Action.REFORMAT, text, textAfter); } - public void doTextTest(final Action action, final String text, String textAfter) throws IncorrectOperationException { + public void doTextTest(@NotNull final Action action, @NotNull final String text, @NotNull String textAfter) throws IncorrectOperationException { final PsiFile file = createFile("A.java", text); - - if (myLineRange != null) { - final DocumentImpl document = new DocumentImpl(text); - myTextRange = - new TextRange(document.getLineStartOffset(myLineRange.getStartOffset()), document.getLineEndOffset(myLineRange.getEndOffset())); - } - - /* - CommandProcessor.getInstance().executeCommand(getProject(), new Runnable() { - public void run() { - ApplicationManager.getApplication().runWriteAction(new Runnable() { - public void run() { - performFormatting(file); - } - }); - } - }, null, null); - - assertEquals(prepareText(textAfter), prepareText(file.getText())); - - - */ - final PsiDocumentManager manager = PsiDocumentManager.getInstance(getProject()); final Document document = manager.getDocument(file); + if (document == null) { + fail("Document is null"); + return; + } + replaceAndProcessDocument(action, text, file, document); + assertEquals(textAfter, document.getText()); + manager.commitDocument(document); + assertEquals(textAfter, file.getText()); + } + public void formatEveryoneAndCheckIfResultEqual(@NotNull final String...before) { + assert before.length > 1; + final PsiFile file = createFile("A.java", ""); + final PsiDocumentManager manager = PsiDocumentManager.getInstance(getProject()); + final Document document = manager.getDocument(file); + String afterFirst = replaceAndProcessDocument(Action.REFORMAT, before[0], file, document); + for (String nextBefore: before) { + assertEquals(afterFirst, replaceAndProcessDocument(Action.REFORMAT, nextBefore, file, document)); + } + } + @NotNull + private String replaceAndProcessDocument(@NotNull final Action action, + @NotNull final String text, + @NotNull final PsiFile file, + @Nullable final Document document) throws IncorrectOperationException + { + if (document == null) { + fail("Don't expect the document to be null"); + return null; + } + if (myLineRange != null) { + final DocumentImpl doc = new DocumentImpl(text); + myTextRange = + new TextRange(doc.getLineStartOffset(myLineRange.getStartOffset()), doc.getLineEndOffset(myLineRange.getEndOffset())); + } + final PsiDocumentManager manager = PsiDocumentManager.getInstance(getProject()); CommandProcessor.getInstance().executeCommand(getProject(), new Runnable() { @Override public void run() { @@ -181,15 +194,7 @@ public abstract class AbstractJavaFormatterTest extends LightIdeaTestCase { } }, action == Action.REFORMAT ? ReformatCodeProcessor.COMMAND_NAME : "", ""); - - if (document == null) { - fail("Don't expect the document to be null"); - return; - } - assertEquals(textAfter, document.getText()); - manager.commitDocument(document); - assertEquals(textAfter, file.getText()); - + return document.getText(); } public void doMethodTest(@NonNls final String before, @NonNls final String after) throws Exception { diff --git a/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavadocFormatterTest.java b/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavadocFormatterTest.java index 28e4de5099f6..d46b5ecb336b 100644 --- a/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavadocFormatterTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavadocFormatterTest.java @@ -352,4 +352,168 @@ public class JavadocFormatterTest extends AbstractJavaFormatterTest { doClassTest(before, after); } + + public void testJavadocFormattingIndependentOfMethodIndentation() { + getCurrentCodeStyleSettings().RIGHT_MARGIN = 50; + getCurrentCodeStyleSettings().ENABLE_JAVADOC_FORMATTING = true; + getCurrentCodeStyleSettings().WRAP_COMMENTS = true; + getCurrentCodeStyleSettings().JD_LEADING_ASTERISKS_ARE_ENABLED = true; + getCurrentCodeStyleSettings().JD_P_AT_EMPTY_LINES = false; + getCurrentCodeStyleSettings().JD_KEEP_EMPTY_LINES = false; + getCurrentCodeStyleSettings().JD_ADD_BLANK_AFTER_DESCRIPTION = false; + String before1 = "class A {\n" + + " /**\n" + + " * Some really great independent test approach purpose live fish\n" + + " * banana split string be accurate when writing tests and code\n" + + " * read write buffer.\n" + + " *\n" + + " * Some text after empty line\n" + + " *\n" + + " */\n" + + "void foo() {\n" + + "\n" + + "}\n" + + "}"; + + String before2 = "class A {\n" + + " /**\n" + + " * Some really great independent test approach purpose live fish\n" + + " * banana split string be accurate when writing tests and code\n" + + " * read write buffer.\n" + + " *\n" + + " * Some text after empty line\n" + + " *\n" + + " */\n" + + " void foo() {\n" + + "\n" + + " }\n" + + "}"; + + formatEveryoneAndCheckIfResultEqual(before1, before2); + } + + public void testJavadocAlignmentForInnerClasses() { + getCurrentCodeStyleSettings().RIGHT_MARGIN = 40; + getCurrentCodeStyleSettings().ENABLE_JAVADOC_FORMATTING = true; + getCurrentCodeStyleSettings().WRAP_COMMENTS = true; + getCurrentCodeStyleSettings().JD_LEADING_ASTERISKS_ARE_ENABLED = true; + + String code = "public class Outer {\n" + + " class Inner {\n" + + " /**\n" + + " * Password from wild forest big house\n" + + " */\n" + + " public int getMagic() {\n" + + " return 312;\n" + + " }\n" + + "\n" + + "class InnerInner {\n" + + "/**\n" + + " * Special magic needs special rules\n" + + " */\n" + + "public int innerMagic() {\n" + + " return 1;\n" + + "}\n" + + "}\n" + + " }\n" + + "}"; + + String result = "public class Outer {\n" + + " class Inner {\n" + + " /**\n" + + " * Password from wild forest big\n" + + " * house\n" + + " */\n" + + " public int getMagic() {\n" + + " return 312;\n" + + " }\n" + + "\n" + + " class InnerInner {\n" + + " /**\n" + + " * Special magic needs\n" + + " * special rules\n" + + " */\n" + + " public int innerMagic() {\n" + + " return 1;\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; + doTextTest(code, result); + } + + public void testAlignmentWithNoTopClassMembersIndentation() { + getCurrentCodeStyleSettings().RIGHT_MARGIN = 40; + getCurrentCodeStyleSettings().WRAP_COMMENTS = true; + getCurrentCodeStyleSettings().JD_LEADING_ASTERISKS_ARE_ENABLED = true; + getCurrentCodeStyleSettings().getCommonSettings(JavaLanguage.INSTANCE).DO_NOT_INDENT_TOP_LEVEL_CLASS_MEMBERS = true; + + String before = "public class Outer {\n" + + "class Inner {\n" + + "/**\n" + + " * Password from wild forest big\n" + + " * house\n" + + " */\n" + + "public int getMagic() {\n" + + " return 312;\n" + + "}\n" + + "\n" + + "class InnerInner {\n" + + "/**\n" + + " * Special magic needs special rules\n" + + " */\n" + + "public int innerMagic() {\n" + + " return 1;\n" + + "}\n" + + "\n" + + "class InnerInnerInner {\n" + + "int iii;\n" + + "class TripleInner {\n" + + "int ti;\n" + + "}\n" + + "}\n" + + "}\n" + + "}\n" + + " public static void main(String[] args) {\n" + + " System.out.println(\"AAA!\");\n" + + " }\n" + + "}"; + + String after = "public class Outer {\n" + + "class Inner {\n" + + " /**\n" + + " * Password from wild forest big\n" + + " * house\n" + + " */\n" + + " public int getMagic() {\n" + + " return 312;\n" + + " }\n" + + "\n" + + " class InnerInner {\n" + + " /**\n" + + " * Special magic needs special\n" + + " * rules\n" + + " */\n" + + " public int innerMagic() {\n" + + " return 1;\n" + + " }\n" + + "\n" + + " class InnerInnerInner {\n" + + " int iii;\n" + + "\n" + + " class TripleInner {\n" + + " int ti;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + + "\n" + + "public static void main(String[] args) {\n" + + " System.out.println(\"AAA!\");\n" + + "}\n" + + "}"; + + doTextTest(before, after); + + } }