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 4, 2016
1 parent 548ecd7 commit 5e5e932
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 21 deletions.
60 changes: 57 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,40 @@ 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) {
final int commentType = comment.getType();
boolean result = false;
if (commentType == TokenTypes.SINGLE_LINE_COMMENT) {
result = comment.getLineNo() == comment.getPreviousSibling().getLineNo();
}
else if (commentType == TokenTypes.BLOCK_COMMENT_BEGIN) {
//check if comment is not multiline and on same line that previous sibling
result = comment.getLineNo() == comment.getLastChild().getLineNo()
&& comment.getLineNo() == comment.getPreviousSibling().getLineNo();
}
return result;
}

/**
* If variable definition is a type field.
* @param variableDef variable definition.
Expand Down
87 changes: 72 additions & 15 deletions ...t/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java
View file
Expand Up @@ -55,14 +55,17 @@ 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, "//"),
"30: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "/*"),
"34: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "/*"),
"38: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"),
"41: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "VARIABLE_DEF"),
"42: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"),
"46: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"),
"60: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"),
"65: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
"82: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
"113: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
};
verify(checkConfig, getPath("InputEmptyLineSeparator.java"), expected);
}
Expand All @@ -75,13 +78,16 @@ 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, "//"),
"30: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "/*"),
"34: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "/*"),
"38: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CLASS_DEF"),
"42: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "STATIC_INIT"),
"46: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INSTANCE_INIT"),
"60: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "CTOR_DEF"),
"65: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
"82: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "METHOD_DEF"),
"113: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
};
verify(checkConfig, getPath("InputEmptyLineSeparator.java"), expected);
}
Expand Down Expand Up @@ -191,4 +197,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);
}
}
9 changes: 6 additions & 3 deletions .../resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparator.java
View file
Expand Up @@ -17,18 +17,21 @@
// 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; //no violation: trailing comment
import java.io.Serializable;
import java.util.ArrayList;
import java.util.ArrayList; /*no violation: trailing comment*/
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.Collections;
//violation: not separated by empty line and not trailing comment

import com.oracle.net.Sdp;
import com.oracle.net.Sdp; /*violation: not separated by empty line
and not trailing comment*/

import javax.swing.AbstractAction;
/*violation: not separated by empty line and not trailing comment*/

import org.apache.commons.beanutils.locale.converters.ByteLocaleConverter;
import org.apache.commons.beanutils.BasicDynaBean;
Expand Down
9 changes: 9 additions & 0 deletions ...awl/tools/checkstyle/checks/whitespace/InputEmptyLineSeparatorMultilineCommentHeader.java
View file
@@ -0,0 +1,9 @@
/*violation: to check warning
that causes by miltiline comment
before package definition.
*/
package com.puppycrawl.tools.checkstyle.checks.whitespace;

public class InputEmptyLineSeparatorMultilineCommentHeader
{
}
7 changes: 7 additions & 0 deletions ...es/com/puppycrawl/tools/checkstyle/checks/whitespace/package-info/test1/package-info.java
View file
@@ -0,0 +1,7 @@
/*OK: for test allowing to place annotation before PACKAGE_DEF.*/
@CheckReturnValue
@ParametersAreNonnullByDefault
package com.puppycrawl.tools.checkstyle.checks.whitespace.packageinfo.test1;

import javax.annotation.CheckReturnValue;
import javax.annotation.ParametersAreNonnullByDefault;
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 @@
/**OK: for test allowing to place javadoc before PACKAGE_DEF.*/
package com.puppycrawl.tools.checkstyle.checks.whitespace.packageinfo.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 @@
/*violation: for test that there's warning when block comment isn't
separated from PACKAGE_DEF by line.*/
package com.puppycrawl.tools.checkstyle.checks.whitespace.packageinfo.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 there's warning when single line comment isn't separated from PACKAGE_DEF by line
//violation is expected after this line
package com.puppycrawl.tools.checkstyle.checks.whitespace.packageinfo.test4;

import java.lang.System;

0 comments on commit 5e5e932

Please sign in to comment.