Issue #3126: Fix a lot of CommentsIndentationCheck false-positives and false-negatives

This commit is contained in:
Vladislav Lisetskiy 2016-04-26 23:41:31 +03:00 committed by Roman Ivanov
parent 2438c5aca2
commit e5c8a2a884
10 changed files with 553 additions and 178 deletions

View File

@ -121,6 +121,8 @@
<!-- There are a lot of setters/getters in the Check. A small number of methods is left for Check's logic -->
<suppress checks="MethodCount" files="[\\/]JavadocMethodCheck.java$"/>
<!-- Apart from a complex logic there is a lot of small methods for a better readability. -->
<suppress checks="MethodCount" files="[\\/]CommentsIndentationCheck.java$"/>
<!-- getDetails() method - huge Switch, it has to be monolithic -->
<suppress checks="ExecutableStatementCount" files="RightCurlyCheck\.java" lines="313"/>

View File

@ -48,7 +48,7 @@ public class CommentsIndentationTest extends BaseCheckTestSupport {
"47: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
46, 15, 12),
"49: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
48, 10, 8),
45, 10, 8),
"54: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
53, 13, 8),
"74: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
@ -90,7 +90,7 @@ public class CommentsIndentationTest extends BaseCheckTestSupport {
"322: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
323, 0, 4),
"336: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
337, 0, 4),
333, 0, 8),
"355: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
352, 9, 8),
};
@ -106,6 +106,8 @@ public class CommentsIndentationTest extends BaseCheckTestSupport {
@Test
public void testCommentIsInsideSwitchBlock() throws Exception {
final String[] expected = {
"19: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.block",
20, 12, 16),
"25: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
"24, 26", 19, "16, 12"),
"31: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",

View File

@ -16,7 +16,7 @@ public class InputCommentsIndentationInSwitchBlock {
// comment
break;
case "3":
/* com */
/* // warn */
foo1();
/* com */
break;

View File

@ -109,10 +109,8 @@ public class CommentsIndentationCheck extends AbstractCheck {
public void visitToken(DetailAST commentAst) {
switch (commentAst.getType()) {
case TokenTypes.SINGLE_LINE_COMMENT:
visitSingleLineComment(commentAst);
break;
case TokenTypes.BLOCK_COMMENT_BEGIN:
visitBlockComment(commentAst);
visitComment(commentAst);
break;
default:
final String exceptionMsg = "Unexpected token type: " + commentAst.getText();
@ -121,7 +119,7 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Checks single line comment indentations over surrounding code, e.g.:
* Checks comment indentations over surrounding code, e.g.:
* <p>
* {@code
* // some comment - this is ok
@ -130,104 +128,128 @@ public class CommentsIndentationCheck extends AbstractCheck {
* double d1 = 5.0;
* }
* </p>
* @param singleLineComment {@link TokenTypes#SINGLE_LINE_COMMENT single line comment}.
* @param comment comment to check.
*/
private void visitSingleLineComment(DetailAST singleLineComment) {
final DetailAST prevStmt = getPreviousStatementOfSingleLineComment(singleLineComment);
final DetailAST nextStmt = singleLineComment.getNextSibling();
private void visitComment(DetailAST comment) {
final DetailAST prevStmt = getPreviousStatement(comment);
final DetailAST nextStmt = comment.getNextSibling();
if (!isTrailingSingleLineComment(singleLineComment)) {
if (!isTrailingComment(comment)) {
if (isInEmptyCaseBlock(prevStmt, nextStmt)) {
handleSingleLineCommentInEmptyCaseBlock(prevStmt, singleLineComment,
nextStmt);
handleCommentInEmptyCaseBlock(prevStmt, comment, nextStmt);
}
else if (isFallThroughSingleLineComment(prevStmt, nextStmt)) {
handleFallThroughtSingleLineComment(prevStmt, singleLineComment,
nextStmt);
else if (isFallThroughComment(prevStmt, nextStmt)) {
handleFallThroughtComment(prevStmt, comment, nextStmt);
}
else if (isInEmptyCodeBlock(prevStmt, nextStmt)) {
handleSingleLineCommentInEmptyCodeBlock(singleLineComment, nextStmt);
handleCommentInEmptyCodeBlock(comment, nextStmt);
}
else if (isSingleLineCommentAtTheEndOfTheCodeBlock(nextStmt)) {
handleSingleLineCommentAtTheEndOfTheCodeBlock(prevStmt, singleLineComment,
nextStmt);
else if (isCommentAtTheEndOfTheCodeBlock(nextStmt)) {
handleCommentAtTheEndOfTheCodeBlock(prevStmt, comment, nextStmt);
}
else if (nextStmt != null
&& !areSameLevelIndented(singleLineComment, nextStmt, nextStmt)) {
log(singleLineComment.getLineNo(), MSG_KEY_SINGLE, nextStmt.getLineNo(),
singleLineComment.getColumnNo(), nextStmt.getColumnNo());
else if (nextStmt != null && !areSameLevelIndented(comment, nextStmt, nextStmt)) {
log(comment.getLineNo(), getMessageKey(comment), nextStmt.getLineNo(),
comment.getColumnNo(), nextStmt.getColumnNo());
}
}
}
/**
* Returns the previous statement of a single line comment.
* @param comment single line comment.
* @return the previous statement of a single line comment.
* Returns the previous statement of a comment.
* @param comment comment.
* @return the previous statement of a comment.
*/
private static DetailAST getPreviousStatementOfSingleLineComment(DetailAST comment) {
private DetailAST getPreviousStatement(DetailAST comment) {
final DetailAST prevStatement;
if (isDistributedPreviousStatement(comment)) {
prevStatement = getDistributedPreviousStatementOfSingleLineComment(comment);
prevStatement = getDistributedPreviousStatement(comment);
}
else {
prevStatement = getOneLinePreviousStatementOfSingleLineComment(comment);
prevStatement = getOneLinePreviousStatement(comment);
}
return prevStatement;
}
/**
* Checks whether the previous statement of a single line comment is distributed over two or
* more lines.
* @param singleLineComment single line comment.
* @return true if the previous statement of a single line comment is distributed over two or
* more lines.
* Checks whether the previous statement of a comment is distributed over two or more lines.
* @param comment comment to check.
* @return true if the previous statement of a comment is distributed over two or more lines.
*/
private static boolean isDistributedPreviousStatement(DetailAST singleLineComment) {
final DetailAST previousSibling = singleLineComment.getPreviousSibling();
return isDistributedMethodChainOrConcatenationStatement(singleLineComment, previousSibling)
private boolean isDistributedPreviousStatement(DetailAST comment) {
final DetailAST previousSibling = comment.getPreviousSibling();
return isDistributedExpression(comment)
|| isDistributedReturnStatement(previousSibling)
|| isDistributedThrowStatement(previousSibling);
}
/**
* Checks whether the previous statement of a single line comment is a method call chain or
* Checks whether the previous statement of a comment is a method call chain or
* string concatenation statemen distributed over two ore more lines.
* @param comment single line comment.
* @param commentPreviousSibling previous sibling of the sinle line comment.
* @return if the previous statement of a single line comment is a method call chain or
* string concatenation statemen distributed over two ore more lines.
* @param comment comment to check.
* @return true if the previous statement is a distributed expression.
*/
private static boolean isDistributedMethodChainOrConcatenationStatement(
DetailAST comment, DetailAST commentPreviousSibling) {
private boolean isDistributedExpression(DetailAST comment) {
DetailAST previousSibling = comment.getPreviousSibling();
while (previousSibling != null && isComment(previousSibling)) {
previousSibling = previousSibling.getPreviousSibling();
}
boolean isDistributed = false;
if (commentPreviousSibling != null
&& commentPreviousSibling.getType() == TokenTypes.SEMI
&& comment.getLineNo() - commentPreviousSibling.getLineNo() == 1) {
DetailAST currentToken = commentPreviousSibling.getPreviousSibling();
while (currentToken.getFirstChild() != null) {
currentToken = currentToken.getFirstChild();
}
if (currentToken.getType() == TokenTypes.COMMENT_CONTENT) {
currentToken = currentToken.getParent();
while (currentToken.getType() == TokenTypes.SINGLE_LINE_COMMENT
|| currentToken.getType() == TokenTypes.BLOCK_COMMENT_BEGIN) {
currentToken = currentToken.getNextSibling();
if (previousSibling != null) {
if (previousSibling.getType() == TokenTypes.SEMI
&& isOnPreviousLineIgnoringComments(comment, previousSibling)) {
DetailAST currentToken = previousSibling.getPreviousSibling();
while (currentToken.getFirstChild() != null) {
currentToken = currentToken.getFirstChild();
}
if (currentToken.getType() == TokenTypes.COMMENT_CONTENT) {
currentToken = currentToken.getParent();
while (isComment(currentToken)) {
currentToken = currentToken.getNextSibling();
}
}
if (previousSibling.getLineNo() != currentToken.getLineNo()) {
isDistributed = true;
}
}
if (commentPreviousSibling.getLineNo() != currentToken.getLineNo()) {
isDistributed = true;
else {
isDistributed = isStatementWithPossibleCurlies(previousSibling);
}
}
return isDistributed;
}
/**
* Checks whether the previous statement of a single line comment is a destributed return
* statement.
* @param commentPreviousSibling previous sibling of the single line comment.
* @return true if the previous statement of a single line comment is a destributed return
* statement.
* Whether the statement can have or always have curly brackets.
* @param previousSibling the statement to check.
* @return true if the statement can have or always have curly brackets.
*/
private boolean isStatementWithPossibleCurlies(DetailAST previousSibling) {
return previousSibling.getType() == TokenTypes.LITERAL_IF
|| previousSibling.getType() == TokenTypes.LITERAL_TRY
|| previousSibling.getType() == TokenTypes.LITERAL_FOR
|| previousSibling.getType() == TokenTypes.LITERAL_DO
|| previousSibling.getType() == TokenTypes.LITERAL_WHILE
|| previousSibling.getType() == TokenTypes.LITERAL_SWITCH
|| isDefinition(previousSibling);
}
/**
* Whether the statement is a kind of definition (method, class etc.).
* @param previousSibling the statement to check.
* @return true if the statement is a kind of definition.
*/
private boolean isDefinition(DetailAST previousSibling) {
return previousSibling.getType() == TokenTypes.METHOD_DEF
|| previousSibling.getType() == TokenTypes.CLASS_DEF
|| previousSibling.getType() == TokenTypes.INTERFACE_DEF
|| previousSibling.getType() == TokenTypes.ENUM_DEF
|| previousSibling.getType() == TokenTypes.ANNOTATION_DEF;
}
/**
* Checks whether the previous statement of a comment is a destributed return statement.
* @param commentPreviousSibling previous sibling of the comment.
* @return true if the previous statement of a comment is a destributed return statement.
*/
private static boolean isDistributedReturnStatement(DetailAST commentPreviousSibling) {
boolean isDistributed = false;
@ -243,11 +265,9 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Checks whether the previous statement of a single line comment is a destributed throw
* statement.
* @param commentPreviousSibling previous sibling of the single line comment.
* @return true if the previous statement of a single line comment is a destributed throw
* statement.
* Checks whether the previous statement of a comment is a destributed throw statement.
* @param commentPreviousSibling previous sibling of the comment.
* @return true if the previous statement of a comment is a destributed throw statement.
*/
private static boolean isDistributedThrowStatement(DetailAST commentPreviousSibling) {
boolean isDistributed = false;
@ -263,24 +283,26 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Returns the first token of the destributed previous statement of single line comment.
* @param comment single line comment.
* @return the first token of the destributed previous statement of single line comment.
* Returns the first token of the destributed previous statement of comment.
* @param comment comment to check.
* @return the first token of the destributed previous statement of comment.
*/
private static DetailAST getDistributedPreviousStatementOfSingleLineComment(DetailAST comment) {
final DetailAST previousStatement;
private static DetailAST getDistributedPreviousStatement(DetailAST comment) {
DetailAST currentToken = comment.getPreviousSibling();
if (currentToken.getType() == TokenTypes.LITERAL_RETURN
|| currentToken.getType() == TokenTypes.LITERAL_THROW) {
previousStatement = currentToken;
while (isComment(currentToken)) {
currentToken = currentToken.getPreviousSibling();
}
else {
final DetailAST previousStatement;
if (currentToken.getType() == TokenTypes.SEMI) {
currentToken = currentToken.getPreviousSibling();
while (currentToken.getFirstChild() != null) {
currentToken = currentToken.getFirstChild();
}
previousStatement = currentToken;
}
else {
previousStatement = currentToken;
}
return previousStatement;
}
@ -300,7 +322,7 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Checks whether single line comment is a 'fall through' comment.
* Checks whether comment is a 'fall through' comment.
* For example:
* <p>
* {@code
@ -316,9 +338,9 @@ public class CommentsIndentationCheck extends AbstractCheck {
* </p>
* @param prevStmt previous statement.
* @param nextStmt next statement.
* @return true if a single line comment is a 'fall through' comment.
* @return true if a comment is a 'fall through' comment.
*/
private static boolean isFallThroughSingleLineComment(DetailAST prevStmt, DetailAST nextStmt) {
private static boolean isFallThroughComment(DetailAST prevStmt, DetailAST nextStmt) {
return prevStmt != null
&& nextStmt != null
&& prevStmt.getType() != TokenTypes.LITERAL_CASE
@ -327,11 +349,11 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Checks whether a single line comment is placed at the end of the code block.
* Checks whether a comment is placed at the end of the code block.
* @param nextStmt next statement.
* @return true if a single line comment is placed at the end of the block.
* @return true if a comment is placed at the end of the block.
*/
private static boolean isSingleLineCommentAtTheEndOfTheCodeBlock(DetailAST nextStmt) {
private static boolean isCommentAtTheEndOfTheCodeBlock(DetailAST nextStmt) {
return nextStmt != null
&& nextStmt.getType() == TokenTypes.RCURLY;
}
@ -355,12 +377,13 @@ public class CommentsIndentationCheck extends AbstractCheck {
return prevStmt != null
&& nextStmt != null
&& (prevStmt.getType() == TokenTypes.SLIST
|| prevStmt.getType() == TokenTypes.LCURLY
|| prevStmt.getType() == TokenTypes.OBJBLOCK)
&& nextStmt.getType() == TokenTypes.RCURLY;
}
/**
* Handles a single line comment which is plased within empty case block.
* Handles a comment which is plased within empty case block.
* Note, if comment is placed at the end of the empty case block, we have Checkstyle's
* limitations to clearly detect user intention of explanation target - above or below. The
* only case we can assume as a violation is when a single line comment within the empty case
@ -379,8 +402,8 @@ public class CommentsIndentationCheck extends AbstractCheck {
* @param comment single line comment.
* @param nextStmt next statement.
*/
private void handleSingleLineCommentInEmptyCaseBlock(DetailAST prevStmt, DetailAST comment,
DetailAST nextStmt) {
private void handleCommentInEmptyCaseBlock(DetailAST prevStmt, DetailAST comment,
DetailAST nextStmt) {
if (comment.getColumnNo() < prevStmt.getColumnNo()
|| comment.getColumnNo() < nextStmt.getColumnNo()) {
@ -421,17 +444,16 @@ public class CommentsIndentationCheck extends AbstractCheck {
* @param comment single line comment.
* @param nextStmt next statement.
*/
private void handleFallThroughtSingleLineComment(DetailAST prevStmt, DetailAST comment,
DetailAST nextStmt) {
private void handleFallThroughtComment(DetailAST prevStmt, DetailAST comment,
DetailAST nextStmt) {
if (!areSameLevelIndented(comment, prevStmt, nextStmt)) {
logMultilineIndentation(prevStmt, comment, nextStmt);
}
}
/**
* Hendles a single line comment which is placed at the end of non empty code block.
* Handles a comment which is placed at the end of non empty code block.
* Note, if single line comment is plcaed at the end of non empty block the comment should have
* the same indentation level as the previous statement. For example:
* <p>
@ -443,25 +465,23 @@ public class CommentsIndentationCheck extends AbstractCheck {
* }
* </p>
* @param prevStmt previous statement.
* @param comment single line statement.
* @param comment comment to check.
* @param nextStmt next statement.
*/
private void handleSingleLineCommentAtTheEndOfTheCodeBlock(DetailAST prevStmt,
DetailAST comment,
DetailAST nextStmt) {
private void handleCommentAtTheEndOfTheCodeBlock(DetailAST prevStmt, DetailAST comment,
DetailAST nextStmt) {
if (prevStmt != null) {
if (prevStmt.getType() == TokenTypes.LITERAL_CASE
|| prevStmt.getType() == TokenTypes.CASE_GROUP
|| prevStmt.getType() == TokenTypes.LITERAL_DEFAULT
|| prevStmt.getType() == TokenTypes.SINGLE_LINE_COMMENT) {
|| prevStmt.getType() == TokenTypes.LITERAL_DEFAULT) {
if (comment.getColumnNo() < nextStmt.getColumnNo()) {
log(comment.getLineNo(), MSG_KEY_SINGLE, nextStmt.getLineNo(),
log(comment.getLineNo(), getMessageKey(comment), nextStmt.getLineNo(),
comment.getColumnNo(), nextStmt.getColumnNo());
}
}
else if (!areSameLevelIndented(comment, prevStmt, prevStmt)) {
final int prevStmtLineNo = prevStmt.getLineNo();
log(comment.getLineNo(), MSG_KEY_SINGLE, prevStmtLineNo,
log(comment.getLineNo(), getMessageKey(comment), prevStmtLineNo,
comment.getColumnNo(), getLineStart(prevStmtLineNo));
}
}
@ -469,7 +489,7 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Handles a single line comment which is placed within the empty code block.
* Handles a comment which is placed within the empty code block.
* Note, if comment is placed at the end of the empty code block, we have Checkstyle's
* limitations to clearly detect user intention of explanation target - above or below. The
* only case we can assume as a violation is when a single line comment within the empty
@ -483,35 +503,38 @@ public class CommentsIndentationCheck extends AbstractCheck {
* }
* </p>
*
* @param comment single line comment.
* @param comment comment to check.
* @param nextStmt next statement.
*/
private void handleSingleLineCommentInEmptyCodeBlock(DetailAST comment, DetailAST nextStmt) {
private void handleCommentInEmptyCodeBlock(DetailAST comment, DetailAST nextStmt) {
if (comment.getColumnNo() < nextStmt.getColumnNo()) {
log(comment.getLineNo(), MSG_KEY_SINGLE, nextStmt.getLineNo(),
log(comment.getLineNo(), getMessageKey(comment), nextStmt.getLineNo(),
comment.getColumnNo(), nextStmt.getColumnNo());
}
}
/**
* Does pre-order traverse of abstract syntax tree to find the previous statement of the
* single line comment. If previous statement of the comment is found, then the traverse will
* comment. If previous statement of the comment is found, then the traverse will
* be finished.
* @param comment current statement.
* @return previous statement of the comment or null if the comment does not have previous
* statement.
*/
private static DetailAST getOneLinePreviousStatementOfSingleLineComment(DetailAST comment) {
DetailAST previousStatement = null;
final Deque<DetailAST> stack = new ArrayDeque<>();
private DetailAST getOneLinePreviousStatement(DetailAST comment) {
DetailAST root = comment.getParent();
while (root != null && !isBlockStart(root)) {
root = root.getParent();
}
final Deque<DetailAST> stack = new ArrayDeque<>();
DetailAST previousStatement = null;
while (root != null || !stack.isEmpty()) {
if (!stack.isEmpty()) {
root = stack.pop();
}
while (root != null) {
previousStatement = findPreviousStatementOfSingleLineComment(comment, root);
previousStatement = findPreviousStatement(comment, root);
if (previousStatement != null) {
root = null;
stack.clear();
@ -527,15 +550,37 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Finds a previous statement of the single line comment.
* Uses root token of the line while searching.
* @param comment single line comment.
* @param root root token of the line.
* @return previous statement of the single line comment or null if previous statement was not
* found.
* Whether the ast is a comment.
* @param ast the ast to check.
* @return true if the ast is a comment.
*/
private static DetailAST findPreviousStatementOfSingleLineComment(DetailAST comment,
DetailAST root) {
private static boolean isComment(DetailAST ast) {
final int astType = ast.getType();
return astType == TokenTypes.SINGLE_LINE_COMMENT
|| astType == TokenTypes.BLOCK_COMMENT_BEGIN
|| astType == TokenTypes.COMMENT_CONTENT
|| astType == TokenTypes.BLOCK_COMMENT_END;
}
/**
* Whether the AST node starts a block.
* @param root the AST node to check.
* @return true if the AST node starts a block.
*/
private static boolean isBlockStart(DetailAST root) {
return root.getType() == TokenTypes.SLIST
|| root.getType() == TokenTypes.OBJBLOCK
|| root.getType() == TokenTypes.CASE_GROUP;
}
/**
* Finds a previous statement of the comment.
* Uses root token of the line while searching.
* @param comment comment.
* @param root root token of the line.
* @return previous statement of the comment or null if previous statement was not found.
*/
private DetailAST findPreviousStatement(DetailAST comment, DetailAST root) {
DetailAST previousStatement = null;
if (root.getLineNo() >= comment.getLineNo()) {
// ATTENTION: parent of the comment is below the comment in case block
@ -559,8 +604,8 @@ public class CommentsIndentationCheck extends AbstractCheck {
tokenWhichBeginsTheLine = root;
}
if (tokenWhichBeginsTheLine != null
&& isOnPreviousLine(comment, tokenWhichBeginsTheLine)
) {
&& !isComment(tokenWhichBeginsTheLine)
&& isOnPreviousLineIgnoringComments(comment, tokenWhichBeginsTheLine)) {
previousStatement = tokenWhichBeginsTheLine;
}
return previousStatement;
@ -610,14 +655,67 @@ public class CommentsIndentationCheck extends AbstractCheck {
}
/**
* Checks whether the checked statement is on previous line.
* Checks whether the checked statement is on the previous line ignoring empty lines
* and lines which contain only comments.
* @param currentStatement current statement.
* @param checkedStatement checked statement.
* @return true if checked statement is on the line which is previous to current statement.
* @return true if checked statement is on the line which is previous to current statement
* ignoring empty lines and lines which contain only comments.
*/
private static boolean isOnPreviousLine(DetailAST currentStatement,
DetailAST checkedStatement) {
return currentStatement.getLineNo() - checkedStatement.getLineNo() == 1;
private boolean isOnPreviousLineIgnoringComments(DetailAST currentStatement,
DetailAST checkedStatement) {
DetailAST nextToken = getNextToken(checkedStatement);
int distanceAim = 1;
if (nextToken != null && isComment(nextToken)) {
distanceAim += countEmptyLines(checkedStatement, currentStatement);
}
while (nextToken != null && nextToken != currentStatement && isComment(nextToken)) {
if (nextToken.getType() == TokenTypes.BLOCK_COMMENT_BEGIN) {
distanceAim += nextToken.getLastChild().getLineNo() - nextToken.getLineNo();
}
distanceAim++;
nextToken = nextToken.getNextSibling();
}
return currentStatement.getLineNo() - checkedStatement.getLineNo() == distanceAim;
}
/**
* Get the token to start counting the number of lines to add to the distance aim from.
* @param checkedStatement the checked statement.
* @return the token to start counting the number of lines to add to the distance aim from.
*/
private DetailAST getNextToken(DetailAST checkedStatement) {
DetailAST nextToken;
if (checkedStatement.getType() == TokenTypes.SLIST
|| checkedStatement.getType() == TokenTypes.CASE_GROUP) {
nextToken = checkedStatement.getFirstChild();
}
else {
nextToken = checkedStatement.getNextSibling();
}
if (nextToken != null && isComment(nextToken) && isTrailingComment(nextToken)) {
nextToken = nextToken.getNextSibling();
}
return nextToken;
}
/**
* Count the number of empty lines between statements.
* @param startStatement start statement.
* @param endStatement end statement.
* @return the number of empty lines between statements.
*/
private int countEmptyLines(DetailAST startStatement, DetailAST endStatement) {
int emptyLinesNumber = 0;
final String[] lines = getLines();
final int endLineNo = endStatement.getLineNo();
for (int lineNo = startStatement.getLineNo(); lineNo < endLineNo; lineNo++) {
if (CommonUtils.isBlank(lines[lineNo])) {
emptyLinesNumber++;
}
}
return emptyLinesNumber;
}
/**
@ -629,11 +727,27 @@ public class CommentsIndentationCheck extends AbstractCheck {
private void logMultilineIndentation(DetailAST prevStmt, DetailAST comment,
DetailAST nextStmt) {
final String multilineNoTemplate = "%d, %d";
log(comment.getLineNo(), MSG_KEY_SINGLE,
log(comment.getLineNo(), getMessageKey(comment),
String.format(Locale.getDefault(), multilineNoTemplate, prevStmt.getLineNo(),
nextStmt.getLineNo()), comment.getColumnNo(),
String.format(Locale.getDefault(), multilineNoTemplate, prevStmt.getColumnNo(),
nextStmt.getColumnNo()));
String.format(Locale.getDefault(), multilineNoTemplate,
getLineStart(prevStmt.getLineNo()), getLineStart(nextStmt.getLineNo())));
}
/**
* Get a message key depending on a comment type.
* @param comment the comment to process.
* @return a message key.
*/
private String getMessageKey(DetailAST comment) {
final String msgKey;
if (comment.getType() == TokenTypes.SINGLE_LINE_COMMENT) {
msgKey = MSG_KEY_SINGLE;
}
else {
msgKey = MSG_KEY_BLOCK;
}
return msgKey;
}
/**
@ -642,15 +756,13 @@ public class CommentsIndentationCheck extends AbstractCheck {
* @return comment's previous statement or null if previous statement is absent.
*/
private static DetailAST getPrevStatementFromSwitchBlock(DetailAST comment) {
DetailAST prevStmt = null;
final DetailAST prevStmt;
final DetailAST parentStatement = comment.getParent();
if (parentStatement != null) {
if (parentStatement.getType() == TokenTypes.CASE_GROUP) {
prevStmt = getPrevStatementWhenCommentIsUnderCase(parentStatement);
}
else {
prevStmt = getPrevCaseToken(parentStatement);
}
if (parentStatement.getType() == TokenTypes.CASE_GROUP) {
prevStmt = getPrevStatementWhenCommentIsUnderCase(parentStatement);
}
else {
prevStmt = getPrevCaseToken(parentStatement);
}
return prevStmt;
}
@ -665,7 +777,7 @@ public class CommentsIndentationCheck extends AbstractCheck {
final DetailAST prevBlock = parentStatement.getPreviousSibling();
if (prevBlock.getLastChild() != null) {
DetailAST blockBody = prevBlock.getLastChild().getLastChild();
if (blockBody.getPreviousSibling() != null) {
if (blockBody.getType() == TokenTypes.SEMI) {
blockBody = blockBody.getPreviousSibling();
}
if (blockBody.getType() == TokenTypes.EXPR) {
@ -684,6 +796,9 @@ public class CommentsIndentationCheck extends AbstractCheck {
prevStmt = blockBody;
}
}
if (isComment(prevStmt)) {
prevStmt = prevStmt.getNextSibling();
}
}
return prevStmt;
}
@ -696,7 +811,7 @@ public class CommentsIndentationCheck extends AbstractCheck {
private static DetailAST getPrevCaseToken(DetailAST parentStatement) {
final DetailAST prevCaseToken;
final DetailAST parentBlock = parentStatement.getParent();
if (parentBlock != null && parentBlock.getParent() != null
if (parentBlock.getParent() != null
&& parentBlock.getParent().getPreviousSibling() != null
&& parentBlock.getParent().getPreviousSibling().getType()
== TokenTypes.LITERAL_CASE) {
@ -733,15 +848,9 @@ public class CommentsIndentationCheck extends AbstractCheck {
*/
private boolean areSameLevelIndented(DetailAST comment, DetailAST prevStmt,
DetailAST nextStmt) {
final boolean result;
if (prevStmt == null) {
result = comment.getColumnNo() == getLineStart(nextStmt.getLineNo());
}
else {
result = comment.getColumnNo() == getLineStart(nextStmt.getLineNo())
|| comment.getColumnNo() == getLineStart(prevStmt.getLineNo());
}
return result;
return comment.getColumnNo() == getLineStart(nextStmt.getLineNo())
|| comment.getColumnNo() == getLineStart(prevStmt.getLineNo());
}
/**
@ -758,6 +867,22 @@ public class CommentsIndentationCheck extends AbstractCheck {
return lineStart;
}
/**
* Checks if current comment is a trailing comment.
* @param comment comment to check.
* @return true if current comment is a trailing comment.
*/
private boolean isTrailingComment(DetailAST comment) {
final boolean isTrailingComment;
if (comment.getType() == TokenTypes.SINGLE_LINE_COMMENT) {
isTrailingComment = isTrailingSingleLineComment(comment);
}
else {
isTrailingComment = isTrailingBlockComment(comment);
}
return isTrailingComment;
}
/**
* Checks if current single line comment is trailing comment, e.g.:
* <p>
@ -774,31 +899,6 @@ public class CommentsIndentationCheck extends AbstractCheck {
return !CommonUtils.hasWhitespaceBefore(commentColumnNo, targetSourceLine);
}
/**
* Checks comment block indentations over surrounding code, e.g.:
* <p>
* {@code
* /* some comment *&#47; - this is ok
* double d = 3.14;
* /* some comment *&#47; - this is <b>not</b> ok.
* double d1 = 5.0;
* }
* </p>
* @param blockComment {@link TokenTypes#BLOCK_COMMENT_BEGIN block comment begin}.
*/
private void visitBlockComment(DetailAST blockComment) {
final DetailAST nextStatement = blockComment.getNextSibling();
final DetailAST prevStatement = getPrevStatementFromSwitchBlock(blockComment);
if (nextStatement != null
&& nextStatement.getType() != TokenTypes.RCURLY
&& !isTrailingBlockComment(blockComment)
&& !areSameLevelIndented(blockComment, prevStatement, nextStatement)) {
log(blockComment.getLineNo(), MSG_KEY_BLOCK, nextStatement.getLineNo(),
blockComment.getColumnNo(), nextStatement.getColumnNo());
}
}
/**
* Checks if current comment block is trailing comment, e.g.:
* <p>
@ -813,7 +913,8 @@ public class CommentsIndentationCheck extends AbstractCheck {
private boolean isTrailingBlockComment(DetailAST blockComment) {
final String commentLine = getLine(blockComment.getLineNo() - 1);
final int commentColumnNo = blockComment.getColumnNo();
final DetailAST nextSibling = blockComment.getNextSibling();
return !CommonUtils.hasWhitespaceBefore(commentColumnNo, commentLine)
|| blockComment.getNextSibling().getLineNo() == blockComment.getLineNo();
|| nextSibling != null && nextSibling.getLineNo() == blockComment.getLineNo();
}
}

View File

@ -37,7 +37,7 @@ import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
/**
*
* @author <a href="mailto:nesterenko-aleksey@list.ru">Aleksey Nesterenko</a>
* @author <a href="mailto:andreyselkin@gmail.com">Aleksey Nesterenko</a>
* @author <a href="mailto:andreyselkin@gmail.com">Andrei Selkin</a>
*
*/
public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
@ -57,7 +57,7 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
"33: " + getCheckMessage(MSG_KEY_SINGLE, 35, 5, 4),
"37: " + getCheckMessage(MSG_KEY_SINGLE, 36, 0, 8),
"47: " + getCheckMessage(MSG_KEY_SINGLE, 46, 15, 12),
"49: " + getCheckMessage(MSG_KEY_SINGLE, 48, 10, 8),
"49: " + getCheckMessage(MSG_KEY_SINGLE, 45, 10, 8),
"54: " + getCheckMessage(MSG_KEY_SINGLE, 53, 13, 8),
"74: " + getCheckMessage(MSG_KEY_SINGLE, 70, 18, 8),
"88: " + getCheckMessage(MSG_KEY_SINGLE, 85, 31, 8),
@ -78,11 +78,24 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
"277: " + getCheckMessage(MSG_KEY_SINGLE, 276, 9, 8),
"316: " + getCheckMessage(MSG_KEY_SINGLE, 315, 9, 8),
"322: " + getCheckMessage(MSG_KEY_SINGLE, 323, 0, 4),
"336: " + getCheckMessage(MSG_KEY_SINGLE, 337, 0, 4),
"336: " + getCheckMessage(MSG_KEY_SINGLE, 333, 0, 8),
"355: " + getCheckMessage(MSG_KEY_SINGLE, 352, 9, 8),
"380: " + getCheckMessage(MSG_KEY_BLOCK, 381, 12, 8),
"393: " + getCheckMessage(MSG_KEY_SINGLE, 392, 12, 8),
"400: " + getCheckMessage(MSG_KEY_SINGLE, 401, 8, 10),
"457: " + getCheckMessage(MSG_KEY_SINGLE, 455, 0, 8),
"473: " + getCheckMessage(MSG_KEY_BLOCK, 469, 10, 8),
"483: " + getCheckMessage(MSG_KEY_BLOCK, 477, 10, 8),
"491: " + getCheckMessage(MSG_KEY_BLOCK, 487, 10, 8),
"499: " + getCheckMessage(MSG_KEY_BLOCK, 495, 10, 8),
"507: " + getCheckMessage(MSG_KEY_BLOCK, 503, 10, 8),
"518: " + getCheckMessage(MSG_KEY_SINGLE, 511, 10, 8),
"525: " + getCheckMessage(MSG_KEY_SINGLE, 522, 0, 8),
"532: " + getCheckMessage(MSG_KEY_SINGLE, 529, 0, 8),
"538: " + getCheckMessage(MSG_KEY_SINGLE, 536, 0, 8),
"546: " + getCheckMessage(MSG_KEY_SINGLE, 542, 4, 8),
"551: " + getCheckMessage(MSG_KEY_SINGLE, 550, 12, 8),
"557: " + getCheckMessage(MSG_KEY_SINGLE, 555, 0, 8),
};
final String testInputFile = "InputCommentsIndentationCommentIsAtTheEndOfBlock.java";
verify(checkConfig, getPath(testInputFile), expected);
@ -93,6 +106,7 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
final DefaultConfiguration checkConfig =
createCheckConfig(CommentsIndentationCheck.class);
final String[] expected = {
"19: " + getCheckMessage(MSG_KEY_BLOCK, 20, 12, 16),
"25: " + getCheckMessage(MSG_KEY_SINGLE, "24, 26", 19, "16, 12"),
"31: " + getCheckMessage(MSG_KEY_SINGLE, "30, 32", 19, "16, 12"),
"48: " + getCheckMessage(MSG_KEY_SINGLE, 49, 6, 16),
@ -109,6 +123,8 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
"204: " + getCheckMessage(MSG_KEY_SINGLE, 205, 20, 17),
"205: " + getCheckMessage(MSG_KEY_SINGLE, "202, 206", 17, "16, 12"),
"229: " + getCheckMessage(MSG_KEY_SINGLE, "228, 230", 6, "12, 12"),
"276: " + getCheckMessage(MSG_KEY_BLOCK, "275, 279", 11, "16, 12"),
"281: " + getCheckMessage(MSG_KEY_SINGLE, "280, 282", 11, "16, 12"),
};
final String testInputFile = "InputCommentsIndentationInSwitchBlock.java";
verify(checkConfig, getPath(testInputFile), expected);
@ -143,6 +159,9 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
"90: " + getCheckMessage(MSG_KEY_SINGLE, 91, 14, 8),
"98: " + getCheckMessage(MSG_KEY_SINGLE, 99, 13, 8),
"108: " + getCheckMessage(MSG_KEY_SINGLE, 109, 33, 8),
"130: " + getCheckMessage(MSG_KEY_BLOCK, 131, 12, 8),
"135: " + getCheckMessage(MSG_KEY_BLOCK, 136, 4, 8),
"141: " + getCheckMessage(MSG_KEY_BLOCK, 140, 4, 8),
};
final String testInputFile = "InputCommentsIndentationSurroundingCode.java";
verify(checkConfig, getPath(testInputFile), expected);
@ -183,7 +202,10 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
"25: " + getCheckMessage(MSG_KEY_BLOCK, 27, 16, 12),
"28: " + getCheckMessage(MSG_KEY_BLOCK, 31, 16, 12),
"51: " + getCheckMessage(MSG_KEY_BLOCK, 53, 23, 36),
};
"130: " + getCheckMessage(MSG_KEY_BLOCK, 131, 12, 8),
"135: " + getCheckMessage(MSG_KEY_BLOCK, 136, 4, 8),
"141: " + getCheckMessage(MSG_KEY_BLOCK, 140, 4, 8),
};
final String testInputFile = "InputCommentsIndentationSurroundingCode.java";
verify(checkConfig, getPath(testInputFile), expected);
}
@ -203,4 +225,18 @@ public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
Assert.assertEquals("Unexpected token type: methodStub", msg);
}
}
@Test
public void testJavadoc() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(CommentsIndentationCheck.class);
final String[] expected = {
"3: " + getCheckMessage(MSG_KEY_BLOCK, 6, 2, 0),
"8: " + getCheckMessage(MSG_KEY_BLOCK, 9, 0, 4),
"11: " + getCheckMessage(MSG_KEY_BLOCK, 14, 8, 4),
"17: " + getCheckMessage(MSG_KEY_BLOCK, 18, 10, 8),
};
final String testInputFile = "InputCommentsIndentationJavadoc.java";
verify(checkConfig, getPath(testInputFile), expected);
}
}

View File

@ -397,11 +397,166 @@ public class InputCommentsIndentationCommentIsAtTheEndOfBlock {
/*
comment
*/
// comment
// violation
foo1();
// comment
}
void foo59() {
foo1();
/*
comment */
// comment
}
void foo61() {
foo1();
/*
* comment
*/
/*
* comment
*/
}
void foo62() {
if (true) {
System.out.println();
}
else {
}
/*
comment
*/
/*
comment
*/
}
void foo63() {
try {
System.out.println();
}
catch (Exception e){
}
/*
comment
*/
/*
comment
*/
}
void foo64() {
foo1();
// violation
}
void foo65() {
int i = 1
+ 1
+ 1;
// comment
// comment
}
void foo66() {
if (true) {
getClass();
}
/* violation */
}
void foo67() {
try {
getClass();
} finally {
hashCode();
}
/* violation */
}
void foo68() {
for (int i = 0; i < 0; i++) {
getClass();
}
/* violation */
}
void foo69() {
while (true) {
getClass();
}
/* violation */
}
void foo70() {
do {
getClass();
} while (true);
/* violation */
}
void foo71() {
switch("") {
case "!":
break;
default:
break;
}
// violation
}
void foo72() {
int u = 1;
/* comment */
// violation
}
void foo73() {
class Foo { }
/* comment */
// violation
}
interface Bar1 {
interface NestedBar { }
// violation
}
static class Bar2 {
enum Foo {
A;
}
// violation
}
static class Bar3 {
@interface Foo { }
// violation
}
void foo74() {
getClass(); // comment
// comment
// comment
}
// We almost reached the end of the class here.
}
// The END of the class.

View File

@ -83,4 +83,9 @@ public class InputCommentsIndentationInEmptyBlock {
private static class MyClass extends Object {
// no members
}
private static class MyClass1 extends Object {
// no members
}
}

View File

@ -16,7 +16,7 @@ public class InputCommentsIndentationInSwitchBlock {
// comment
break;
case "3":
/* com */
/* violation */
foo1();
/* com */
break;
@ -265,4 +265,41 @@ public class InputCommentsIndentationInSwitchBlock {
case 1:
}
}
public void foo13() {
int a = 5;
switch (a) {
case 1:
/* comment */
case 2:
hashCode();
/*
violation
*/
case 3: // comment
hashCode();
// violation
case 4: // comment
if (true) {
}
else {
}
// comment
case 5:
String s = ""
+ 1
+ "123";
break;
// comment
case 6:
String q = ""
+ 1
+ "123";
// comment
case 7:
break;
}
}
}

View File

@ -0,0 +1,21 @@
package com.puppycrawl.tools.checkstyle.checks.indentation;
/**
* violation
*/
public class InputCommentsIndentationJavadoc {
/** violation */
int i;
/**
* violation
*/
void foo() {}
enum Bar {
/** violation */
A;
}
}

View File

@ -125,5 +125,21 @@ public class InputCommentsIndentationSurroundingCode
};
}
public void foo11() {
/* empty */
hashCode();
}
public void foo12() {
/* empty */
hashCode();
}
public void foo13() {
hashCode();
/* empty */
}
} // The Check should not throw NPE here!
// The Check should not throw NPE here!