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.

Sign up for GitHub

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

Jump to bottom

Add Check Support for Java 21 Unnamed Variables & Patterns Syntax: WhitespaceAfter #15057

Closed
mahfouz72 opened this issue Jun 20, 2024 · 3 comments · Fixed by #15447
Closed

Add Check Support for Java 21 Unnamed Variables & Patterns Syntax: WhitespaceAfter #15057

mahfouz72 opened this issue Jun 20, 2024 · 3 comments · Fixed by #15447
Assignees
@mahfouz72
Labels
approved input update miscellaneous
Milestone
10.18.0

Comments

@mahfouz72
Copy link
Member

mahfouz72 commented Jun 20, 2024 •
edited
Loading

child of #14942

I have read check documentation: https://checkstyle.sourceforge.io/checks/whitespace/whitespaceafter.html#WhitespaceAfter
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

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="WhitespaceAfter">
        </module>
    </module>
</module>
PS D:\CS\test> cat src/Test.java                                                
import java.util.List;

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

public class Test {
    void test(Object obj) {

        switch (obj) {
            case ColoredPoint(int x, _, int c) when c == 0 -> { // expected violation

            }
            case ColoredPoint(int x, _ , int c) -> {}
            default -> { }
        }
    }
}

PS D:\CS\test> java  -jar checkstyle-10.17.0-all.jar -c config.xml src/Test.java
Starting audit...
Audit done.
PS D:\CS\test>

Describe what you want in detail.

I think the check should support UNNAMED_PATTERN_DEF. IMO, it is visually cleaner when we have a space after the underscore. do you think it should be no white space as someone suggested before?
@mahfouz72 mahfouz72 mentioned this issue Jun 20, 2024
Closed
47 tasks
@rnveach
Copy link
Member

rnveach commented Jun 22, 2024 •
edited
Loading

List<String> Xs = list.stream().map(_-> "x").toList(); // expected violation
List<String> Xss = list.stream().map(_ -> "x").toList();

I agree with this violation, but this is more because there should be whitespaces around the -> and not because of the _. I would want this regardless of the variable name, or unnamed. If we don't support this for Lambda, then I think this should be another issue.

case ColoredPoint(int x, _, int c) when c == 0 -> { // expected violation

I will say at first this seems odd to me, but without any type maybe it makes some sense? I would have to code more on this to know for sure.

do you think it should be no white space as someone suggested before?

I more leaning to this.

Parameter names or anything variable name related are not supported in this check currently. This issue seems to me like an odd leap for this check.

Do we have any best practices that say this way is better?

@nrmancuso What is your thoughts?

@nrmancuso
Copy link
Member

Let's revisit this one later on.

@nrmancuso nrmancuso moved this from Needs Triage to For Later in Java 21 Language Features Support Jun 23, 2024
@nrmancuso nrmancuso removed their assignment Jun 23, 2024
@mahfouz72 mahfouz72 mentioned this issue Jul 10, 2024
Closed
@mahfouz72 mahfouz72 mentioned this issue Jul 21, 2024
Closed
@nrmancuso
Copy link
Member

We should rely on the surrounding tokens for checking whitespace, conceptually this is no different than any other identifier. I am approving this for a new test case only.

@nrmancuso nrmancuso moved this from For Later to Todo in Java 21 Language Features Support Aug 3, 2024
mahfouz72 added a commit to mahfouz72/checkstyle that referenced this issue Aug 4, 2024
@mahfouz72
Issue checkstyle#15057: Add new input for unnamed pattern in whitespa…
66037a4 
…ceAfterCheck
@mahfouz72 mahfouz72 mentioned this issue Aug 4, 2024
Merged
@mahfouz72 mahfouz72 moved this from Todo to In Progress in Java 21 Language Features Support Aug 4, 2024
nrmancuso pushed a commit that referenced this issue Aug 6, 2024
@mahfouz72 @nrmancuso
Issue #15057: Add new input for unnamed pattern in whitespaceAfterCheck
5d8743c
@github-project-automation github-project-automation bot moved this from In Progress to Done in Java 21 Language Features Support Aug 6, 2024
@github-actions github-actions bot added this to the 10.18.0 milestone Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mahfouz72 mahfouz72

Labels
approved input update miscellaneous
Projects
No open projects
Java 21 Language Features Support
Status: Done
Milestone
10.18.0
Development

Successfully merging a pull request may close this issue.

Issue #15057: Add new input for unnamed pattern in whitespaceAfterCheck mahfouz72/checkstyle
3 participants
@rnveach @nrmancuso @mahfouz72