Skip to content

Commit

Permalink
Issue #14747: fix ParenPad to not flag unsupported Tokens
Browse files Browse the repository at this point in the history
  • Loading branch information
mahfouz72 committed Apr 14, 2024
1 parent 69337c7 commit c16e3bc
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 93 deletions.
Expand Up @@ -2344,17 +2344,6 @@
</details>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java</fileName>
<specifier>argument</specifier>
<message>incompatible argument for parameter bitIndex of BitSet.get.</message>
<lineContent>return acceptableTokens.get(ast.getType());</lineContent>
<details>
found : int
required: @NonNegative int
</details>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/SeparatorWrapCheck.java</fileName>
<specifier>argument</specifier>
Expand Down
Expand Up @@ -4549,20 +4549,6 @@
<lineContent>result = latestDefinition == methodDef.getParent().getParent();</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java</fileName>
<specifier>not.interned</specifier>
<message>attempting to use a non-@Interned comparison operand</message>
<lineContent>while (currentNode.getNextSibling() == null &amp;&amp; currentNode.getParent() != ast) {</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java</fileName>
<specifier>not.interned</specifier>
<message>attempting to use a non-@Interned comparison operand</message>
<lineContent>while (currentNode.getNextSibling() == null &amp;&amp; currentNode.getParent() != ast) {</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/TypecastParenPadCheck.java</fileName>
<specifier>not.interned</specifier>
Expand Down
9 changes: 9 additions & 0 deletions config/linkcheck-suppressions.txt
Expand Up @@ -1345,3 +1345,12 @@
<a href="apidocs/com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.html#%3Cinit%3E(org.antlr.v4.runtime.Lexer)">#%3Cinit%3E(org.antlr.v4.runtime.Lexer)</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.StringTemplateContext.html#%3Cinit%3E(int,int)">#%3Cinit%3E(int,int)</a>: doesn't exist.
<a href="com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.StringTemplateContext.html#%3Cinit%3E(int,int)">com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.StringTemplateContext.html#%3Cinit%3E(int,int)</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#DIV">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#DIV</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAND">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAND</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LNOT">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LNOT</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LOR">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LOR</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MINUS">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MINUS</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MOD">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MOD</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PLUS">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PLUS</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#STAR">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#STAR</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#TYPECAST">../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#TYPECAST</a>: doesn't exist.
3 changes: 2 additions & 1 deletion config/pmd.xml
Expand Up @@ -288,7 +288,8 @@
| //ClassOrInterfaceDeclaration[@SimpleName='SarifLogger']//MethodDeclaration[@Name='escape']
| //ClassOrInterfaceDeclaration[@SimpleName='LeftCurlyCheck'
or @SimpleName='FinalLocalVariableCheck'
or @SimpleName='NPathComplexityCheck']
or @SimpleName='NPathComplexityCheck'
or @SimpleName='ParenPadCheck']
//MethodDeclaration[@Name='visitToken']
| //ClassOrInterfaceDeclaration[@SimpleName='FallThroughCheck']
//MethodDeclaration[@Name='isTerminated']
Expand Down
Expand Up @@ -19,8 +19,7 @@

package com.puppycrawl.tools.checkstyle.checks.whitespace;

import java.util.BitSet;

