diff --git a/pom.xml b/pom.xml index 8e9be98d6..ab5c42085 100644 --- a/pom.xml +++ b/pom.xml @@ -907,7 +907,6 @@ .*.checks.whitespace.NoWhitespaceAfterCheck9498 .*.checks.whitespace.NoWhitespaceBeforeCheck90100 .*.checks.whitespace.OperatorWrapCheck6881 - .*.checks.whitespace.ParenPadCheck8695 .*.checks.whitespace.SeparatorWrapCheck10093 .*.checks.whitespace.TypecastParenPadCheck8788 .*.checks.whitespace.WhitespaceAfterCheck8690 diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java index e1a5a8a30..66677084f 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java @@ -19,6 +19,8 @@ package com.puppycrawl.tools.checkstyle.checks.whitespace; +import java.util.Arrays; + import com.puppycrawl.tools.checkstyle.api.DetailAST; import com.puppycrawl.tools.checkstyle.api.TokenTypes; @@ -26,9 +28,12 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; *

Checks the padding of parentheses; that is whether a space is required * after a left parenthesis and before a right parenthesis, or such spaces are * forbidden, with the exception that it does - * not check for padding of the right parenthesis at an empty for iterator. + * not check for padding of the right parenthesis at an empty for iterator and + * empty for initializer. * Use Check {@link EmptyForIteratorPadCheck EmptyForIteratorPad} to validate - * empty for iterators. + * empty for iterators and {@link EmptyForInitializerPadCheck EmptyForInitializerPad} + * to validate empty for initializers. Typecasts are also not checked, as there is + * {@link TypecastParenPadCheck TypecastParenPad} to validate them. *

*

* The policy to verify is specified using the {@link PadOption} class and @@ -36,11 +41,25 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; *

*

