Skip to content

Commit

Permalink
Issue #5981: add validation of new lines before comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@kazachka @romani
kazachka authored and romani committed Apr 27, 2019
1 parent 3767e42 commit 8c37431
Show file tree
Hide file tree
Showing 4 changed files with 912 additions and 0 deletions.
75 changes: 75 additions & 0 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package com.puppycrawl.tools.checkstyle.checks.whitespace;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;

import com.puppycrawl.tools.checkstyle.StatelessCheck;
Expand Down Expand Up @@ -218,6 +220,11 @@ public class EmptyLineSeparatorCheck extends AbstractCheck {
public static final String MSG_MULTIPLE_LINES_INSIDE =
"empty.line.separator.multiple.lines.inside";

/** List of AST token types, which can not have comment nodes to check inside. */
private static final List<Integer> TOKEN_TYPES_WITHOUT_COMMENTS_TO_CHECK_INSIDE =
Arrays.asList(TokenTypes.PACKAGE_DEF, TokenTypes.IMPORT, TokenTypes.STATIC_IMPORT,
TokenTypes.STATIC_INIT);

/** Allows no empty line between fields. */
private boolean allowNoEmptyLineBetweenFields;

Expand Down Expand Up @@ -286,6 +293,7 @@ public int[] getRequiredTokens() {

@Override
public void visitToken(DetailAST ast) {
checkComments(ast);
if (hasMultipleLinesBefore(ast)) {
log(ast.getLineNo(), MSG_MULTIPLE_LINES, ast.getText());
}
Expand Down Expand Up @@ -479,6 +487,57 @@ private boolean hasNotAllowedTwoEmptyLinesBefore(DetailAST token) {
&& isPrePreviousLineEmpty(token);
}

/**
* Check if group of comments located right before token has more than one previous empty line.
* @param token DetailAST token
*/
private void checkComments(DetailAST token) {
if (!allowMultipleEmptyLines) {
if (TOKEN_TYPES_WITHOUT_COMMENTS_TO_CHECK_INSIDE.contains(token.getType())) {
DetailAST previousNode = token.getPreviousSibling();
while (isCommentInBeginningOfLine(previousNode)) {
if (hasEmptyLineBefore(previousNode) && isPrePreviousLineEmpty(previousNode)) {
log(previousNode, MSG_MULTIPLE_LINES, previousNode.getText());
}
previousNode = previousNode.getPreviousSibling();
}
}
else {
checkCommentsInsideToken(token);
}
}
}

/**
* Check if group of comments located at the start of token has more than one previous empty
* line.
* @param token DetailAST token
*/
private void checkCommentsInsideToken(DetailAST token) {
final List<DetailAST> childNodes = new LinkedList<>();
DetailAST childNode = token.getLastChild();
while (childNode != null) {
if (childNode.getType() == TokenTypes.MODIFIERS) {
for (DetailAST node = token.getFirstChild().getLastChild();
node != null;
node = node.getPreviousSibling()) {
if (isCommentInBeginningOfLine(node)) {
childNodes.add(node);
}
}
}
else if (isCommentInBeginningOfLine(childNode)) {
childNodes.add(childNode);
}
childNode = childNode.getPreviousSibling();
}
for (DetailAST node : childNodes) {
if (hasEmptyLineBefore(node) && isPrePreviousLineEmpty(node)) {
log(node, MSG_MULTIPLE_LINES, node.getText());
}
}
}

/**
* Checks if a token has empty pre-previous line.
* @param token DetailAST token.
Expand Down Expand Up @@ -555,6 +614,22 @@ private boolean hasEmptyLineBefore(DetailAST token) {
return result;
}

/**
* Check if token is comment, which starting in beginning of line.
* @param comment comment token for check.
* @return true, if token is comment, which starting in beginning of line.
*/
private boolean isCommentInBeginningOfLine(DetailAST comment) {
// [comment.getLineNo() - 1] is the number of the previous line as the numbering starts
// from zero.
boolean result = false;
if (comment != null) {
final String lineWithComment = getLines()[comment.getLineNo() - 1].trim();
result = lineWithComment.startsWith("//") || lineWithComment.startsWith("/*");
}
return result;
}

/**
* Check if token is preceded by javadoc comment.
* @param token token for check.
Expand Down
66 changes: 66 additions & 0 deletions ...t/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,4 +300,70 @@ public void testClassOnly() throws Exception {
expected);
}

@Test
public void testLineSeparationBeforeComments() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(EmptyLineSeparatorCheck.class);
checkConfig.addAttribute("allowMultipleEmptyLines", "false");
final String[] expected = {
"19: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "package"),
"23:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"27: " + getCheckMessage(MSG_MULTIPLE_LINES, "import"),
"32:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"39:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"50:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"67:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"78:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"83: " + getCheckMessage(MSG_MULTIPLE_LINES, "import"),
"89:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"93:1: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"101: " + getCheckMessage(MSG_MULTIPLE_LINES, "VARIABLE_DEF"),
"106:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"113:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"126:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"139: " + getCheckMessage(MSG_MULTIPLE_LINES, "METHOD_DEF"),
"146:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"156:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"171:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"188: " + getCheckMessage(MSG_MULTIPLE_LINES, "CLASS_DEF"),
"194:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"198:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"204:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"216:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"229:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"243:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"246: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
"251:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "/*"),
"266:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"275:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"288:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"293:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"299:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"307:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"316:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"322:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"342:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
"350:5: " + getCheckMessage(MSG_MULTIPLE_LINES, "//"),
};
verify(checkConfig, getPath("InputEmptyLineSeparatorWithComments.java"), expected);
}

@Test
public void testIgnoreEmptyLinesBeforeCommentsWhenItIsAllowed() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(EmptyLineSeparatorCheck.class);
final String[] expected = {
"19: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "package"),
"246: " + getCheckMessage(MSG_SHOULD_BE_SEPARATED, "INTERFACE_DEF"),
};
verify(checkConfig, getPath("InputEmptyLineSeparatorWithComments.java"), expected);
}

@Test
public void testNoViolationsOnEmptyLinesBeforeComments() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(EmptyLineSeparatorCheck.class);
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verify(checkConfig,
getPath("InputEmptyLineSeparatorNoViolationOnEmptyLineBeforeComments.java"),
expected);
}

}
Loading

0 comments on commit 8c37431

Please sign in to comment.