From e545ce56a19877ef98f367b3aa20fd6098bbeca9 Mon Sep 17 00:00:00 2001 From: liscju Date: Mon, 25 Jan 2016 12:09:33 +0100 Subject: [PATCH] Issue #2838: Make WhitespaceAround allow double-brace initialization. --- .../whitespace/WhitespaceAroundCheck.java | 195 +++++++++++++----- .../whitespace/WhitespaceAroundCheckTest.java | 14 ++ .../InputDoubleBraceInitialization.java | 19 ++ src/xdocs/config_whitespace.xml | 6 + 4 files changed, 178 insertions(+), 56 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputDoubleBraceInitialization.java diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheck.java index 1dae34e91..adf8d5db5 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheck.java @@ -93,7 +93,18 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * 
  * <module name="WhitespaceAround">
  *     <property name="tokens"
- *               value="ASSIGN,DIV_ASSIGN,PLUS_ASSIGN,MINUS_ASSIGN,STAR_ASSIGN,MOD_ASSIGN,SR_ASSIGN,BSR_ASSIGN,SL_ASSIGN,BXOR_ASSIGN,BOR_ASSIGN,BAND_ASSIGN"/>
+ *               value="ASSIGN,DIV_ASSIGN,PLUS_ASSIGN,MINUS_ASSIGN,STAR_ASSIGN,
+ *                      MOD_ASSIGN,SR_ASSIGN,BSR_ASSIGN,SL_ASSIGN,BXOR_ASSIGN,
+ *                      BOR_ASSIGN,BAND_ASSIGN"/>
+ * </module>
+ *
+ * + *

An example of how to configure the check for whitespace only around + * curly braces is: + * 

+ * <module name="WhitespaceAround">
+ *     <property name="tokens"
+ *               value="LCURLY,RCURLY"/>
  * </module>
  *
* @@ -115,6 +126,13 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * public @interface Beta {} // empty annotation type * } * + *

This check does not flag as violation double brace initialization like:

+ * 
+ *   new Properties() {{
+ *     setProperty("key", "value");
+ *   }};
+ *
+ * *

To configure the check to allow empty method blocks use * * 

   <property name="allowEmptyMethods" value="true" />
