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

Record inside interface is treated as method #10978

Closed
eorloff opened this issue Nov 25, 2021 · 3 comments · Fixed by #10979
Closed

Record inside interface is treated as method #10978

eorloff opened this issue Nov 25, 2021 · 3 comments · Fixed by #10979

Comments

@eorloff
Copy link
Contributor

eorloff commented Nov 25, 2021

I have read check documentation: https://checkstyle.org/config_naming.html#MethodName
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

$ javac Test.java

$ cat Test.java
public interface Test {
  record MyRecord(int value) {};
}

$ cat config.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">
  <module name="TreeWalker">
      <module name="MethodName"/>
  </module>
</module>

$ RUN_LOCALE="-Duser.language=en -Duser.country=US"
$ java $RUN_LOCALE -jar checkstyle-9.1-all.jar -c config.xml Test.java
Starting audit...
[ERROR] C:\Users\ebo\src\tmp\checkstyle-issue\Test.java:2:10: Name 'MyRecord' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MethodName]
Audit done.
Checkstyle ends with 1 errors.

Expected no errors, but the name of the record is marked as an illegal method name.

If interface Test is replaced with class Test then the check goes through without errors.

@strkkk
Copy link
Member

strkkk commented Nov 25, 2021

I reproduced problem.
@nmancus1 AST for this case looks like

COMPILATION_UNIT -> COMPILATION_UNIT [1:0]
`--INTERFACE_DEF -> INTERFACE_DEF [1:0]
    |--MODIFIERS -> MODIFIERS [1:0]
    |   `--LITERAL_PUBLIC -> public [1:0]
    |--LITERAL_INTERFACE -> interface [1:7]
    |--IDENT -> Test [1:17]
    `--OBJBLOCK -> OBJBLOCK [1:22]
        |--LCURLY -> { [1:22]
        |--METHOD_DEF -> METHOD_DEF [2:2]
        |   |--MODIFIERS -> MODIFIERS [2:2]
        |   |--TYPE -> TYPE [2:2]
        |   |   `--IDENT -> record [2:2]
        |   |--IDENT -> MyRecord [2:9]
        |   |--LPAREN -> ( [2:17]
        |   |--PARAMETERS -> PARAMETERS [2:18]
        |   |   `--PARAMETER_DEF -> PARAMETER_DEF [2:18]
        |   |       |--MODIFIERS -> MODIFIERS [2:18]
        |   |       |--TYPE -> TYPE [2:18]
        |   |       |   `--LITERAL_INT -> int [2:18]
        |   |       `--IDENT -> value [2:22]
        |   |--RPAREN -> ) [2:27]
        |   `--SLIST -> { [2:29]
        |       `--RCURLY -> } [2:30]
        |--SEMI -> ; [2:31]
        `--RCURLY -> } [3:0]

so nested record is parsed as method def. Is there any way to fix parser for this case?

It can be fixed in check but it doesnt feel like a right thing to do.

@romani
Copy link
Member

romani commented Nov 25, 2021

Better to fix AST, but until last is bad Check can have hack workaround

@romani
Copy link
Member

romani commented Nov 27, 2021

I placed breaking compatibility just warn user who created custom Check during last 2 years.

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