Skip to content

Commit

Permalink
Add possibility to configure targets for ParenPad check, #1189
Browse files Browse the repository at this point in the history
  • Loading branch information
@Vladlis @romani
Vladlis authored and romani committed Jun 25, 2015
1 parent 3e25a49 commit 7f452f6
Show file tree
Hide file tree
Showing 5 changed files with 325 additions and 66 deletions.
1 change: 0 additions & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,6 @@
<regex><pattern>.*.checks.whitespace.NoWhitespaceAfterCheck</pattern><branchRate>94</branchRate><lineRate>98</lineRate></regex>
<regex><pattern>.*.checks.whitespace.NoWhitespaceBeforeCheck</pattern><branchRate>90</branchRate><lineRate>100</lineRate></regex>
<regex><pattern>.*.checks.whitespace.OperatorWrapCheck</pattern><branchRate>68</branchRate><lineRate>81</lineRate></regex>
<regex><pattern>.*.checks.whitespace.ParenPadCheck</pattern><branchRate>86</branchRate><lineRate>95</lineRate></regex>
<regex><pattern>.*.checks.whitespace.SeparatorWrapCheck</pattern><branchRate>100</branchRate><lineRate>93</lineRate></regex>
<regex><pattern>.*.checks.whitespace.TypecastParenPadCheck</pattern><branchRate>87</branchRate><lineRate>88</lineRate></regex>
<regex><pattern>.*.checks.whitespace.WhitespaceAfterCheck</pattern><branchRate>86</branchRate><lineRate>90</lineRate></regex>
Expand Down
229 changes: 181 additions & 48 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,47 @@

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;

/**
* <p>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.
* </p>
* <p>
* The policy to verify is specified using the {@link PadOption} class and
* defaults to {@link PadOption#NOSPACE}.
* </p>
* <p> 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},
* </p>
* <p>
* An example of how to configure the check is:
Expand All @@ -60,86 +79,200 @@
* &lt;/module&gt;
* </pre>
* @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;
}
}
37 changes: 37 additions & 0 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

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;
Expand Down Expand Up @@ -184,6 +185,42 @@ public void testNospaceWithComplexInput() throws Exception {
"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);
}
Expand Down
25 changes: 25 additions & 0 deletions src/test/resources/com/puppycrawl/tools/checkstyle/whitespace/InputParenPad.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,29 @@ void method(boolean status ) {
}
}
}

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)
);
};
}

Loading

0 comments on commit 7f452f6

Please sign in to comment.