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

LITERAL_CASE token support in RightCurlyCheck #14620

Closed
mahfouz72 opened this issue Mar 6, 2024 · 4 comments · Fixed by #14623
Closed

LITERAL_CASE token support in RightCurlyCheck #14620

mahfouz72 opened this issue Mar 6, 2024 · 4 comments · Fixed by #14623

Comments

@mahfouz72
Copy link
Member

mahfouz72 commented Mar 6, 2024

Currently RightCurlyCheck accepts a less tokens than LeftCurlyCheck (19 tokens)
This issue aims to add support for LITERAL_CASE token in RightCurlyCheck
this issue is a child issue for #3547

For LeftCurlyCheck:

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="LeftCurly">
            <property name="tokens" value="LITERAL_CASE "/>
          <property name="option" value="eol"/>
        </module>
    </module>
</module>
PS D:\test> cat src/Test.java                                                
class Test{

    void test(){
        int x = 1;
        switch(x){
            case 1:
            {        // violation
                System.out.println("x is 1");
                break;
            }
            case 2:
            {     // violation
                System.out.println("x is 2");
                break;
            }
            default:
                System.out.println("x is neither 1 nor 2");
        }
    }
}
PS D:\test> java  -jar checkstyle-10.13.0-all.jar -c config.xml src/Test.java
Starting audit...
[ERROR] D:\test\src\Test.java:7:13: '{' at column 13 should be on the previous line. [LeftCurly]
[ERROR] D:\test\src\Test.java:12:13: '{' at column 13 should be on the previous line. [LeftCurly]
Audit done.
Checkstyle ends with 2 errors.

for RightCurlyCheck:

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="RightCurly">
            <property name="tokens" value="LITERAL_CASE"/>
          <property name="option" value="alone"/>
        </module>
    </module>
</module>
PS D:\test> cat src/Test.java                                                
class Test{

    void test(){
        int x = 1;
        switch(x){
            case 1:
            {
                System.out.println("x is 1");
                break;
            } case 2:                       // expected violation '}' should be alone in the line
            {
                System.out.println("x is 2");
                break;
            } default:                        // expected violation '}' should be alone in the line
                System.out.println("x is neither 1 nor 2");
        }
    }
}

Expected:- Violation for the right curly brace for LITERAL_CASE

@nrmancuso
Copy link
Member

@mahfouz72 thanks for your report, issue is approved.

Tests for this need to include:

  • Different inputs with all RightCurlyOptions
  • switch statements, expressions
  • switch rules and block statements

@mahfouz72
Copy link
Member Author

I am on it

@mahfouz72
Copy link
Member Author

switch statements, expressions

@nrmancuso but I found in the check code that the check is handling statements only not expressions so do you mean to add tests to show that it isn't getting checked?

@nrmancuso
Copy link
Member

switch statements, expressions

@nrmancuso but I found in the check code that the check is handling statements only not expressions so do you mean to add tests to show that it isn't getting checked?

@mahfouz72 i like to think about issues vs PRs (generally) like this: in issues, we talk about behavior, and in PRs, we talk about code.

As far as behavior goes, I would expect braces for the SLIST of any LITERAL_CASE to be checked, regardless of where this token appears.

If RightCurlyCheck doesn’t check the closing right curly brace of a switch expression, this sounds like a different (but valid) issue.

mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 7, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 7, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 7, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 8, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 9, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 9, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 9, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 9, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 9, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 10, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 10, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 10, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 10, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 10, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 16, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 16, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 16, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 16, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 16, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 16, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Mar 30, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 3, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 8, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 8, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 8, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 8, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 8, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 15, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Apr 15, 2024
@github-actions github-actions bot added this to the 10.15.1 milestone Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants