diff --git a/config/suppressions.xml b/config/suppressions.xml index b0aaec12e168..29b8c88ad941 100644 --- a/config/suppressions.xml +++ b/config/suppressions.xml @@ -121,6 +121,8 @@ + + diff --git a/src/it/java/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/CommentsIndentationTest.java b/src/it/java/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/CommentsIndentationTest.java index 8414be495766..64c211acf806 100644 --- a/src/it/java/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/CommentsIndentationTest.java +++ b/src/it/java/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/CommentsIndentationTest.java @@ -90,7 +90,7 @@ public void testCommentIsAtTheEndOfBlock() throws Exception { "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 void testCommentIsAtTheEndOfBlock() throws Exception { @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", diff --git a/src/it/resources/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/InputCommentsIndentationInSwitchBlock.java b/src/it/resources/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/InputCommentsIndentationInSwitchBlock.java index 3ce959b5d5cf..fca025b72532 100644 --- a/src/it/resources/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/InputCommentsIndentationInSwitchBlock.java +++ b/src/it/resources/com/google/checkstyle/test/chapter4formatting/rule4861blockcommentstyle/InputCommentsIndentationInSwitchBlock.java @@ -16,7 +16,7 @@ private static void fooSwitch() { // comment break; case "3": - /* com */ + /* // warn */ foo1(); /* com */ break; diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java index 90ad4fd8558a..9802aa72342a 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java @@ -109,10 +109,8 @@ public boolean isCommentNodesRequired() { 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 void visitToken(DetailAST commentAst) { } /** - * Checks single line comment indentations over surrounding code, e.g.: + * Checks comment indentations over surrounding code, e.g.: *

* {@code * // some comment - this is ok @@ -130,92 +128,86 @@ public void visitToken(DetailAST commentAst) { * double d1 = 5.0; * } *

- * @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(); + if (previousSibling != null + && 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 (currentToken.getType() == TokenTypes.SINGLE_LINE_COMMENT - || currentToken.getType() == TokenTypes.BLOCK_COMMENT_BEGIN) { + while (isComment(currentToken)) { currentToken = currentToken.getNextSibling(); } } - if (commentPreviousSibling.getLineNo() != currentToken.getLineNo()) { + if (previousSibling.getLineNo() != currentToken.getLineNo()) { isDistributed = true; } } @@ -223,11 +215,9 @@ private static boolean isDistributedMethodChainOrConcatenationStatement( } /** - * 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. + * 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 +233,9 @@ private static boolean isDistributedReturnStatement(DetailAST commentPreviousSib } /** - * 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,13 +251,13 @@ private static boolean isDistributedThrowStatement(DetailAST commentPreviousSibl } /** - * 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(); + final DetailAST previousStatement; if (currentToken.getType() == TokenTypes.LITERAL_RETURN || currentToken.getType() == TokenTypes.LITERAL_THROW) { previousStatement = currentToken; @@ -300,7 +288,7 @@ private static boolean isInEmptyCaseBlock(DetailAST prevStmt, DetailAST nextStmt } /** - * Checks whether single line comment is a 'fall through' comment. + * Checks whether comment is a 'fall through' comment. * For example: *

* {@code @@ -316,9 +304,9 @@ private static boolean isInEmptyCaseBlock(DetailAST prevStmt, DetailAST nextStmt *

* @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 +315,11 @@ private static boolean isFallThroughSingleLineComment(DetailAST prevStmt, Detail } /** - * 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; } @@ -360,7 +348,7 @@ private static boolean isInEmptyCodeBlock(DetailAST prevStmt, DetailAST nextStmt } /** - * 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 +367,8 @@ private static boolean isInEmptyCodeBlock(DetailAST prevStmt, DetailAST nextStmt * @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 +409,16 @@ private void handleSingleLineCommentInEmptyCaseBlock(DetailAST prevStmt, DetailA * @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: *

@@ -443,25 +430,23 @@ private void handleFallThroughtSingleLineComment(DetailAST prevStmt, DetailAST c * } *

* @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 +454,7 @@ else if (!areSameLevelIndented(comment, prevStmt, prevStmt)) { } /** - * 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 +468,38 @@ else if (!areSameLevelIndented(comment, prevStmt, prevStmt)) { * } *

* - * @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 stack = new ArrayDeque<>(); + private DetailAST getOneLinePreviousStatement(DetailAST comment) { DetailAST root = comment.getParent(); + while (root != null && !isBlockStart(root)) { + root = root.getParent(); + } + final Deque 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 +515,37 @@ private static DetailAST getOneLinePreviousStatementOfSingleLineComment(DetailAS } /** - * Finds a previous statement of the single line comment. + * Whether the ast is a comment. + * @param ast the ast to check. + * @return true if the ast is a comment. + */ + 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 single line comment. + * @param comment comment. * @param root root token of the line. - * @return previous statement of the single line comment or null if previous statement was not - * found. + * @return previous statement of the comment or null if previous statement was not found. */ - private static DetailAST findPreviousStatementOfSingleLineComment(DetailAST comment, - DetailAST root) { + 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 +569,8 @@ else if (root.getType() == TokenTypes.PLUS) { tokenWhichBeginsTheLine = root; } if (tokenWhichBeginsTheLine != null - && isOnPreviousLine(comment, tokenWhichBeginsTheLine) - ) { + && !isComment(tokenWhichBeginsTheLine) + && isOnPreviousLineIgnoringComments(comment, tokenWhichBeginsTheLine)) { previousStatement = tokenWhichBeginsTheLine; } return previousStatement; @@ -610,14 +620,52 @@ private static DetailAST findStartTokenOfMethodCallChain(DetailAST root) { } /** - * Checks whether the checked statement is on previous line. + * Checks whether the checked statement is on previous line ignoring comments which separate + * the statements. * @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 comments which separate the statements. + */ + private boolean isOnPreviousLineIgnoringComments(DetailAST currentStatement, + DetailAST checkedStatement) { + DetailAST nextSibling; + if (isBlockStart(checkedStatement)) { + nextSibling = checkedStatement.getFirstChild(); + } + else { + nextSibling = checkedStatement.getNextSibling(); + } + int distanceAim = 1; + while (nextSibling != null && nextSibling != currentStatement && isComment(nextSibling)) { + if (nextSibling.getType() == TokenTypes.SINGLE_LINE_COMMENT) { + distanceAim++; + } + else { + distanceAim += nextSibling.getLastChild().getLineNo() - nextSibling.getLineNo(); + } + nextSibling = nextSibling.getNextSibling(); + } + distanceAim += countEmptyLines(checkedStatement, currentStatement); + return currentStatement.getLineNo() - checkedStatement.getLineNo() == distanceAim; + } + + /** + * 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 static boolean isOnPreviousLine(DetailAST currentStatement, - DetailAST checkedStatement) { - return currentStatement.getLineNo() - checkedStatement.getLineNo() == 1; + private int countEmptyLines(DetailAST startStatement, DetailAST endStatement) { + int emptyLinesNumber = 0; + final String[] lines = getLines(); + final int endLineNo = endStatement.getLineNo(); + for (int i = startStatement.getLineNo(); i < endLineNo; i++) { + if (CommonUtils.isBlank(lines[i])) { + emptyLinesNumber++; + } + } + return emptyLinesNumber; } /** @@ -629,11 +677,27 @@ private static boolean isOnPreviousLine(DetailAST currentStatement, 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 +706,13 @@ private void logMultilineIndentation(DetailAST prevStmt, DetailAST comment, * @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 +727,7 @@ private static DetailAST getPrevStatementWhenCommentIsUnderCase(DetailAST parent 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 +746,9 @@ private static DetailAST getPrevStatementWhenCommentIsUnderCase(DetailAST parent prevStmt = blockBody; } } + if (isComment(prevStmt)) { + prevStmt = prevStmt.getNextSibling(); + } } return prevStmt; } @@ -696,7 +761,7 @@ private static DetailAST getPrevStatementWhenCommentIsUnderCase(DetailAST parent 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 +798,9 @@ private static DetailAST getPrevCaseToken(DetailAST parentStatement) { */ 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 +817,22 @@ private int getLineStart(int lineNo) { 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.: *

@@ -774,31 +849,6 @@ private boolean isTrailingSingleLineComment(DetailAST singleLineComment) { return !CommonUtils.hasWhitespaceBefore(commentColumnNo, targetSourceLine); } - /** - * Checks comment block indentations over surrounding code, e.g.: - *

- * {@code - * /* some comment */ - this is ok - * double d = 3.14; - * /* some comment */ - this is not ok. - * double d1 = 5.0; - * } - *

- * @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.: *

@@ -813,7 +863,8 @@ private void visitBlockComment(DetailAST blockComment) { 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(); } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheckTest.java index 449c2092a467..3e3625461421 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheckTest.java @@ -37,7 +37,7 @@ /** * * @author Aleksey Nesterenko -* @author Aleksey Nesterenko +* @author Andrei Selkin * */ public class CommentsIndentationCheckTest extends BaseCheckTestSupport { @@ -78,11 +78,12 @@ public void testCommentIsAtTheEndOfBlock() throws Exception { "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), + "456: " + getCheckMessage(MSG_KEY_SINGLE, 454, 0, 8), }; final String testInputFile = "InputCommentsIndentationCommentIsAtTheEndOfBlock.java"; verify(checkConfig, getPath(testInputFile), expected); @@ -93,6 +94,7 @@ public void testCommentIsInsideSwitchBlock() throws Exception { 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 +111,8 @@ public void testCommentIsInsideSwitchBlock() throws Exception { "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 +147,9 @@ public void testSurroundingCode() throws Exception { "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 +190,10 @@ public void testCheckOnlyBlockComments() throws Exception { "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); } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationCommentIsAtTheEndOfBlock.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationCommentIsAtTheEndOfBlock.java index 1e769770f9a7..196557b58345 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationCommentIsAtTheEndOfBlock.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationCommentIsAtTheEndOfBlock.java @@ -397,11 +397,65 @@ void foo58() { /* 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(); + +// comment + } + // We almost reached the end of the class here. } // The END of the class. diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationInSwitchBlock.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationInSwitchBlock.java index 69c419cfdc1b..000a4ee8bd2a 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationInSwitchBlock.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationInSwitchBlock.java @@ -16,7 +16,7 @@ private static void fooSwitch() { // comment break; case "3": - /* com */ + /* violation */ foo1(); /* com */ break; @@ -265,4 +265,41 @@ public void foo12() { 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; + } + } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationSurroundingCode.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationSurroundingCode.java index 0a13df6e76ca..3040dc93bb79 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationSurroundingCode.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationSurroundingCode.java @@ -125,5 +125,21 @@ public void foo10() }; } + 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!