import com.puppycrawl.tools.checkstyle.StatelessCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
Expand Down Expand Up @@ -97,7 +96,26 @@
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAMBDA">
* LAMBDA</a>,
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#RECORD_DEF">
* RECORD_DEF</a>.
* RECORD_DEF</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#TYPECAST">
* TYPECAST</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#STAR">
* STAR</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PLUS">
* PLUS</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MINUS">
* MINUS</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#DIV">
* DIV</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MOD">
* MOD</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAND">
* LAND</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LOR">
* LOR</a>,
* <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LNOT">
* LNOT</a>
* .
* </li>
* </ul>
* <p>
Expand All @@ -123,20 +141,9 @@
*
* @since 3.0
*/
@StatelessCheck
public class ParenPadCheck extends AbstractParenPadCheck {

/**
* Tokens that this check handles.
*/
private final BitSet acceptableTokens;

/**
* Initializes acceptableTokens.
*/
public ParenPadCheck() {
acceptableTokens = TokenUtil.asBitSet(makeAcceptableTokens());
}

@Override
public int[] getDefaultTokens() {
return makeAcceptableTokens();
Expand All @@ -162,6 +169,15 @@ public void visitToken(DetailAST ast) {
case TokenTypes.DOT:
case TokenTypes.EXPR:
case TokenTypes.QUESTION:
case TokenTypes.TYPECAST:
case TokenTypes.STAR:
case TokenTypes.PLUS:
case TokenTypes.MINUS:
case TokenTypes.DIV:
case TokenTypes.MOD:
case TokenTypes.LAND:
case TokenTypes.LOR:
case TokenTypes.LNOT:
processExpression(ast);
break;
case TokenTypes.LITERAL_FOR:
Expand Down Expand Up @@ -239,8 +255,7 @@ private void visitLiteralFor(DetailAST ast) {
}

/**
* Checks parens inside {@link TokenTypes#EXPR}, {@link TokenTypes#QUESTION}
* and {@link TokenTypes#METHOD_CALL}.
* Checks all parens that are children of a given Token.
*
* @param ast the token to check.
*/
Expand All @@ -253,31 +268,10 @@ private void processExpression(DetailAST ast) {
else if (currentNode.getType() == TokenTypes.RPAREN && !isInTypecast(currentNode)) {
processRight(currentNode);
}
else if (currentNode.hasChildren() && !isAcceptableToken(currentNode)) {
// Traverse all subtree tokens which will never be configured
// to be launched in visitToken()
currentNode = currentNode.getFirstChild();
continue;
}

// Go up after processing the last child
while (currentNode.getNextSibling() == null && currentNode.getParent() != ast) {
currentNode = currentNode.getParent();
}
currentNode = currentNode.getNextSibling();
}
}

/**
* 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) {
return acceptableTokens.get(ast.getType());
}

/**
* Returns array of acceptable tokens.
*
Expand Down Expand Up @@ -306,6 +300,15 @@ private static int[] makeAcceptableTokens() {
TokenTypes.SUPER_CTOR_CALL,
TokenTypes.LAMBDA,
TokenTypes.RECORD_DEF,
TokenTypes.TYPECAST,
TokenTypes.STAR,
TokenTypes.PLUS,
TokenTypes.MINUS,
TokenTypes.DIV,
TokenTypes.MOD,
TokenTypes.LAND,
TokenTypes.LOR,
TokenTypes.LNOT,
};
}

Expand Down
Expand Up @@ -26,7 +26,7 @@
type="com.puppycrawl.tools.checkstyle.checks.whitespace.PadOption">
<description>Specify policy on how to pad parentheses.</description>
</property>
<property default-value="ANNOTATION,ANNOTATION_FIELD_DEF,CTOR_CALL,CTOR_DEF,DOT,ENUM_CONSTANT_DEF,EXPR,LITERAL_CATCH,LITERAL_DO,LITERAL_FOR,LITERAL_IF,LITERAL_NEW,LITERAL_SWITCH,LITERAL_SYNCHRONIZED,LITERAL_WHILE,METHOD_CALL,METHOD_DEF,QUESTION,RESOURCE_SPECIFICATION,SUPER_CTOR_CALL,LAMBDA,RECORD_DEF"
<property default-value="ANNOTATION,ANNOTATION_FIELD_DEF,CTOR_CALL,CTOR_DEF,DOT,ENUM_CONSTANT_DEF,EXPR,LITERAL_CATCH,LITERAL_DO,LITERAL_FOR,LITERAL_IF,LITERAL_NEW,LITERAL_SWITCH,LITERAL_SYNCHRONIZED,LITERAL_WHILE,METHOD_CALL,METHOD_DEF,QUESTION,RESOURCE_SPECIFICATION,SUPER_CTOR_CALL,LAMBDA,RECORD_DEF,TYPECAST,STAR,PLUS,MINUS,DIV,MOD,LAND,LOR,LNOT"
name="tokens"
type="java.lang.String[]"
validation-type="tokenSet">
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/google_checks.xml
Expand Up @@ -292,7 +292,7 @@
EXPR, LITERAL_CATCH, LITERAL_DO, LITERAL_FOR, LITERAL_IF, LITERAL_NEW,
LITERAL_SWITCH, LITERAL_SYNCHRONIZED, LITERAL_WHILE, METHOD_CALL,
METHOD_DEF, QUESTION, RESOURCE_SPECIFICATION, SUPER_CTOR_CALL, LAMBDA,
RECORD_DEF"/>
RECORD_DEF, TYPECAST, STAR, PLUS, MINUS, DIV, MOD, LAND, LOR, LNOT"/>
</module>
<module name="OperatorWrap">
<property name="option" value="NL"/>
Expand Down
Expand Up @@ -28,11 +28,8 @@
import org.junit.jupiter.api.Test;

import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
import com.puppycrawl.tools.checkstyle.DetailAstImpl;
import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
import com.puppycrawl.tools.checkstyle.internal.utils.TestUtil;
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
import com.puppycrawl.tools.checkstyle.utils.TokenUtil;

public class ParenPadCheckTest
extends AbstractModuleTestSupport {
Expand Down Expand Up @@ -543,28 +540,4 @@ public void testParenPadForEnum() throws Exception {
getPath("InputParenPadForEnum.java"), expected);
}

/**
* Pitest requires us to specify more concrete lower bound for condition for
* ParenPadCheck#isAcceptableToken as nodes of first several types like CTOR_DEF,
* METHOD_DEF will never reach this method. It is hard to recreate conditions for
* all tokens to go through this method. We do not want to change main code to have
* this set ok tokens more exact, because it will not be ease to understand.
* So we have to use reflection to be sure all
* acceptable tokens pass that check.
*/
@Test
public void testIsAcceptableToken() throws Exception {
final ParenPadCheck check = new ParenPadCheck();
final DetailAstImpl ast = new DetailAstImpl();
final String message = "Expected that all acceptable tokens will pass isAcceptableToken "
+ "method, but some token don't: ";

for (int token : check.getAcceptableTokens()) {
ast.setType(token);
assertWithMessage(message + TokenUtil.getTokenName(token))
.that(TestUtil.<Boolean>invokeMethod(check, "isAcceptableToken", ast))
.isTrue();
}
}

}
36 changes: 36 additions & 0 deletions src/xdocs/checks/whitespace/parenpad.xml
Expand Up @@ -92,6 +92,24 @@
LAMBDA</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#RECORD_DEF">
RECORD_DEF</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#TYPECAST">
TYPECAST</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#STAR">
STAR</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PLUS">
PLUS</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MINUS">
MINUS</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#DIV">
DIV</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MOD">
MOD</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAND">
LAND</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LOR">
LOR</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LNOT">
LNOT</a>
.
</td>
<td>
Expand Down Expand Up @@ -139,6 +157,24 @@
LAMBDA</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#RECORD_DEF">
RECORD_DEF</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#TYPECAST">
TYPECAST</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#STAR">
STAR</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PLUS">
PLUS</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MINUS">
MINUS</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#DIV">
DIV</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#MOD">
MOD</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAND">
LAND</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LOR">
LOR</a>
, <a href="../../apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LNOT">
LNOT</a>
.
</td>
<td>3.0</td>
Expand Down

0 comments on commit c16e3bc

Please sign in to comment.