checkstyle
checkstyle copied to clipboard
IllegalIdentifierName should be updated to support new Java 22 keywords
Identified at https://github.com/checkstyle/checkstyle/pull/15084#discussion_r1649748634 ,
https://checkstyle.org/checks/naming/illegalidentifiername.html#IllegalIdentifierName
Checks identifiers with a pattern for a set of illegal names, such as those that are restricted or contextual keywords.
JLS Keywords as of Java 22: https://docs.oracle.com/javase/specs/jls/se22/html/jls-3.html#jls-3.9
$ cat TestClass.java
public class Test {
public void test() {
int exports = 0;
int opens = 0;
int requires = 0;
int uses = 0;
int yield = 0; // violation
int module = 0;
int permits = 0; // violation
int sealed = 0; // violation
int var = 0; // violation
// int non-sealed = 0; // doesn't compile
int provides = 0;
int to = 0;
int when = 0;
int open = 0;
int record = 0; // violation
int transitive = 0;
int with = 0;
}
}
PS> C:\Programs\jdk-22.0.1\bin\javac.exe .\TestClass.java
PS>
$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
<property name="charset" value="UTF-8"/>
<property name="severity" value="warning"/>
<property name="haltOnException" value="false"/>
<module name="TreeWalker">
<module name="IllegalIdentifierName"/>
</module>
</module>
$ java -jar checkstyle-10.17.0-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[WARN] TestClass.java:7:5: Name 'yield' must match pattern '(?i)^(?!(record|yield|var|permits|sealed|_)$).+$'. [IllegalIdentifierName]
[WARN] TestClass.java:9:5: Name 'permits' must match pattern '(?i)^(?!(record|yield|var|permits|sealed|_)$).+$'. [IllegalIdentifierName]
[WARN] TestClass.java:10:5: Name 'sealed' must match pattern '(?i)^(?!(record|yield|var|permits|sealed|_)$).+$'. [IllegalIdentifierName]
[WARN] TestClass.java:11:5: Name 'var' must match pattern '(?i)^(?!(record|yield|var|permits|sealed|_)$).+$'. [IllegalIdentifierName]
[WARN] TestClass.java:17:5: Name 'record' must match pattern '(?i)^(?!(record|yield|var|permits|sealed|_)$).+$'. [IllegalIdentifierName]
Audit done.
I am expecting violations on the other local variables not commented out. The JLS defines them as Contextual Keywords and our documentation clearly says these are targeted by this check.
Documentation should be updated. We will only support the latest keywords and users can apply an override to the property to revert to older Java version keywords (example at https://github.com/checkstyle/checkstyle/pull/15117#discussion_r1654849384 ).
We should also extend our check update docs to always consider this check when new keywords are added to the language.
We should also extend our check update docs to always consider this check when new keywords are added to the language.
Identified at https://github.com/checkstyle/checkstyle/pull/15117#discussion_r1654849384 , we should also document that we will only support the latest keywords and users can apply an override to the property to revert to older Java version keywords. I will also update first post.
I am expecting violations on the other local variables not commented out. The JLS defines them as Contextual Keywords and our documentation clearly says these are targeted by this check.
Do we need to add all of them in a single PR?
I am removing "approval" label until we agree on it. we can update examples to show user how to violate names is context where it is confusing - https://github.com/checkstyle/checkstyle/pull/15486#discussion_r1713058911
We need to rethink this Check, something wrong in trend to forbid general words.
record is good word and used a lot, especially in DB related applications
@romani Please open this issue you talk about removing this check and having something else in it place.
https://github.com/checkstyle/checkstyle/issues/15499