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

OpenJDK 21 Syntax Related Check Updates #14444

Open
nrmancuso opened this issue Feb 7, 2024 · 8 comments
Open

OpenJDK 21 Syntax Related Check Updates #14444

nrmancuso opened this issue Feb 7, 2024 · 8 comments
Assignees

Comments

@nrmancuso
Copy link
Member

nrmancuso commented Feb 7, 2024

Now that we are close to full support of Java 21 syntax, we need to evaluate a few things:

  1. Checks that should be updated to support new tokens
  2. Checks that need to be updated to avoid false positive/negative related to changes in child tokens, etc
  3. Gaps in check coverage, i.e. creation of new checks

Really, we have mostly relied on the community to report false positive/negatives, and have not done a thorough analysis of our checks since a few java versions ago (during GSOC 2021).

Over the next few weeks, I will be periodically updating this issue. @rdiachenko @Vyom-Yadav @romani @rnveach you can drop any comments in here about ideas you have and I will consolidate them into the issue description.

For Unnamed variables:

  • IllegalIdentitifer - we need to reconsider all naming checks
  • PatternVariableName
  • Indentation
  • IllegalToken
  • NoWhitespace After/Before/Around
@rnveach
Copy link
Member

rnveach commented Feb 9, 2024

For unnamed variables:

One check needs to be Indentation. It has to execute on all tokens otherwise things are "hidden" from it. IllegalToken should support it, if it doesn't already.

NoWhitespace After/Before should be considered. Maybe WhitespaceAround should support it.

@romani romani changed the title [WIP] OpenJDK 21 Syntax Related Check Updates OpenJDK 21 Syntax Related Check Updates Feb 9, 2024
@apflieger
Copy link

PatternVariableName doesn't allow unnamed variables by default.

@romani
Copy link
Member

romani commented Feb 14, 2024

We unlikely to change default behavior to keep compatibility with previous behavior. Please feel free to customize allowed pattern in your config. If think that it is very reasonable to have it by default , please create separate issue and share all details there.

@nrmancuso
Copy link
Member Author

nrmancuso commented Feb 17, 2024

We unlikely to change default behavior to keep compatibility with previous behavior.

I tend to disagree here. We need to evolve, checks should reflect today's best practices and styles by default, not best practices from 10/15/20 years ago. Checkstyle should be leading the community in best practices.

Imagine you are a brand new user with a Java 21 project; you will need to go through a bunch of pain to configure your checks to follow modern practices. I would expect the latest version of any tool to support modern Java paradigms, with up to date syntax and and contemporary style.

On the other hand, I would also expect to have to do some minor updates to my config over the lifetime of a 15 year old project.

@romani
Copy link
Member

romani commented Feb 18, 2024

@nrmancuso or @apflieger, please create new issue, to discuss exact code and check and reasoning of changing default.

We traditionalally keep compatibility, rather than change behavior in new releases. But in new issue it will be a way better see all details and vote on exact update.

@nrmancuso
Copy link
Member Author

Yup, this is issue is just a convenient place to easily drop ideas. We will analyse what we come up with here and generate comprehensive issues when the time is right.

@nrmancuso
Copy link
Member Author

Another issue that could be considered for this: #14825

@nrmancuso
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants