Skip to content

Commit

Permalink
Issue #3426: remove warning on PACKAGE_DEF preceded by javadoc not se…
Browse files Browse the repository at this point in the history 
…parated by line
  • Loading branch information
@kazachka
kazachka committed Dec 2, 2016
1 parent 548ecd7 commit 831949a
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 19 deletions.
51 changes: 48 additions & 3 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java
View file
Expand Up @@ -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,
Expand Down Expand Up @@ -250,6 +251,11 @@ public void setAllowMultipleEmptyLinesInsideClassMembers(boolean allow) {
allowMultipleEmptyLinesInsideClassMembers = allow;
}

@Override
public boolean isCommentNodesRequired() {
return true;
}

@Override
public int[] getDefaultTokens() {
return getAcceptableTokens();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
83 changes: 68 additions & 15 deletions ...t/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java
View file
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
}
3 changes: 2 additions & 1 deletion .../resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java
View file
Expand Up @@ -17,14 +17,15 @@
// 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;
import java.util.List;
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;

Expand Down
9 changes: 9 additions & 0 deletions ...awl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java
View file
@@ -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
{
}
5 changes: 5 additions & 0 deletions ...es/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test1/package-info.java
View file
@@ -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;
4 changes: 4 additions & 0 deletions ...es/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test2/package-info.java
View file
@@ -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;
5 changes: 5 additions & 0 deletions ...es/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test3/package-info.java
View file
@@ -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;
5 changes: 5 additions & 0 deletions ...es/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test4/package-info.java
View file
@@ -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;

0 comments on commit 831949a

Please sign in to comment.