Repository: checkstyle/checkstyle

Problem statement:
AtclauseOrder: Falsely ignores method with annotation
Identified at https://github.com/checkstyle/checkstyle/pull/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.


Hints:
https://github.com/checkstyle/checkstyle/blob/13481f2c410e4944ecf5ab93ec49948a523a0c82/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java#L267-L277
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.
I am on it.

Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java
@@ -90,13 +90,13 @@ public interface DetailAST {
      * Checks if this branch of the parse tree contains a token
      * of the provided type.
      *
+     * @param type a TokenType
+     * @return true if and only if this branch (including this node)
+     *     contains a token of type {@code type}.
      * @deprecated
      *      Usage of this method is no longer accepted. We encourage
      *      traversal of subtrees to be written per the needs of each check
      *      to avoid unintended side effects.
-     * @param type a TokenType
-     * @return true if and only if this branch (including this node)
-     *     contains a token of type {@code type}.
      */
     @Deprecated
     boolean branchContains(int type);
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java
@@ -272,6 +272,10 @@ private static int getParentType(DetailAST commentBlock) {
             if (result == TokenTypes.TYPE || result == TokenTypes.MODIFIERS) {
                 result = parentNode.getParent().getType();
             }
+            else if (parentNode.getParent() != null
+                    && parentNode.getParent().getType() == TokenTypes.MODIFIERS) {
+                result = parentNode.getParent().getParent().getType();
+            }
         }
         return result;
     }


Test patch:
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheckTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheckTest.java
@@ -188,4 +188,36 @@ public void testAtclauseOrderLotsOfRecords() throws Exception {
         verify(checkConfig, getNonCompilablePath("InputAtclauseOrderLotsOfRecords.java"), expected);
     }
 
+    @Test
+    public void testAtclause() throws Exception {
+        final String tagOrder = "[@author, @version, @param, @return, @throws, @exception, @see,"
+                + " @since, @serial, @serialField, @serialData, @deprecated]";
+        final DefaultConfiguration checkConfig = createModuleConfig(AtclauseOrderCheck.class);
+        final String[] expected = {
+            "12: " + getCheckMessage(MSG_KEY, tagOrder),
+            "13: " + getCheckMessage(MSG_KEY, tagOrder),
+            "14: " + getCheckMessage(MSG_KEY, tagOrder),
+            "24: " + getCheckMessage(MSG_KEY, tagOrder),
+            "25: " + getCheckMessage(MSG_KEY, tagOrder),
+            "26: " + getCheckMessage(MSG_KEY, tagOrder),
+            "36: " + getCheckMessage(MSG_KEY, tagOrder),
+            "37: " + getCheckMessage(MSG_KEY, tagOrder),
+            "38: " + getCheckMessage(MSG_KEY, tagOrder),
+            "51: " + getCheckMessage(MSG_KEY, tagOrder),
+            "52: " + getCheckMessage(MSG_KEY, tagOrder),
+            "53: " + getCheckMessage(MSG_KEY, tagOrder),
+            "65: " + getCheckMessage(MSG_KEY, tagOrder),
+            "66: " + getCheckMessage(MSG_KEY, tagOrder),
+            "67: " + getCheckMessage(MSG_KEY, tagOrder),
+            "80: " + getCheckMessage(MSG_KEY, tagOrder),
+            "81: " + getCheckMessage(MSG_KEY, tagOrder),
+            "82: " + getCheckMessage(MSG_KEY, tagOrder),
+            "92: " + getCheckMessage(MSG_KEY, tagOrder),
+            "93: " + getCheckMessage(MSG_KEY, tagOrder),
+            "94: " + getCheckMessage(MSG_KEY, tagOrder),
+        };
+
+        verify(checkConfig,
+                getPath("InputAtclauseOrderWithAnnotationsOutsideJavadoc.java"), expected);
+    }
 }
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/atclauseorder/InputAtclauseOrderWithAnnotationsOutsideJavadoc.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/atclauseorder/InputAtclauseOrderWithAnnotationsOutsideJavadoc.java
new file mode 100644
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/atclauseorder/InputAtclauseOrderWithAnnotationsOutsideJavadoc.java
@@ -0,0 +1,97 @@
+package com.puppycrawl.tools.checkstyle.checks.javadoc.atclauseorder;
+
+/* Config: default
+ */
+
+public class InputAtclauseOrderWithAnnotationsOutsideJavadoc {
+    /**
+     * Some javadoc.
+     *
+     * @author max
+     * @deprecated Some javadoc.
+     * @see Some javadoc.   // violation
+     * @version 1.0         // violation
+     * @since Some javadoc. // violation
+     */
+    @Deprecated
+    public boolean branchContains(int type) { return true; }
+
+    /**
+     * Some javadoc.
+     *
+     * @author max
+     * @deprecated Some javadoc.
+     * @see Some javadoc.   // violation
+     * @version 1.0         // violation
+     * @since Some javadoc. // violation
+     */
+    public boolean branchContains2(int type) { return true; }
+}
+
+/**
+ * Some javadoc.
+ *
+ * @author max
+ * @deprecated Some javadoc.
+ * @see Some javadoc.   // violation
+ * @version 1.0         // violation
+ * @since Some javadoc. // violation
+ */
+@Deprecated
+class TestClass {
+
+}
+
+class TestInnerClasses extends InputAtclauseOrderWithAnnotationsOutsideJavadoc{
+    /**
+     * Some javadoc.
+     *
+     * @author max
+     * @deprecated Some javadoc.
+     * @see Some javadoc.   // violation
+     * @version 1.0         // violation
+     * @since Some javadoc. // violation
+     */
+    @Deprecated
+    TestClass one = new TestClass(){
+
+    };
+
+    /**
+     * Some javadoc.
+     *
+     * @author max
+     * @deprecated Some javadoc.
+     * @see Some javadoc.   // violation
+     * @version 1.0         // violation
+     * @since Some javadoc. // violation
+     */
+    @Override
+    public boolean branchContains(int type) {
+        return false;
+    }
+}
+
+/**
+ * Some javadoc.
+ *
+ * @author max
+ * @deprecated Some javadoc.
+ * @see Some javadoc.   // violation
+ * @version 1.0         // violation
+ * @since Some javadoc. // violation
+ */
+@Deprecated
+enum TestEnums {}
+
+/**
+ * Some javadoc.
+ *
+ * @author max
+ * @deprecated Some javadoc.
+ * @see Some javadoc.   // violation
+ * @version 1.0         // violation
+ * @since Some javadoc. // violation
+ */
+@Deprecated
+interface TestInterfaces {}