Issue #2095: Fix false negative in AtclauseOrder check

checkstyle · Sep 17, 2015 · 3fe3fc6 · 3fe3fc6
1 parent dc3ac39
commit 3fe3fc6
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 13 deletions.
diff --git a/...t/java/com/google/checkstyle/test/chapter7javadoc/rule713atclauses/AtclauseOrderTest.java b/...t/java/com/google/checkstyle/test/chapter7javadoc/rule713atclauses/AtclauseOrderTest.java
@@ -44,11 +44,14 @@ public void testIncorrect() throws Exception {
             "69: " + msg,
             "86: " + msg,
             "87: " + msg,
+            "99: " + msg,
             "101: " + msg,
             "123: " + msg,
+            "124: " + msg,
             "134: " + msg,
             "135: " + msg,
             "153: " + msg,
+            "161: " + msg,
             "172: " + msg,
             "183: " + msg,
             "185: " + msg,

diff --git a/...le/checkstyle/test/chapter7javadoc/rule713atclauses/InputIncorrectAtClauseOrderCheck.java b/...le/checkstyle/test/chapter7javadoc/rule713atclauses/InputIncorrectAtClauseOrderCheck.java
@@ -13,7 +13,7 @@
  */
 class WithAnnotations12 implements Serializable
 {
-	/**
+    /**
      * The client's first name.
      * @serial
      */
@@ -30,7 +30,7 @@ class WithAnnotations12 implements Serializable
      * @serialField
      */
     private String tThirdName;
-	
+        
     /**
      * Some text.
      * @param aString Some text.
@@ -90,13 +90,13 @@ String method5(String aString)
     {
         return "null";
     }
-    
+
     /**
      * Some text.
      * @param aString Some text.
      * @return Some text.
      * @serialData Some javadoc.
-     * @param aInt Some text.
+     * @param aInt Some text. //warn
      * @throws Exception Some text.
      * @param aBoolean Some text. //warn
      * @deprecated Some text.
@@ -121,7 +121,7 @@ class InnerClassWithAnnotations
          * @return Some text.
          * @deprecated Some text.
          * @param aString Some text. //warn
-         * @throws Exception Some text.
+         * @throws Exception Some text. //warn
          */
         String method(String aString) throws Exception
         {
@@ -158,7 +158,7 @@ void method3() throws Exception {}
          * Some text.
          * @throws Exception Some text.
          * @serialData Some javadoc.
-         * @return Some text.
+         * @return Some text. //warn
          */
         String method4() throws Exception
         {

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
@@ -158,18 +158,21 @@ public void visitJavadocToken(DetailNode ast) {
      * @param javadoc Javadoc root node.
      */
     private void checkOrderInTagSection(DetailNode javadoc) {
-        int indexOrderOfPreviousTag = 0;
+        int maxIndexOfPreviousTag = 0;
 
         for (DetailNode node : javadoc.getChildren()) {
             if (node.getType() == JavadocTokenTypes.JAVADOC_TAG) {
                 final String tagText = JavadocUtils.getFirstChild(node).getText();
-                final int indexOrderOfCurrentTag = tagOrder.indexOf(tagText);
-
-                if (tagOrder.contains(tagText)
-                        && indexOrderOfCurrentTag < indexOrderOfPreviousTag) {
-                    log(node.getLineNumber(), MSG_KEY, tagOrder.toString());
+                final int indexOfCurrentTag = tagOrder.indexOf(tagText);
+
+                if (indexOfCurrentTag != -1) {
+                    if (indexOfCurrentTag < maxIndexOfPreviousTag) {
+                        log(node.getLineNumber(), MSG_KEY, tagOrder.toString());
+                    }
+                    else {
+                        maxIndexOfPreviousTag = indexOfCurrentTag;
+                    }
                 }
-                indexOrderOfPreviousTag = indexOrderOfCurrentTag;
             }
         }
     }

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
@@ -65,17 +65,21 @@ public void testIncorrect() throws Exception {
             "40: " + getCheckMessage(MSG_KEY, tagOrder),
             "50: " + getCheckMessage(MSG_KEY, tagOrder),
             "51: " + getCheckMessage(MSG_KEY, tagOrder),
+            "52: " + getCheckMessage(MSG_KEY, tagOrder),
             "62: " + getCheckMessage(MSG_KEY, tagOrder),
             "69: " + getCheckMessage(MSG_KEY, tagOrder),
             "86: " + getCheckMessage(MSG_KEY, tagOrder),
             "87: " + getCheckMessage(MSG_KEY, tagOrder),
             "99: " + getCheckMessage(MSG_KEY, tagOrder),
+            "100: " + getCheckMessage(MSG_KEY, tagOrder),
             "101: " + getCheckMessage(MSG_KEY, tagOrder),
             "115: " + getCheckMessage(MSG_KEY, tagOrder),
             "123: " + getCheckMessage(MSG_KEY, tagOrder),
+            "124: " + getCheckMessage(MSG_KEY, tagOrder),
             "134: " + getCheckMessage(MSG_KEY, tagOrder),
             "135: " + getCheckMessage(MSG_KEY, tagOrder),
             "145: " + getCheckMessage(MSG_KEY, tagOrder),
+            "146: " + getCheckMessage(MSG_KEY, tagOrder),
             "153: " + getCheckMessage(MSG_KEY, tagOrder),
             "161: " + getCheckMessage(MSG_KEY, tagOrder),
             "172: " + getCheckMessage(MSG_KEY, tagOrder),