@@ -353,39 +371,23 @@ public class WhitespaceAroundCheck extends AbstractCheck { final int before = ast.getColumnNo() - 1; final int after = ast.getColumnNo() + ast.getText().length(); - if (before >= 0 && !Character.isWhitespace(line.charAt(before))) { - log(ast.getLineNo(), ast.getColumnNo(), - MSG_WS_NOT_PRECEDED, ast.getText()); + if (before >= 0) { + final char prevChar = line.charAt(before); + if (shouldCheckSeparationFromPreviousToken(ast) + && !Character.isWhitespace(prevChar)) { + log(ast.getLineNo(), ast.getColumnNo(), + MSG_WS_NOT_PRECEDED, ast.getText()); + } } - if (after >= line.length()) { - return; + if (after < line.length()) { + final char nextChar = line.charAt(after); + if (shouldCheckSeparationFromNextToken(ast, nextChar) + && !Character.isWhitespace(nextChar)) { + log(ast.getLineNo(), ast.getColumnNo() + ast.getText().length(), + MSG_WS_NOT_FOLLOWED, ast.getText()); + } } - - final char nextChar = line.charAt(after); - if (!Character.isWhitespace(nextChar) - // Check for "return;" - && !(currentType == TokenTypes.LITERAL_RETURN - && ast.getFirstChild().getType() == TokenTypes.SEMI) - && !isAnonymousInnerClassEnd(currentType, nextChar)) { - - log(ast.getLineNo(), ast.getColumnNo() + ast.getText().length(), - MSG_WS_NOT_FOLLOWED, ast.getText()); - } - } - - /** - * Check for "})" or "};" or "},". Happens with anon-inners - * @param currentType token - * @param nextChar next symbol - * @return true is that is end of anon inner class - */ - private static boolean isAnonymousInnerClassEnd(int currentType, char nextChar) { - return currentType == TokenTypes.RCURLY - && (nextChar == ')' - || nextChar == ';' - || nextChar == ',' - || nextChar == '.'); } /** @@ -404,9 +406,10 @@ public class WhitespaceAroundCheck extends AbstractCheck { final boolean starImportOrSlistInsideCaseGroup = starImport || slistInsideCaseGroup; final boolean colonOfCaseOrDefaultOrForEach = isColonOfCaseOrDefault(currentType, parentType) - || isColonOfForEach(currentType, parentType); - final boolean emptyBlockOrType = isEmptyBlock(ast, parentType) - || allowEmptyTypes && isEmptyType(ast); + || isColonOfForEach(currentType, parentType); + final boolean emptyBlockOrType = + isEmptyBlock(ast, parentType) + || allowEmptyTypes && isEmptyType(ast); return starImportOrSlistInsideCaseGroup || colonOfCaseOrDefaultOrForEach @@ -414,6 +417,56 @@ public class WhitespaceAroundCheck extends AbstractCheck { || isArrayInitialization(currentType, parentType); } + /** + * Check if it should be checked if previous token is separated from current by + * whitespace. + * This function is needed to recognise double brace initialization as valid, + * unfortunately its not possible to implement this functionality + * in isNotRelevantSituation method, because in this method when we return + * true(is not relevant) ast is later doesnt check at all. For example: + * new Properties() {{setProperty("double curly braces", "are not a style error"); + * }}; + * For second left curly brace in first line when we would return true from + * isNotRelevantSituation it wouldn't later check that the next token(setProperty) + * is not separated from previous token. + * @param ast current AST. + * @return true if it should be checked if previous token is separated by whitespace, + * false otherwise. + */ + private static boolean shouldCheckSeparationFromPreviousToken(DetailAST ast) { + return !isPartOfDoubleBraceInitializerForPreviousToken(ast); + } + + /** + * Check if it should be checked if next token is separated from current by + * whitespace. Explanation why this method is needed is identical to one + * included in shouldCheckSeparationFromPreviousToken method. + * @param ast current AST. + * @param nextChar next character. + * @return true if it should be checked if next token is separated by whitespace, + * false otherwise. + */ + private static boolean shouldCheckSeparationFromNextToken(DetailAST ast, char nextChar) { + return !(ast.getType() == TokenTypes.LITERAL_RETURN + && ast.getFirstChild().getType() == TokenTypes.SEMI) + && !isAnonymousInnerClassEnd(ast.getType(), nextChar) + && !isPartOfDoubleBraceInitializerForNextToken(ast); + } + + /** + * Check for "})" or "};" or "},". Happens with anon-inners + * @param currentType token + * @param nextChar next symbol + * @return true is that is end of anon inner class + */ + private static boolean isAnonymousInnerClassEnd(int currentType, char nextChar) { + return currentType == TokenTypes.RCURLY + && (nextChar == ')' + || nextChar == ';' + || nextChar == ',' + || nextChar == '.'); + } + /** * Is empty block. * @param ast ast @@ -448,13 +501,13 @@ public class WhitespaceAroundCheck extends AbstractCheck { final DetailAST parent = ast.getParent(); final DetailAST grandParent = ast.getParent().getParent(); return parentType == TokenTypes.SLIST - && parent.getFirstChild().getType() == TokenTypes.RCURLY - && grandParent.getType() == match; + && parent.getFirstChild().getType() == TokenTypes.RCURLY + && grandParent.getType() == match; } return type == TokenTypes.SLIST - && parentType == match - && ast.getFirstChild().getType() == TokenTypes.RCURLY; + && parentType == match + && ast.getFirstChild().getType() == TokenTypes.RCURLY; } /** @@ -466,7 +519,7 @@ public class WhitespaceAroundCheck extends AbstractCheck { private static boolean isColonOfCaseOrDefault(int currentType, int parentType) { return currentType == TokenTypes.COLON && (parentType == TokenTypes.LITERAL_DEFAULT - || parentType == TokenTypes.LITERAL_CASE); + || parentType == TokenTypes.LITERAL_CASE); } /** @@ -477,8 +530,8 @@ public class WhitespaceAroundCheck extends AbstractCheck { */ private boolean isColonOfForEach(int currentType, int parentType) { return currentType == TokenTypes.COLON - && parentType == TokenTypes.FOR_EACH_CLAUSE - && ignoreEnhancedForColon; + && parentType == TokenTypes.FOR_EACH_CLAUSE + && ignoreEnhancedForColon; } /** @@ -488,10 +541,9 @@ public class WhitespaceAroundCheck extends AbstractCheck { * @return true is current token inside array initialization */ private static boolean isArrayInitialization(int currentType, int parentType) { - return (currentType == TokenTypes.RCURLY - || currentType == TokenTypes.LCURLY) - && (parentType == TokenTypes.ARRAY_INIT - || parentType == TokenTypes.ANNOTATION_ARRAY_INIT); + return (currentType == TokenTypes.RCURLY || currentType == TokenTypes.LCURLY) + && (parentType == TokenTypes.ARRAY_INIT + || parentType == TokenTypes.ANNOTATION_ARRAY_INIT); } /** @@ -504,7 +556,7 @@ public class WhitespaceAroundCheck extends AbstractCheck { */ private boolean isEmptyMethodBlock(DetailAST ast, int parentType) { return allowEmptyMethods - && isEmptyBlock(ast, parentType, TokenTypes.METHOD_DEF); + && isEmptyBlock(ast, parentType, TokenTypes.METHOD_DEF); } /** @@ -517,7 +569,7 @@ public class WhitespaceAroundCheck extends AbstractCheck { */ private boolean isEmptyCtorBlock(DetailAST ast, int parentType) { return allowEmptyConstructors - && isEmptyBlock(ast, parentType, TokenTypes.CTOR_DEF); + && isEmptyBlock(ast, parentType, TokenTypes.CTOR_DEF); } /** @@ -529,11 +581,9 @@ public class WhitespaceAroundCheck extends AbstractCheck { */ private boolean isEmptyLoop(DetailAST ast, int parentType) { return allowEmptyLoops - && (isEmptyBlock(ast, parentType, TokenTypes.LITERAL_FOR) - || isEmptyBlock(ast, - parentType, TokenTypes.LITERAL_WHILE) - || isEmptyBlock(ast, - parentType, TokenTypes.LITERAL_DO)); + && (isEmptyBlock(ast, parentType, TokenTypes.LITERAL_FOR) + || isEmptyBlock(ast, parentType, TokenTypes.LITERAL_WHILE) + || isEmptyBlock(ast, parentType, TokenTypes.LITERAL_DO)); } /** @@ -565,9 +615,42 @@ public class WhitespaceAroundCheck extends AbstractCheck { final DetailAST nextSibling = ast.getNextSibling(); final DetailAST previousSibling = ast.getPreviousSibling(); return type == TokenTypes.LCURLY - && nextSibling.getType() == TokenTypes.RCURLY - || type == TokenTypes.RCURLY - && previousSibling != null - && previousSibling.getType() == TokenTypes.LCURLY; + && nextSibling.getType() == TokenTypes.RCURLY + || type == TokenTypes.RCURLY + && previousSibling != null + && previousSibling.getType() == TokenTypes.LCURLY; + } + + /** + * Check if given ast is part of double brace initializer and if it + * should omit checking if previous token is separated by whitespace. + * @param ast ast to check + * @return true if it should omit checking for previous token, false otherwise + */ + private static boolean isPartOfDoubleBraceInitializerForPreviousToken(DetailAST ast) { + final boolean initializerBeginsAfterClassBegins = ast.getType() == TokenTypes.SLIST + && ast.getParent().getType() == TokenTypes.INSTANCE_INIT; + final boolean classEndsAfterInitializerEnds = ast.getType() == TokenTypes.RCURLY + && ast.getPreviousSibling() != null + && ast.getPreviousSibling().getType() == TokenTypes.INSTANCE_INIT; + return initializerBeginsAfterClassBegins || classEndsAfterInitializerEnds; + } + + /** + * Check if given ast is part of double brace initializer and if it + * should omit checking if next token is separated by whitespace. + * See + * PR#2845 for more information why this function was needed. + * @param ast ast to check + * @return true if it should omit checking for next token, false otherwise + */ + private static boolean isPartOfDoubleBraceInitializerForNextToken(DetailAST ast) { + final boolean classBeginBeforeInitializerBegin = ast.getType() == TokenTypes.LCURLY + && ast.getNextSibling().getType() == TokenTypes.INSTANCE_INIT; + final boolean initalizerEndsBeforeClassEnds = ast.getType() == TokenTypes.RCURLY + && ast.getParent().getType() == TokenTypes.SLIST + && ast.getParent().getParent().getType() == TokenTypes.INSTANCE_INIT + && ast.getParent().getParent().getNextSibling().getType() == TokenTypes.RCURLY; + return classBeginBeforeInitializerBegin || initalizerEndsBeforeClassEnds; } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheckTest.java index d570ceade..0891c6bae 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/WhitespaceAroundCheckTest.java @@ -164,6 +164,20 @@ public class WhitespaceAroundCheckTest expected); } + @Test + public void testAllowDoubleBraceInitialization() throws Exception { + final String[] expected = { + "11:73: " + getCheckMessage(MSG_WS_NOT_PRECEDED, "}"), + "12:28: " + getCheckMessage(MSG_WS_NOT_FOLLOWED, "{"), + "14:28: " + getCheckMessage(MSG_WS_NOT_FOLLOWED, "{"), + "14:88: " + getCheckMessage(MSG_WS_NOT_PRECEDED, "}"), + "17:10: " + getCheckMessage(MSG_WS_NOT_FOLLOWED, "}"), + "17:24: " + getCheckMessage(MSG_WS_NOT_PRECEDED, "}"), + }; + verify(checkConfig, getPath("InputDoubleBraceInitialization.java"), + expected); + } + @Test public void testIgnoreEnhancedForColon() throws Exception { checkConfig.addAttribute("ignoreEnhancedForColon", "false"); diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputDoubleBraceInitialization.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputDoubleBraceInitialization.java new file mode 100644 index 000000000..5c24aecb8 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/InputDoubleBraceInitialization.java @@ -0,0 +1,19 @@ +package com.puppycrawl.tools.checkstyle.checks.whitespace; + +import java.util.Properties; + +public class InputDoubleBraceInitialization { + public InputDoubleBraceInitialization() { + new Properties() {{ + setProperty("double curly braces", "are not a style error"); + }}; + new Properties() {{ + setProperty("double curly braces", "are not a style error");}}; + new Properties() {{setProperty("double curly braces", "are not a style error"); + }}; + new Properties() {{setProperty("double curly braces", "are not a style error");}}; + new Properties() {{ + setProperty("double curly braces", "are not a style error"); + }private int i;}; + } +} diff --git a/src/xdocs/config_whitespace.xml b/src/xdocs/config_whitespace.xml index 1fd76713a..54e4c3ca0 100644 --- a/src/xdocs/config_whitespace.xml +++ b/src/xdocs/config_whitespace.xml @@ -1833,6 +1833,12 @@ public @interface Beta {} // empty annotation type , allowEmptyTypes, allowEmptyLoops and allowEmptyLambdas properties.

+

This check does not flag as violation double brace initialization like:

+ 

+new Properties() {{
+    setProperty("key", "value");
+}};
+