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

AtclauseOrder: Falsely ignores method with annotation #9941

Closed
rnveach opened this issue Apr 27, 2021 · 2 comments · Fixed by #9942
Closed

AtclauseOrder: Falsely ignores method with annotation #9941

rnveach opened this issue Apr 27, 2021 · 2 comments · Fixed by #9942

Comments

@rnveach
Copy link
Member

rnveach commented Apr 27, 2021

Identified at #9939

$ cat TestClass.java
public class TestClass {
    /**
     * Test.
     *
     * @deprecated Test.
     * @param type Test.
     * @return Test.
     */
    @Deprecated
    public boolean branchContains(int type) { return true; }

    /**
     * Test.
     *
     * @deprecated Test.
     * @param type Test.
     * @return Test.
     */
    public boolean branchContains2(int type) { return true; }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
    <module name="AtclauseOrder">
      <property name="tagOrder" value="@param, @return, @throws, @deprecated"/>
      <property name="target"
               value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/>
    </module>
    </module>
</module>

$ java -jar checkstyle-8.41.1-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:16: Block tags have to appear in the order '[@param, @return, @throws, @deprecated]'. [AtclauseOrder]
[ERROR] TestClass.java:17: Block tags have to appear in the order '[@param, @return, @throws, @deprecated]'. [AtclauseOrder]
Audit done.
Checkstyle ends with 2 errors.

Method without annotation prints 2 violations while one with annotation prints none.

Method annotations should not play into this check's behavior and I expect the same violations between both examples.

@rnveach
Copy link
Member Author

rnveach commented Apr 27, 2021

private static int getParentType(DetailAST commentBlock) {
final DetailAST parentNode = commentBlock.getParent();
int result = 0;
if (parentNode != null) {
result = parentNode.getType();
if (result == TokenTypes.TYPE || result == TokenTypes.MODIFIERS) {
result = parentNode.getParent().getType();
}
}
return result;
}

Issue is method returns the annotation definition instead of the method definition. The code wrongly believes we are looking at an annotation instead of method.

    |--METHOD_DEF -> METHOD_DEF [9:4]
    |  |--MODIFIERS -> MODIFIERS [9:4]
    |  |  |--ANNOTATION -> ANNOTATION [9:4]
    |  |  |  |--BLOCK_COMMENT_BEGIN -> /* [2:4]

As seen in structure, getting the parent on the block just gives us the annotation and should skip that and the modifiers to give us the method definition.

@rnveach rnveach changed the title AtclauseOrder: Ignore method with annotation. AtclauseOrder: Falsely ignores method with annotation Apr 27, 2021
@Vyom-Yadav
Copy link
Member

I am on it.

Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Apr 27, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Apr 28, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jun 7, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jun 7, 2021
@romani romani added the bug label Jun 12, 2021
@romani romani added this to the 8.44 milestone Jun 12, 2021
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