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

Add Check Support for Java 21 Pattern Matching for Switch Syntax: CyclomaticComplexity #15045

Open
mahfouz72 opened this issue Jun 20, 2024 · 2 comments · May be fixed by #15341
Open

Add Check Support for Java 21 Pattern Matching for Switch Syntax: CyclomaticComplexity #15045

mahfouz72 opened this issue Jun 20, 2024 · 2 comments · May be fixed by #15341
Assignees
Labels
approved false negative issues where check should place violations on code, but does not

Comments

@mahfouz72
Copy link
Member

mahfouz72 commented Jun 20, 2024

child of #14961

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


➜  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="CyclomaticComplexity">
            <property name="max" value="3"/>
        </module>
    </module>
</module>
➜  src cat 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 {

    // violation,  Cyclomatic Complexity is 4
    void test(Object obj) {  // 1, function declaration
        switch (obj) {
            case ColoredPoint(int a, int b, _)  // 2 , case
                    when (a >= b) -> {        // should we add 1 for when ? it was literally a `&&` before
            }
            case ColoredPoint(int a, int b, _) -> {}  // 3, case
            case Rectangle(_, _) -> {}  // 4, case
            default -> System.out.println("none");
        }
    }

    // violation,  Cyclomatic Complexity is 5
    void test2(Object obj) {  // 1, function declaration
        // I interpreted `when` as `&&`
        if (obj instanceof ColoredPoint(int a, int b, _) && a >= b) { // 2, if and 3, '&&'

        } else if (obj instanceof ColoredPoint(int a, int b, _)) { // 4 if

        } else if (obj instanceof Rectangle(_, _)) {   // 5 if

        }
    }
}
➜  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:11:5: Cyclomatic Complexity is 4 (max allowed is 3). [CyclomaticComplexity]
[ERROR] /home/nick/IdeaProjects/tester/src/Test.java:23:5: Cyclomatic Complexity is 5 (max allowed is 3). [CyclomaticComplexity]
Audit done.
Checkstyle ends with 2 errors.


Describe what you want in detail

Since we add 1 for &&. I think it makes sense that when should be considered as +1 here. It is just the same as && in if conditions see the similarities between the two examples

@nrmancuso
Copy link
Member

We will add support for https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_WHEN as a default token, and count this token as +1 as recommended by @mahfouz72 .

I am approving this issue.

@nrmancuso
Copy link
Member

Ok, I have updated the OP, result is the same as before we had a nested EXPR token. Re-approving for same conditions from #15045 (comment)

@nrmancuso nrmancuso added false negative issues where check should place violations on code, but does not approved labels Jul 9, 2024
@nrmancuso nrmancuso assigned mahfouz72 and unassigned nrmancuso Jul 9, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Jul 22, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Jul 22, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Jul 25, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Jul 25, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Jul 27, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved false negative issues where check should place violations on code, but does not
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants