Add Check Support for Java 21 Pattern Matching for Switch Syntax: JavaNCSS

[mahfouz72]

child of #14961

I have read check documentation:https://checkstyle.org/checks/metrics/javancss.html#JavaNCSS
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

---

check doc:

Roughly said the NCSS metric is calculated by counting the source lines which are not comments, (nearly) equivalent to counting the semicolons and opening curly braces.

---

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="JavaNCSS">
            <property name="methodMaximum" value="1"/>
        </module>
    </module>
</module>
PS D:\CS\test> cat src/Test.java                                                
import java.awt.color.ICC_ColorSpace;

record ColoredPoint(int p, int x, boolean c) {
}
record Rectangle(ColoredPoint upperLeft, ColoredPoint lowerRight) {
}

public class Test {

    // this is 5 as expected
    void test(Object obj) {  // opening braces +1
        switch (obj) {   // opening braces +1
            case ColoredPoint(int a, int b, _)
                    when (a >= b) ->
            { // opening braces +1

            }
            case ColoredPoint(int a, int b, _) -> {}   // opening braces +1
            default -> System.out.println("none");   // semi colon +1
        }
    }

    // this is 6, shouldn't be 4  ?
    void test2(Object obj) { // opening braces +1
        if (obj instanceof ColoredPoint(int a, int b, _)
                && a >= b)
        {   // opening braces +1

        }
        else if (obj instanceof ColoredPoint(int a, int b, _)) { }  // opening braces +1
        else System.out.println("none"); // semi colon +1
    }
}
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:11:5: NCSS for this method is 5 (max allowed is 1). [JavaNCSS]
[ERROR] D:\CS\test\src\Test.java:24:5: NCSS for this method is 6 (max allowed is 1). [JavaNCSS]
Audit done.
Checkstyle ends with 2 errors.
PS D:\CS\test> 

---

Describe what you want in detail

I think the code for the switch is working as expected here. There may be a bug with the code with if statments

[mahfouz72]

this has nothing to do with the new syntax. But this check is weird in counting I can't even get the correct behavior.

the difference between the two methods is just removing one line ( else ). but the JavaNCSS dropped by 2.

PS D:\CS\test> cat src/Test.java                                                
record ColoredPoint(int p, int x, boolean c) { }
record Rectangle(ColoredPoint upperLeft, ColoredPoint lowerRight) { }
public class Test {
    
    void test2(Object obj) {
        if (obj instanceof ColoredPoint(int a, int b, _)
                && a >= b)
        {  

        }
        else if (obj instanceof ColoredPoint(int a, int b, _)) { }
        else System.out.println("none");
    }

    void test3(Object obj) { 
        if (obj instanceof ColoredPoint(int a, int b, _)
                && a >= b)
        {  

        }
        else if (obj instanceof ColoredPoint(int a, int b, _)) { }  
    }
}
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:5: NCSS for this method is 6 (max allowed is 1). [JavaNCSS]
[ERROR] D:\CS\test\src\Test.java:15:5: NCSS for this method is 4 (max allowed is 1). [JavaNCSS]
Audit done.
Checkstyle ends with 2 errors.
PS D:\CS\test> 

[nrmancuso]

I am approving this for new test input only. I also struggle to understand exactly how this check actually works vs how it is supposed to work. If we don't find anything crazy during our PR to add new tests inputs, I suggest that we don't spend a lot of time on this check.

[rnveach]

how it is supposed to work
Roughly said the NCSS metric is calculated by counting the source lines which are not comments, (nearly) equivalent to counting the semicolons and opening curly braces.

It seems to me this check is suppose to cover something like "How complex can a method without comments be before it becomes too complex and you must comment it and describe the complexity".

However, I may disagree that it goes far enough since it doesn't count boolean operators like && and ||.

the difference between the two methods is just removing one line ( else ). but the JavaNCSS dropped by 2.
else System.out.println("none");

https://checkstyle.org/checks/metrics/javancss.html#Example1-code
Expressions are counted as 1. I am assuming the else also counts as 1. So 1+1 = 2, which explains why it drops by 2.

===

I thought there was some issue with this on switches, but maybe it was fixed.

I am no expert with this check.

Here is what the input in the first issue is counting (subtract 10 from the line numbers below to match the above):

import[11x0] - 1
RECORD_DEF[13x0] - 1
RECORD_DEF[15x0] - 1
CLASS_DEF[18x0] - 1
METHOD_DEF[21x4] - 1
switch[22x8] - 2
case[23x12] - 3
case[28x12] - 4
default[29x12] - 5
METHOD_DEF[34x4] - 1
if[35x8] - 2
else[40x8] - 3
if[40x13] - 4
else[41x8] - 5
EXPR[41x31] - 6

We do know when may not be calculated correctly from #15044 (comment) since we are missing an expression. This alone may be a reason to delay verifying this check.

// this is 5 as expected

when has an expression, however we don't count expressions if they are put of an if. We already count the if itself.

I think this may cause a bug in this check when we fix when.

// this is 6, shouldn't be 4 ?

Now that I see what is counted. It seems odd we add +2 for an else if.

http://www.kclee.de/clemens/java/javancss/
This mentions } else { but not } else if (...) {. I wonder if we just see it as } else { if (...) { or if this isn't correct.

[rnveach]

default[29x12] - 5
METHOD_DEF[34x4] - 1
...
else[41x8] - 5
EXPR[41x31] - 6

There is a disconnect here between the 2 methods.

default -> System.out.println("none"); +1 count
else System.out.println("none"); +2 count

We should be counting the expression inside the default ->.

This looks like a bug, possibly with the SWITCH_RULE?

@nrmancuso We may need to re-evaluate this being an input only issue.