By default the check will check parentheses that occur with the following * tokens: + * {@link TokenTypes#ANNOTATION ANNOTATION}, + * {@link TokenTypes#ANNOTATION_FIELD_DEF ANNOTATION_FIELD_DEF}, + * {@link TokenTypes#CTOR_DEF CTOR_DEF}, * {@link TokenTypes#CTOR_CALL CTOR_CALL}, - * {@link TokenTypes#LPAREN LPAREN}, + * {@link TokenTypes#ENUM_CONSTANT_DEF ENUM_CONSTANT_DEF}, + * {@link TokenTypes#EXPR EXPR}, + * {@link TokenTypes#LITERAL_CATCH LITERAL_CATCH}, + * {@link TokenTypes#LITERAL_DO LITERAL_DO}, + * {@link TokenTypes#LITERAL_FOR LITERAL_FOR}, + * {@link TokenTypes#LITERAL_IF LITERAL_IF}, + * {@link TokenTypes#LITERAL_NEW LITERAL_NEW}, + * {@link TokenTypes#LITERAL_SWITCH LITERAL_SWITCH}, + * {@link TokenTypes#LITERAL_SYNCHRONIZED LITERAL_SYNCHRONIZED}, + * {@link TokenTypes#LITERAL_WHILE LITERAL_WHILE}, * {@link TokenTypes#METHOD_CALL METHOD_CALL}, - * {@link TokenTypes#RPAREN RPAREN}, + * {@link TokenTypes#METHOD_DEF METHOD_DEF}, + * {@link TokenTypes#RESOURCE_SPECIFICATION RESOURCE_SPECIFICATION}, * {@link TokenTypes#SUPER_CTOR_CALL SUPER_CTOR_CALL}, + * {@link TokenTypes#QUESTION QUESTION}, *

*

* An example of how to configure the check is: @@ -60,86 +79,200 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * </module> * * @author Oliver Burn + * @author Vladislav Lisetskiy */ public class ParenPadCheck extends AbstractParenPadCheck { + + /** + * The array of Acceptable Tokens. + */ + private final int[] acceptableTokens; + + /** + * Initializes and sorts acceptableTokens to make binary search over it possible. + */ + public ParenPadCheck() { + acceptableTokens = makeAcceptableTokens(); + Arrays.sort(acceptableTokens); + } + @Override public int[] getDefaultTokens() { - return new int[] {TokenTypes.RPAREN, - TokenTypes.LPAREN, - TokenTypes.CTOR_CALL, - TokenTypes.SUPER_CTOR_CALL, - TokenTypes.METHOD_CALL, - }; + return getAcceptableTokens(); } @Override public int[] getAcceptableTokens() { - return new int[] {TokenTypes.RPAREN, - TokenTypes.LPAREN, - TokenTypes.CTOR_CALL, - TokenTypes.SUPER_CTOR_CALL, - TokenTypes.METHOD_CALL, - }; + return makeAcceptableTokens(); } @Override public void visitToken(DetailAST ast) { - DetailAST theAst = ast; - // Strange logic in this method to guard against checking RPAREN tokens - // that are associated with a TYPECAST token. - if (theAst.getType() != TokenTypes.RPAREN) { - if (theAst.getType() == TokenTypes.CTOR_CALL - || theAst.getType() == TokenTypes.SUPER_CTOR_CALL) { - theAst = theAst.getFirstChild(); - } - if (!isPreceedsEmptyForInit(theAst)) { - processLeft(theAst); + switch (ast.getType()) { + case TokenTypes.METHOD_CALL: + processLeft(ast); + processRight(ast.findFirstToken(TokenTypes.RPAREN)); + processExpression(ast); + break; + case TokenTypes.EXPR: + case TokenTypes.QUESTION: + processExpression(ast); + break; + case TokenTypes.LITERAL_FOR: + visitLiteralFor(ast); + break; + case TokenTypes.ANNOTATION: + case TokenTypes.ENUM_CONSTANT_DEF: + case TokenTypes.LITERAL_NEW: + case TokenTypes.LITERAL_SYNCHRONIZED: + visitNewEnumConstDefAnnotationSync(ast); + break; + default: + processLeft(ast.findFirstToken(TokenTypes.LPAREN)); + processRight(ast.findFirstToken(TokenTypes.RPAREN)); + } + } + + /** + * Checks parens in {@link TokenTypes#ENUM_CONSTANT_DEF}, {@link TokenTypes#ANNOTATION} + * {@link TokenTypes#LITERAL_SYNCHRONIZED} and {@link TokenTypes#LITERAL_NEW}. + * @param ast the token to check. + */ + private void visitNewEnumConstDefAnnotationSync(DetailAST ast) { + final DetailAST parenAst = ast.findFirstToken(TokenTypes.LPAREN); + if (parenAst != null) { + processLeft(parenAst); + processRight(ast.findFirstToken(TokenTypes.RPAREN)); + } + } + + /** + * Checks parens in {@link TokenTypes#FOR_LITERAL}. + * @param ast the token to check. + */ + private void visitLiteralFor(DetailAST ast) { + DetailAST parenAst = ast.findFirstToken(TokenTypes.LPAREN); + if (!isPreceedsEmptyForInit(parenAst)) { + processLeft(parenAst); + } + parenAst = ast.findFirstToken(TokenTypes.RPAREN); + if (!isFollowsEmptyForIterator(parenAst)) { + processRight(parenAst); + } + } + + /** + * Checks parens inside {@link TokenTypes#EXPR}, {@link TokenTypes#QUESTION} + * and {@link TokenTypes#METHOD_CALL}. + * @param ast the token to check. + */ + private void processExpression(DetailAST ast) { + if (ast.branchContains(TokenTypes.LPAREN)) { + DetailAST childAst = ast.getFirstChild(); + while (childAst != null) { + if (childAst.getType() == TokenTypes.LPAREN) { + processLeft(childAst); + processExpression(childAst); + } + else if (childAst.getType() == TokenTypes.RPAREN && !isInTypecast(childAst)) { + processRight(childAst); + } + else if (!isAcceptableToken(childAst)) { + //Traverse all subtree tokens which will never be configured + //to be launched in visitToken() + processExpression(childAst); + } + childAst = childAst.getNextSibling(); } } - else if ((theAst.getParent() == null - || theAst.getParent().getType() != TokenTypes.TYPECAST - || theAst.getParent().findFirstToken(TokenTypes.RPAREN) - != theAst) - && !isFollowsEmptyForIterator(theAst)) { - processRight(theAst); + } + + /** + * Checks whether AcceptableTokens contains the given ast. + * @param ast the token to check. + * @return true if the ast is in AcceptableTokens. + */ + private boolean isAcceptableToken(DetailAST ast) { + boolean result = false; + if (Arrays.binarySearch(acceptableTokens, ast.getType()) >= 0) { + result = true; } + return result; + } + + /** + * @return acceptableTokens. + */ + private static int[] makeAcceptableTokens() { + return new int[] {TokenTypes.ANNOTATION, + TokenTypes.ANNOTATION_FIELD_DEF, + TokenTypes.CTOR_CALL, + TokenTypes.CTOR_DEF, + TokenTypes.ENUM_CONSTANT_DEF, + TokenTypes.EXPR, + TokenTypes.LITERAL_CATCH, + TokenTypes.LITERAL_DO, + TokenTypes.LITERAL_FOR, + TokenTypes.LITERAL_IF, + TokenTypes.LITERAL_NEW, + TokenTypes.LITERAL_SWITCH, + TokenTypes.LITERAL_SYNCHRONIZED, + TokenTypes.LITERAL_WHILE, + TokenTypes.METHOD_CALL, + TokenTypes.METHOD_DEF, + TokenTypes.QUESTION, + TokenTypes.RESOURCE_SPECIFICATION, + TokenTypes.SUPER_CTOR_CALL, + }; + } + + /** + * Checks whether {@link TokenTypes#RPAREN} is a closing paren + * of a {@link TokenTypes#TYPECAST}. + * @param ast of a {@link TokenTypes#RPAREN} to check. + * @return true if ast is a closing paren of a {@link TokenTypes#TYPECAST}. + */ + private static boolean isInTypecast(DetailAST ast) { + boolean result = false; + if (ast.getParent().getType() == TokenTypes.TYPECAST) { + final DetailAST firstRparen = ast.getParent().findFirstToken(TokenTypes.RPAREN); + if (firstRparen.getLineNo() == ast.getLineNo() + && firstRparen.getColumnNo() == ast.getColumnNo()) { + result = true; + } + } + return result; } /** * @param ast the token to check * @return whether a token follows an empty for iterator */ - private boolean isFollowsEmptyForIterator(DetailAST ast) { - boolean followsEmptyForIterator = false; + private static boolean isFollowsEmptyForIterator(DetailAST ast) { + boolean result = false; final DetailAST parent = ast.getParent(); //Only traditional for statements are examined, not for-each statements - if (parent != null - && parent.getType() == TokenTypes.LITERAL_FOR - && parent.findFirstToken(TokenTypes.FOR_EACH_CLAUSE) == null) { + if (parent.findFirstToken(TokenTypes.FOR_EACH_CLAUSE) == null) { final DetailAST forIterator = parent.findFirstToken(TokenTypes.FOR_ITERATOR); - followsEmptyForIterator = forIterator.getChildCount() == 0 - && ast == forIterator.getNextSibling(); + result = forIterator.getChildCount() == 0; } - return followsEmptyForIterator; + return result; } /** * @param ast the token to check * @return whether a token preceeds an empty for initializer */ - private boolean isPreceedsEmptyForInit(DetailAST ast) { - boolean preceedsEmptyForInintializer = false; + private static boolean isPreceedsEmptyForInit(DetailAST ast) { + boolean result = false; final DetailAST parent = ast.getParent(); //Only traditional for statements are examined, not for-each statements - if (parent != null - && parent.getType() == TokenTypes.LITERAL_FOR - && parent.findFirstToken(TokenTypes.FOR_EACH_CLAUSE) == null) { + if (parent.findFirstToken(TokenTypes.FOR_EACH_CLAUSE) == null) { final DetailAST forIterator = parent.findFirstToken(TokenTypes.FOR_INIT); - preceedsEmptyForInintializer = forIterator.getChildCount() == 0 - && ast == forIterator.getPreviousSibling(); + result = forIterator.getChildCount() == 0; } - return preceedsEmptyForInintializer; + return result; } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheckTest.java index 4d752df6c..1b7c1aa12 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheckTest.java @@ -21,6 +21,7 @@ package com.puppycrawl.tools.checkstyle.checks.whitespace; import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport; import com.puppycrawl.tools.checkstyle.DefaultConfiguration; + import org.junit.Test; import static com.puppycrawl.tools.checkstyle.checks.whitespace.AbstractParenPadCheck.WS_FOLLOWED; @@ -184,6 +185,42 @@ public class ParenPadCheckTest "96:47: " + getCheckMessage(WS_PRECEDED, ")"), "97:42: " + getCheckMessage(WS_PRECEDED, ")"), "99:44: " + getCheckMessage(WS_PRECEDED, ")"), + "112:17: " + getCheckMessage(WS_FOLLOWED, "("), + "113:23: " + getCheckMessage(WS_FOLLOWED, "("), + "113:25: " + getCheckMessage(WS_FOLLOWED, "("), + "113:31: " + getCheckMessage(WS_PRECEDED, ")"), + "114:26: " + getCheckMessage(WS_FOLLOWED, "("), + "114:28: " + getCheckMessage(WS_FOLLOWED, "("), + "114:34: " + getCheckMessage(WS_PRECEDED, ")"), + "114:50: " + getCheckMessage(WS_PRECEDED, ")"), + "115:26: " + getCheckMessage(WS_FOLLOWED, "("), + "115:28: " + getCheckMessage(WS_FOLLOWED, "("), + "115:35: " + getCheckMessage(WS_PRECEDED, ")"), + "115:53: " + getCheckMessage(WS_PRECEDED, ")"), + "115:55: " + getCheckMessage(WS_PRECEDED, ")"), + "119:17: " + getCheckMessage(WS_FOLLOWED, "("), + "119:22: " + getCheckMessage(WS_PRECEDED, ")"), + "123:30: " + getCheckMessage(WS_FOLLOWED, "("), + "123:44: " + getCheckMessage(WS_PRECEDED, ")"), + "126:22: " + getCheckMessage(WS_FOLLOWED, "("), + "126:22: " + getCheckMessage(WS_PRECEDED, ")"), + "130:19: " + getCheckMessage(WS_FOLLOWED, "("), + "130:19: " + getCheckMessage(WS_PRECEDED, ")"), + }; + verify(checkConfig, getPath("whitespace/InputParenPad.java"), expected); + } + + @Test + public void testConfigureTokens() throws Exception { + final DefaultConfiguration checkConfig = + createCheckConfig(ParenPadCheck.class); + checkConfig.addAttribute("tokens", "METHOD_CALL"); + final String[] expected = { + "90:38: " + getCheckMessage(WS_PRECEDED, ")"), + "112:17: " + getCheckMessage(WS_FOLLOWED, "("), + "113:23: " + getCheckMessage(WS_FOLLOWED, "("), + "115:53: " + getCheckMessage(WS_PRECEDED, ")"), + "115:55: " + getCheckMessage(WS_PRECEDED, ")"), }; verify(checkConfig, getPath("whitespace/InputParenPad.java"), expected); } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/whitespace/InputParenPad.java b/src/test/resources/com/puppycrawl/tools/checkstyle/whitespace/InputParenPad.java index 0775f85d5..3a3cb34b7 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/whitespace/InputParenPad.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/whitespace/InputParenPad.java @@ -107,4 +107,29 @@ public class InputParenPad } } } + + String foo() { + return ( (Object + ) bar( ( 1 > 2 ) ? + ( ( 3 < 4 )? false : true ) : + ( ( 1 == 1 ) ? false : true) ) ).toString(); + } + @MyAnnotation + public boolean bar(boolean a) { + assert ( true ); + return true; + } + + boolean fooo = this.bar(( true && false ) && true); } +@interface MyAnnotation { + String someField( ) default "Hello world"; +} + +enum MyEnum { + SOME_CONSTANT( ) { + int i = (int) (2 * (4 / 2) + ); + }; +} + diff --git a/src/xdocs/config_whitespace.xml b/src/xdocs/config_whitespace.xml index c581ba5f3..a192d3159 100644 --- a/src/xdocs/config_whitespace.xml +++ b/src/xdocs/config_whitespace.xml @@ -712,7 +712,16 @@ for (Iterator foo = very.long.line.iterator();

Checks the policy on the padding of parentheses; i.e. whether a space is required after a left parenthesis and before a right - parenthesis, or such spaces are forbidden. + parenthesis, or such spaces are forbidden, with the exception that it does + not check for padding of the right parenthesis at an empty for iterator and + empty for initializer. + Use Check EmptyForIteratorPad + to validate empty for iterators and EmptyForInitializerPad + to validate empty for initializers. Typecasts are also not checked, as there is TypecastParenPad + to validate them.

@@ -736,28 +745,84 @@ for (Iterator foo = very.long.line.iterator(); subset of tokens ANNOTATION, + ANNOTATION_FIELD_DEF, + CTOR_DEF, + CTOR_CALL, LPAREN, - METHOD_CALL, - RPAREN, - SUPER_CTOR_CALL + href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#ENUM_CONSTANT_DEF">ENUM_CONSTANT_DEF, + EXPR, + LITERAL_CATCH, + LITERAL_DO, + LITERAL_FOR, + LITERAL_IF, + LITERAL_NEW, + LITERAL_SWITCH, + LITERAL_SYNCHRONIZED, + LITERAL_WHILE, + METHOD_CALL, + METHOD_DEF, + RESOURCE_SPECIFICATION, + SUPER_CTOR_CALL, + QUESTION CTOR_CALL, - LPAREN, - METHOD_CALL, - RPAREN, - SUPER_CTOR_CALL + href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#ANNOTATION">ANNOTATION, + ANNOTATION_FIELD_DEF, + CTOR_DEF, + CTOR_CALL, + ENUM_CONSTANT_DEF, + EXPR, + LITERAL_CATCH, + LITERAL_DO, + LITERAL_FOR, + LITERAL_IF, + LITERAL_NEW, + LITERAL_SWITCH, + LITERAL_SYNCHRONIZED, + LITERAL_WHILE, + METHOD_CALL, + METHOD_DEF, + RESOURCE_SPECIFICATION, + SUPER_CTOR_CALL, + QUESTION