Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array declarator bracket placement in TYPE_ARGUMENTS AST is incorrect #10181

Closed
nrmancuso opened this issue Jun 25, 2021 · 2 comments
Closed

Comments

@nrmancuso
Copy link
Member

nrmancuso commented Jun 25, 2021

➜  src javac Test.java

➜  src cat Test.java
import java.lang.reflect.TypeVariable;

public class Test {
    public TypeVariable<Class<String>>[] getTypeParameters() {
        return null;
    }
}

➜  src java -jar checkstyle-8.43-all.jar -t Test.javav
Files to process must be specified, found 0.

➜  src java -jar checkstyle-8.44-SNAPSHOT-all.jar -t Test.java 
IMPORT -> import [1:0]
|--DOT -> . [1:24]
|   |--DOT -> . [1:16]
|   |   |--DOT -> . [1:11]
|   |   |   |--IDENT -> java [1:7]
|   |   |   `--IDENT -> lang [1:12]
|   |   `--IDENT -> reflect [1:17]
|   `--IDENT -> TypeVariable [1:25]
`--SEMI -> ; [1:37]
CLASS_DEF -> CLASS_DEF [3:0]
|--MODIFIERS -> MODIFIERS [3:0]
|   `--LITERAL_PUBLIC -> public [3:0]
|--LITERAL_CLASS -> class [3:7]
|--IDENT -> Test [3:13]
`--OBJBLOCK -> OBJBLOCK [3:18]
    |--LCURLY -> { [3:18]
    |--METHOD_DEF -> METHOD_DEF [4:4]
    |   |--MODIFIERS -> MODIFIERS [4:4]
    |   |   `--LITERAL_PUBLIC -> public [4:4]
    |   |--TYPE -> TYPE [4:11]
    |   |   |--IDENT -> TypeVariable [4:11]
    |   |   `--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:23]
    |   |       |--GENERIC_START -> < [4:23]
    |   |       |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:24]
    |   |       |   |--IDENT -> Class [4:24]
    |   |       |   |--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:29]
    |   |       |   |   |--GENERIC_START -> < [4:29]
    |   |       |   |   |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:30]
    |   |       |   |   |   |--IDENT -> String [4:30]
    |   |       |   |   |   `--ARRAY_DECLARATOR -> [ [4:36] // Correct
    |   |       |   |   |       `--RBRACK -> ] [4:37]
    |   |       |   |   `--GENERIC_END -> > [4:38]
    |   |       |   `--ARRAY_DECLARATOR -> [ [4:40] // Incorrect
    |   |       |       `--RBRACK -> ] [4:41]
    |   |       `--GENERIC_END -> > [4:39]
    |   |--IDENT -> getTypeParameters [4:43]
    |   |--LPAREN -> ( [4:60]
    |   |--PARAMETERS -> PARAMETERS [4:61]
    |   |--RPAREN -> ) [4:61]
    |   `--SLIST -> { [4:63]
    |       |--LITERAL_RETURN -> return [5:8]
    |       |   |--EXPR -> EXPR [5:15]
    |       |   |   `--LITERAL_NULL -> null [5:15]
    |       |   `--SEMI -> ; [5:19]
    |       `--RCURLY -> } [6:4]
    `--RCURLY -> } [7:0]

The [] that follow the type arguments of the getTypeParameters() method are part of the return type of the method itself, and should be located in the method declaration's TYPE subtree.
Example:

IMPORT -> import [1:0]
|--DOT -> . [1:24]
|   |--DOT -> . [1:16]
|   |   |--DOT -> . [1:11]
|   |   |   |--IDENT -> java [1:7]
|   |   |   `--IDENT -> lang [1:12]
|   |   `--IDENT -> reflect [1:17]
|   `--IDENT -> TypeVariable [1:25]
`--SEMI -> ; [1:37]
CLASS_DEF -> CLASS_DEF [3:0]
|--MODIFIERS -> MODIFIERS [3:0]
|   `--LITERAL_PUBLIC -> public [3:0]
|--LITERAL_CLASS -> class [3:7]
|--IDENT -> Test [3:13]
`--OBJBLOCK -> OBJBLOCK [3:18]
    |--LCURLY -> { [3:18]
    |--METHOD_DEF -> METHOD_DEF [4:4]
    |   |--MODIFIERS -> MODIFIERS [4:4]
    |   |   `--LITERAL_PUBLIC -> public [4:4]
    |   |--TYPE -> TYPE [4:11]
    |   |   |--IDENT -> TypeVariable [4:11]
    |   |   |--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:23]
    |   |   |   |--GENERIC_START -> < [4:23]
    |   |   |   |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:24]
    |   |   |   |   |--IDENT -> Class [4:24]
    |   |   |   |   `--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:29]
    |   |   |   |       |--GENERIC_START -> < [4:29]
    |   |   |   |       |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:30]
    |   |   |   |       |   |--IDENT -> String [4:30]
    |   |   |   |       |   `--ARRAY_DECLARATOR -> [ [4:36] // Correct
    |   |   |   |       |       `--RBRACK -> ] [4:37]
    |   |   |   |       `--GENERIC_END -> > [4:38]
    |   |   |   `--GENERIC_END -> > [4:39]
    |   |   `--ARRAY_DECLARATOR -> [ [4:40]     // Correct
    |   |       `--RBRACK -> ] [4:41]             
    |   |--IDENT -> getTypeParameters [4:43]
    |   |--LPAREN -> ( [4:60]
    |   |--PARAMETERS -> PARAMETERS [4:61]
    |   |--RPAREN -> ) [4:61]
    |   `--SLIST -> { [4:63]
    |       |--LITERAL_RETURN -> return [5:8]
    |       |   |--EXPR -> EXPR [5:15]
    |       |   |   `--LITERAL_NULL -> null [5:15]
    |       |   `--SEMI -> ; [5:19]
    |       `--RCURLY -> } [6:4]
    `--RCURLY -> } [7:0]
@pbludov
Copy link
Member

pbludov commented Jun 26, 2021

This is a bug. Such tree should be created for something like

    public TypeVariable<Class<String>[]> getTypeParameters()

interesting that for

    public TypeVariable<Class<String[]>[]>[] getTypeParameters()

the AST is correct:

    |--METHOD_DEF -> METHOD_DEF [4:4]
    |   |--MODIFIERS -> MODIFIERS [4:4]
    |   |   |--LITERAL_PUBLIC -> public [4:4]
    |   |   `--ABSTRACT -> abstract [4:11]
    |   |--TYPE -> TYPE [4:20]
    |   |   |--IDENT -> TypeVariable [4:20]
    |   |   |--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:32]
    |   |   |   |--GENERIC_START -> < [4:32]
    |   |   |   |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:33]
    |   |   |   |   |--IDENT -> Class [4:33]
    |   |   |   |   |--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:38]
    |   |   |   |   |   |--GENERIC_START -> < [4:38]
    |   |   |   |   |   |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:39]
    |   |   |   |   |   |   |--IDENT -> String [4:39]
    |   |   |   |   |   |   `--ARRAY_DECLARATOR -> [ [4:45]
    |   |   |   |   |   |       `--RBRACK -> ] [4:46]
    |   |   |   |   |   `--GENERIC_END -> > [4:47]
    |   |   |   |   `--ARRAY_DECLARATOR -> [ [4:48]
    |   |   |   |       `--RBRACK -> ] [4:49]
    |   |   |   `--GENERIC_END -> > [4:50]
    |   |   `--ARRAY_DECLARATOR -> [ [4:51]
    |   |       `--RBRACK -> ] [4:52]
    |   |--IDENT -> getTypeParameters [4:54]
    |   |--LPAREN -> ( [4:71]
    |   |--PARAMETERS -> PARAMETERS [4:72]
    |   |--RPAREN -> ) [4:72]

@nrmancuso
Copy link
Member Author

Fixed via #10280:

 ➜  src cat Test.java
import java.lang.reflect.TypeVariable;

public class Test {
    public TypeVariable<Class<String>>[] getTypeParameters() {
        return null;
    }
}

➜  src java -jar ~/IdeaProjects/checkstyle/target/checkstyle-9.0-SNAPSHOT-all.jar -t Test.java
IMPORT -> import [1:0]
|--DOT -> . [1:24]
|   |--DOT -> . [1:16]
|   |   |--DOT -> . [1:11]
|   |   |   |--IDENT -> java [1:7]
|   |   |   `--IDENT -> lang [1:12]
|   |   `--IDENT -> reflect [1:17]
|   `--IDENT -> TypeVariable [1:25]
`--SEMI -> ; [1:37]
CLASS_DEF -> CLASS_DEF [3:0]
|--MODIFIERS -> MODIFIERS [3:0]
|   `--LITERAL_PUBLIC -> public [3:0]
|--LITERAL_CLASS -> class [3:7]
|--IDENT -> Test [3:13]
`--OBJBLOCK -> OBJBLOCK [3:18]
    |--LCURLY -> { [3:18]
    |--METHOD_DEF -> METHOD_DEF [4:4]
    |   |--MODIFIERS -> MODIFIERS [4:4]
    |   |   `--LITERAL_PUBLIC -> public [4:4]
    |   |--TYPE -> TYPE [4:11]
    |   |   |--IDENT -> TypeVariable [4:11]
    |   |   |--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:23]
    |   |   |   |--GENERIC_START -> < [4:23]
    |   |   |   |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:24]
    |   |   |   |   |--IDENT -> Class [4:24]
    |   |   |   |   `--TYPE_ARGUMENTS -> TYPE_ARGUMENTS [4:29]
    |   |   |   |       |--GENERIC_START -> < [4:29]
    |   |   |   |       |--TYPE_ARGUMENT -> TYPE_ARGUMENT [4:30]
    |   |   |   |       |   `--IDENT -> String [4:30]
    |   |   |   |       `--GENERIC_END -> > [4:36]
    |   |   |   `--GENERIC_END -> > [4:37]
    |   |   `--ARRAY_DECLARATOR -> [ [4:38]
    |   |       `--RBRACK -> ] [4:39]
    |   |--IDENT -> getTypeParameters [4:41]
    |   |--LPAREN -> ( [4:58]
    |   |--PARAMETERS -> PARAMETERS [4:59]
    |   |--RPAREN -> ) [4:59]
    |   `--SLIST -> { [4:61]
    |       |--LITERAL_RETURN -> return [5:8]
    |       |   |--EXPR -> EXPR [5:15]
    |       |   |   `--LITERAL_NULL -> null [5:15]
    |       |   `--SEMI -> ; [5:19]
    |       `--RCURLY -> } [6:4]
    `--RCURLY -> } [7:0]


@strkkk strkkk closed this as completed Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants