Add Check Support for Java 21 Unnamed Variables & Patterns Syntax: AnnotationOnSameLineCheck

[mahfouz72]

child of #14942
Check documentation: https://checkstyle.org/checks/annotation/annotationonsameline.html#AnnotationOnSameLine

PS D:\CS\test> javac src/Test.java
PS D:\CS\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="AnnotationOnSameLine">
        </module>
    </module>
</module>
PS D:\CS\test> cat src/Test.java  
record ColoredPoint(int p, int x, String c) { }
record Rectangle(ColoredPoint upperLeft, ColoredPoint lowerRight) { }
public class Test {
    void test(Object obj) {
        @Deprecated
        int _ = 0;

        if (obj instanceof ColoredPoint(@Deprecated // expected violation
                                          int _, _,_)){
        }
    }
}
PS D:\CS\test> java  -jar checkstyle-10.17.0-all.jar -c config.xml src/Test.java
Starting audit...
[ERROR] D:\CS\test\src\Test.java:5:9: Annotation 'Deprecated' should be on the same line with its target. [AnnotationOnSameLine]
Audit done.
Checkstyle ends with 1 errors.
PS D:\CS\test> 

---

This is not very specific to unnamed patterns. but for patterns in general.
I expect a violation on the annotations on the pattern variable. The annotation should be on the same line as the pattern. We should add PATTERN_VARIABLE_DEF in the tokens

[rnveach]

if (obj instanceof ColoredPoint( @Deprecated int _, _,_)){

Show a violation on this line if possible.

I am ok to add an input if there is no concern.

[mahfouz72]

PATTERN_VARAIABLE_DEF is not in the acceptable tokens of this check.

---

PS D:\CS\test> cat src/Test.java                                                
record ColoredPoint(int p, int x, String c) { } 
record Rectangle(ColoredPoint upperLeft, ColoredPoint lowerRight) { }  
public class Test {
    void test(Object obj) {
        if (obj instanceof ColoredPoint(@Deprecated // should this be violation ?
                                          int _, _,_)){
        }
        @Deprecated
        int _ = 0;
    }
}
PS D:\CS\test> java  -jar checkstyle-10.17.0-all.jar -c config.xml src/Test.java
Starting audit...
[ERROR] D:\CS\test\src\Test.java:8:9: Annotation 'Deprecated' should be on the same line with its target. [AnnotationOnSameLine]
Audit done.
Checkstyle ends with 1 errors.
PS D:\CS\test> 

---

I guess this should be ok this is all technically in the same if condition I think there is no point in requiring it to be on the same line

[rnveach]

@Deprecated
int _ = 0;
8:9: Annotation 'Deprecated' should be on the same line with its target.

This seems to cover local variables but all examples look to be class/javadoc level related.

@Deprecated // should this be violation ?
int _, _,_)){

To me it seems we need to support this and print a violation. These pattern variables are almost a combination of local variables and field variables.

@mahfouz72 Please add all examples to first post.

[mahfouz72]

done first post is updated

[nrmancuso]

This should not be a violation. This is a style check, so we should follow logic similar to conceptually/structurally similar code (in record component list in record definitions, constructors definition parameters, method definition parameters).

Example:

➜  src 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="AnnotationOnSameLine">
        </module>
    </module>
</module>
➜  src cat Test.java                                

record ColoredPoint(@Deprecated int p, int x, String c) { }
record ColoredPointOther(@Deprecated
                         int p, int x, String c) { }

public class Test {

    Test(@Deprecated int x) {}
    Test(@Deprecated
         String s) {}

    void m1(@Deprecated int x) {}
    void m2(@Deprecated
            int x) {}

    void test(Object obj) {
        @Deprecated
        int _ = 0;

        if (obj instanceof ColoredPoint(@Deprecated
                                        int _, _,_)){
        }

        if (obj instanceof ColoredPoint(@Deprecated int _, _,_)){
        }
    }
}
➜  src javac --enable-preview --release 21 Test.java                        
Note: Test.java uses preview features of Java SE 21.
Note: Recompile with -Xlint:preview for details.
➜  src java -jar checkstyle-10.18.1-SNAPSHOT-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /home/nick/IdeaProjects/tester/src/Test.java:17:9: Annotation 'Deprecated' should be on the same line with its target. [AnnotationOnSameLine]
Audit done.
Checkstyle ends with 1 errors.

I am approving for new test case only.

[rnveach]

@nrmancuso

This is a style check, so we should follow logic similar to conceptually/structurally similar code (in record component list in record definitions, constructors definition parameters, method definition parameters

So you are saying this should be treated similarly to parameters in a method and not as a local variable?

[nrmancuso]

If this check doesn't place violations on record components, it shouldn't place them on record component patterns. Not only are these constructs conceptually similar, but in the case of a style check (where we care about aesthetics), it is good enough that they look similar.

If we are talking about coding checks, then we should probably consider these to be local variables in most cases.

[rnveach]

By record components you are referring to record ColoredPoint and record ColoredPointOther, which looks like line 2 and 3 in your example?

[nrmancuso]

By record components you are referring to record ColoredPoint and record ColoredPointOther, which looks like line 2 and 3 in your example?

See https://docs.oracle.com/javase/specs/jls/se21/html/jls-8.html#jls-8.10.1

[nrmancuso]

I merged the PR for this issue without realizing that @rnveach was in the review queue, I am reopening this issue to make sure that we have resolved all concerns; @rnveach please close this issue or let us know what else we need to do here.

[rnveach]

I am good.