ParenPad incorretly flags unsupported token

[mahfouz72]

Should be considered with #4175

I have read check documentation: https://checkstyle.org/checks/whitespace/parenpad.html#ParenPad
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

PS D:\test> cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

 <module name="TreeWalker">
     <module name="ParenPad"/>
    </module>
</module>

PS D:\test> cat src/Main.java
import static java.lang.StringTemplate.STR;
record ColoredPoint(int x, int y, String c) {
}
record Rectangle(ColoredPoint upperLeft, ColoredPoint lowerRight) {
}

public class Main {
    public static void main(String[] args) {
        ColoredPoint cp = new ColoredPoint(1, 2, "red");
        Rectangle r = new Rectangle(cp, new ColoredPoint(3, 4, "blue"));
        printUpperLeftColoredPoint(r);

    }
    static void printUpperLeftColoredPoint(Object obj) {
            if (obj instanceof Rectangle(  ColoredPoint(_, int y, _), _)) {  // violation
                System.out.println(y);
            }
        }
}
PS D:\test> java  -jar checkstyle-10.14.2-all.jar -c config.xml  src/Main.java
Starting audit...
[ERROR] D:\test\src\Main.java:15:41: '(' is followed by whitespace. [ParenPad]
Audit done.
Checkstyle ends with 1 errors.
PS D:\test> 

Describe what you expect in detail.
no violation as RECORD_PATTERN token is not supported in the check .

Note: Check supports token RECORD_DEF .

[romani]

$ java -jar checkstyle-10.15.0-all.jar -t Test.java
....
|--LITERAL_IF -> if [15:12]
|   |--LPAREN -> ( [15:15]
|   |--EXPR -> EXPR [15:20]
|   |   `--LITERAL_INSTANCEOF -> instanceof [15:20]
|   |       |--IDENT -> obj [15:16]
|   |       `--RECORD_PATTERN_DEF -> RECORD_PATTERN_DEF [15:31]
|   |           |--MODIFIERS -> MODIFIERS [15:31]
|   |           |--TYPE -> TYPE [15:31]
|   |           |   `--IDENT -> Rectangle [15:31]
|   |           |--LPAREN -> ( [15:40]
|   |           |--RECORD_PATTERN_COMPONENTS -> RECORD_PATTERN_COMPONENTS [15:43]
|   |           |   |--RECORD_PATTERN_DEF -> RECORD_PATTERN_DEF [15:43]
|   |           |   |   |--MODIFIERS -> MODIFIERS [15:43]
|   |           |   |   |--TYPE -> TYPE [15:43]
|   |           |   |   |   `--IDENT -> ColoredPoint [15:43]
|   |           |   |   |--LPAREN -> ( [15:55]
|   |           |   |   |--RECORD_PATTERN_COMPONENTS -> RECORD_PATTERN_COMPONENTS [15:56]
|   |           |   |   |   |--UNNAMED_PATTERN_DEF -> _ [15:56]
|   |           |   |   |   |--COMMA -> , [15:57]
|   |           |   |   |   |--PATTERN_VARIABLE_DEF -> PATTERN_VARIABLE_DEF [15:59]
|   |           |   |   |   |   |--MODIFIERS -> MODIFIERS [15:59]
|   |           |   |   |   |   |--TYPE -> TYPE [15:59]
|   |           |   |   |   |   |   `--LITERAL_INT -> int [15:59]
|   |           |   |   |   |   `--IDENT -> y [15:63]
|   |           |   |   |   |--COMMA -> , [15:64]
|   |           |   |   |   `--UNNAMED_PATTERN_DEF -> _ [15:66]
|   |           |   |   `--RPAREN -> ) [15:67]
|   |           |   |--COMMA -> , [15:68]
|   |           |   `--UNNAMED_PATTERN_DEF -> _ [15:70]
|   |           `--RPAREN -> ) [15:71]

[rnveach]

This sounds like a duplicate of #4175

[mahfouz72]

@romani @rnveach I am confirming that the issue in processExpression method

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java

Lines 179 to 198 in e70836b

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

We are traversing the whole subtree under any expression and validating every ( and ) the thing is that we are now flagging all tokens under expressions even if it is not in the configuration.

My proposed solution is to remove the deep scan from EXPR token. this token can have under it a huge number of parens and it may be weird and confusing to validate all of them. then see where we missing functionality and add those as new tokens

example:

  for ( int i = 0; i < ( long) ( 2 * ( 4 / 2)); i++) {}

all the parens in the above example were checked under expression now we should remove this deep scanning under expression and add TYPECAST token , STAR token etc... as default tokens in the checks

please share your thoughts I am not sure if this is a good solution because we may end up adding a lot of new tokens in the check

[mahfouz72]

I am on it and waiting for a confirmation of the approach above