From 831949acee04ff2aeb9d7c8394a5089831e9184d Mon Sep 17 00:00:00 2001 From: kazachka Date: Sat, 12 Nov 2016 21:32:39 +0300 Subject: [PATCH] Issue #3426: remove warning on PACKAGE_DEF preceded by javadoc not separated by line --- .../whitespace/EmptyLineSeparatorCheck.java | 51 +++++++++++- .../EmptyLineSeparatorCheckTest.java | 83 +++++++++++++++---- .../whitespace/InputEmptyLineSeparator.java | 3 +- ...tyLineSeparatorMultilineCommentHeader.java | 9 ++ .../package-info/test1/package-info.java | 5 ++ .../package-info/test2/package-info.java | 4 + .../package-info/test3/package-info.java | 5 ++ .../package-info/test4/package-info.java | 5 ++ 8 files changed, 146 insertions(+), 19 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test1/package-info.java create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test2/package-info.java create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test3/package-info.java create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test4/package-info.java diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java index caccf1cd965e..2ac62c767aa0 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java @@ -27,6 +27,7 @@ import com.puppycrawl.tools.checkstyle.api.FileContents; import com.puppycrawl.tools.checkstyle.api.TokenTypes; import com.puppycrawl.tools.checkstyle.utils.CommonUtils; +import com.puppycrawl.tools.checkstyle.utils.JavadocUtils; /** * Checks for empty line separators after header, package, all import declarations, @@ -250,6 +251,11 @@ public void setAllowMultipleEmptyLinesInsideClassMembers(boolean allow) { allowMultipleEmptyLinesInsideClassMembers = allow; } + @Override + public boolean isCommentNodesRequired() { + return true; + } + @Override public int[] getDefaultTokens() { return getAcceptableTokens(); @@ -285,7 +291,10 @@ public void visitToken(DetailAST ast) { processMultipleLinesInside(ast); } - final DetailAST nextToken = ast.getNextSibling(); + DetailAST nextToken = ast.getNextSibling(); + if (nextToken != null && isTrailingComment(nextToken)) { + nextToken = nextToken.getNextSibling(); + } if (nextToken != null) { final int astType = ast.getType(); switch (astType) { @@ -405,7 +414,14 @@ && hasNotAllowedTwoEmptyLinesBefore(ast)) { */ private void processPackage(DetailAST ast, DetailAST nextToken) { if (ast.getLineNo() > 1 && !hasEmptyLineBefore(ast)) { - log(ast.getLineNo(), MSG_SHOULD_BE_SEPARATED, ast.getText()); + if (getFileContents().getFileName().endsWith("package-info.java")) { + if (ast.getFirstChild().getChildCount() == 0 && !isPrecededByJavadoc(ast)) { + log(ast.getLineNo(), MSG_SHOULD_BE_SEPARATED, ast.getText()); + } + } + else { + log(ast.getLineNo(), MSG_SHOULD_BE_SEPARATED, ast.getText()); + } } if (!hasEmptyLineAfter(ast)) { log(nextToken.getLineNo(), MSG_SHOULD_BE_SEPARATED, nextToken.getText()); @@ -487,8 +503,12 @@ private boolean hasEmptyLineAfter(DetailAST token) { if (lastToken == null) { lastToken = token.getLastChild(); } + DetailAST nextToken = token.getNextSibling(); + if (isTrailingComment(nextToken)) { + nextToken = nextToken.getNextSibling(); + } // Start of the next token - final int nextBegin = token.getNextSibling().getLineNo(); + final int nextBegin = nextToken.getLineNo(); // End of current token. final int currentEnd = lastToken.getLineNo(); return hasEmptyLine(currentEnd + 1, nextBegin - 1); @@ -533,6 +553,31 @@ private boolean hasEmptyLineBefore(DetailAST token) { return lineBefore.trim().isEmpty(); } + /** + * Check if token is preceded by javadoc comment. + * @param token token for check. + * @return true, if token is preceded by javadoc comment. + */ + private static boolean isPrecededByJavadoc(DetailAST token) { + boolean result = false; + final DetailAST previous = token.getPreviousSibling(); + if (previous.getType() == TokenTypes.BLOCK_COMMENT_BEGIN + && JavadocUtils.isJavadocComment(previous)) { + result = true; + } + return result; + } + + /** + * Check if token is a trailing comment. + * @param comment ast node that represents comment. + * @return true, if given ast is trailing comment. + */ + private static boolean isTrailingComment(DetailAST comment) { + return comment.getType() == TokenTypes.SINGLE_LINE_COMMENT + && comment.getLineNo() == comment.getPreviousSibling().getLineNo(); + } + /** * If variable definition is a type field. * @param variableDef variable definition. diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java index e00cbff60269..77db21d20ede 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java @@ -55,14 +55,15 @@ public void testDefault() throws Exception { final String[] expected = { "21: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "import"), - "35: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"), - "38: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "VARIABLE_DEF"), - "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"), - "43: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"), - "57: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"), - "62: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), - "79: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), - "110: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"), + "28: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "//"), + "36: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"), + "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "VARIABLE_DEF"), + "40: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"), + "44: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"), + "58: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"), + "63: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), + "80: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), + "111: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"), }; verify(checkConfig, getPath("InputEmptyLineSeparator.java"), expected); } @@ -75,13 +76,14 @@ public void testAllowNoEmptyLineBetweenFields() throws Exception { final String[] expected = { "21: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "import"), - "35: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"), - "39: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"), - "43: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"), - "57: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"), - "62: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), - "79: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), - "110: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"), + "28: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "//"), + "36: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"), + "40: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"), + "44: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"), + "58: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"), + "63: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), + "80: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"), + "111: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"), }; verify(checkConfig, getPath("InputEmptyLineSeparator.java"), expected); } @@ -191,4 +193,55 @@ public void testAllowMultipleEmptyLinesInsideClassMembers() throws Exception { getPath("InputEmptyLineSeparatorMultipleEmptyLinesInside.java"), expected); } + + @Test + public void testDisAllowMultilineCommentBeforePackageDefinition() throws Exception { + final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class); + final String[] expected = { + "5: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "package"), + }; + verify(checkConfig, + getPath("InputEmptyLineSeparatorMultilineCommentHeader.java"), + expected); + } + + @Test + public void testAllowPackageAnnotation() throws Exception { + final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class); + final String[] expected = CommonUtils.EMPTY_STRING_ARRAY; + verify(checkConfig, + getPath("package-info/test1/package-info.java"), + expected); + } + + @Test + public void testAllowJavadocBeforePackage() throws Exception { + final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class); + final String[] expected = CommonUtils.EMPTY_STRING_ARRAY; + verify(checkConfig, + getPath("package-info/test2/package-info.java"), + expected); + } + + @Test + public void testDisAllowBlockCommentBeforePackage() throws Exception { + final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class); + final String[] expected = { + "3: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "package"), + }; + verify(checkConfig, + getPath("package-info/test3/package-info.java"), + expected); + } + + @Test + public void testAllowSingleLineCommentPackage() throws Exception { + final DefaultConfiguration checkConfig = createCheckConfig(EmptyLineSeparatorCheck.class); + final String[] expected = { + "3: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "package"), + }; + verify(checkConfig, + getPath("package-info/test4/package-info.java"), + expected); + } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java index 679fb6587a9c..f41b85d28ce7 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java @@ -17,7 +17,7 @@ // Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA //////////////////////////////////////////////////////////////////////////////// -package com.puppycrawl.tools.checkstyle.checks.whitespace; +package com.puppycrawl.tools.checkstyle.checks.whitespace; //to test trailing comments import java.io.Serializable; import java.util.ArrayList; import java.util.HashMap; @@ -25,6 +25,7 @@ import java.util.Map; import java.util.concurrent.Callable; import java.util.Collections; +//not separated by empty line and not trailing comment should cause violation import com.oracle.net.Sdp; diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java new file mode 100644 index 000000000000..20da68ffe466 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java @@ -0,0 +1,9 @@ +/*To check warning +that causes by miltiline comment +before package definition. +*/ +package com.puppycrawl.tools.checkstyle.checks.whitespace; + +public class InputEmptyLineSeparatorMultilineCommentHeader +{ +} diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test1/package-info.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test1/package-info.java new file mode 100644 index 000000000000..a6f90778e2f8 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test1/package-info.java @@ -0,0 +1,5 @@ +/*For test allowing to place annotation before PACKAGE_DEF. Should not cause violation.*/ +@Deprecated +package com.puppycrawl.tools.checkstyle.checks.whitespace.test1; + +import java.lang.System; diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test2/package-info.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test2/package-info.java new file mode 100644 index 000000000000..6a02a704ee4c --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test2/package-info.java @@ -0,0 +1,4 @@ +/**For test allowing to place javadoc before PACKAGE_DEF Should not cause violation.*/ +package com.puppycrawl.tools.checkstyle.checks.whitespace.test2; + +import java.lang.System; diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test3/package-info.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test3/package-info.java new file mode 100644 index 000000000000..e95eb9620138 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test3/package-info.java @@ -0,0 +1,5 @@ +/*For test that there's warning when block comment isn't separated from PACKAGE_DEF by line. + Should cause violation after this comment*/ +package com.puppycrawl.tools.checkstyle.checks.whitespace.test3; + +import java.lang.System; diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test4/package-info.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test4/package-info.java new file mode 100644 index 000000000000..c0cfe16ff6ba --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test4/package-info.java @@ -0,0 +1,5 @@ +//for test that there's warning when single line comment isn't separated from PACKAGE_DEF by line +//Should cause violation after this comment +package com.puppycrawl.tools.checkstyle.checks.whitespace.test4; + +import java.lang.System;