Issue checkstyle#3126: Fix a lot of CommentsIndentationCheck false-po…

…sitives and false-negatives
Vladlis · May 24, 2016 · 3bd4c5a · 3bd4c5a
1 parent 91dd77d
commit 3bd4c5a
Show file tree

Hide file tree

Showing 9 changed files with 444 additions and 175 deletions.
diff --git a/circle.yml b/circle.yml
@@ -33,8 +33,13 @@ machine:
     CASSANDRA: " && sed -i.'' 's/#cassandra/cassandra/' projects-for-circle.properties"
     SCOUTER: " && sed -i.'' 's/#scouter/scouter/' projects-for-circle.properties"
     GROOVY: " && sed -i.'' 's/#groovy/groovy/' projects-for-circle.properties"
+
+    # Until performance issue is resolved
     # Test over openjdk7,8,9
-    TEST_1: $CMD1$CMD2$OPENJDK7$OPENJDK8$OPENJDK9$CMD3$CMD4
+    # TEST_1: $CMD1$CMD2$OPENJDK7$OPENJDK8$OPENJDK9$CMD3$CMD4
+
+    # Test over openjdk7
+    TEST_1: $CMD1$CMD2$OPENJDK7$CMD3$CMD4
     # Test over infinispan, protonpack, jOOL, lucene-solr
     TEST_2: $CMD1$CMD2$INFINISPAN$PROTONPACK$JOOL$LUCENE$CMD3$CMD4
     # Test over tapestry5, storm, cassandra

diff --git a/config/suppressions.xml b/config/suppressions.xml
@@ -121,6 +121,8 @@
 
     <!-- There are a lot of setters/getters in the Check. A small number of methods is left for Check's logic -->
     <suppress checks="MethodCount" files="[\\/]JavadocMethodCheck.java$"/>
+    <!-- Apart from a complex logic there is a lot of small methods for a better readability.  -->
+    <suppress checks="MethodCount" files="[\\/]CommentsIndentationCheck.java$"/>
 
     <!-- getDetails() method - huge Switch, it has to be monolithic -->
     <suppress checks="ExecutableStatementCount" files="RightCurlyCheck\.java" lines="316"/>

diff --git a/...checkstyle/test/chapter4formatting/rule4861blockcommentstyle/CommentsIndentationTest.java b/...checkstyle/test/chapter4formatting/rule4861blockcommentstyle/CommentsIndentationTest.java
@@ -90,7 +90,7 @@ public void testCommentIsAtTheEndOfBlock() throws Exception {
             "322: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
                 323, 0, 4),
             "336: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
-                337, 0, 4),
+                333, 0, 8),
             "355: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
                 352, 9, 8),
             };
@@ -106,6 +106,8 @@ public void testCommentIsAtTheEndOfBlock() throws Exception {
     @Test
     public void testCommentIsInsideSwitchBlock() throws Exception {
         final String[] expected = {
+            "19: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.block",
+                20, 12, 16),
             "25: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",
                 "24, 26", 19, "16, 12"),
             "31: " + getCheckMessage(CommentsIndentationCheck.class, "comments.indentation.single",

diff --git a/...t/chapter4formatting/rule4861blockcommentstyle/InputCommentsIndentationInSwitchBlock.java b/...t/chapter4formatting/rule4861blockcommentstyle/InputCommentsIndentationInSwitchBlock.java
@@ -16,7 +16,7 @@ private static void fooSwitch() {
                 // comment
                 break;
             case "3":
-            /* com */
+            /* // warn */
                 foo1();
                 /* com */
                 break;

diff --git a/...ain/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java b/...ain/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java
diff --git a/...java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheckTest.java b/...java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheckTest.java
@@ -37,7 +37,7 @@
 /**
 *
 * @author <a href="mailto:nesterenko-aleksey@list.ru">Aleksey Nesterenko</a>
-* @author <a href="mailto:andreyselkin@gmail.com">Aleksey Nesterenko</a>
+* @author <a href="mailto:andreyselkin@gmail.com">Andrei Selkin</a>
 *
 */
 public class CommentsIndentationCheckTest extends BaseCheckTestSupport {
@@ -78,11 +78,18 @@ public void testCommentIsAtTheEndOfBlock() throws Exception {
             "277: " + getCheckMessage(MSG_KEY_SINGLE, 276, 9, 8),
             "316: " + getCheckMessage(MSG_KEY_SINGLE, 315, 9, 8),
             "322: " + getCheckMessage(MSG_KEY_SINGLE, 323, 0, 4),
-            "336: " + getCheckMessage(MSG_KEY_SINGLE, 337, 0, 4),
+            "336: " + getCheckMessage(MSG_KEY_SINGLE, 333, 0, 8),
             "355: " + getCheckMessage(MSG_KEY_SINGLE, 352, 9, 8),
             "380: " + getCheckMessage(MSG_KEY_BLOCK, 381, 12, 8),
             "393: " + getCheckMessage(MSG_KEY_SINGLE, 392, 12, 8),
             "400: " + getCheckMessage(MSG_KEY_SINGLE, 401, 8, 10),
+            "457: " + getCheckMessage(MSG_KEY_SINGLE, 455, 0, 8),
+            "473: " + getCheckMessage(MSG_KEY_BLOCK, 469, 10, 8),
+            "483: " + getCheckMessage(MSG_KEY_BLOCK, 477, 10, 8),
+            "491: " + getCheckMessage(MSG_KEY_BLOCK, 487, 10, 8),
+            "499: " + getCheckMessage(MSG_KEY_BLOCK, 495, 10, 8),
+            "507: " + getCheckMessage(MSG_KEY_BLOCK, 503, 10, 8),
+            "518: " + getCheckMessage(MSG_KEY_BLOCK, 511, 10, 8),
         };
         final String testInputFile = "InputCommentsIndentationCommentIsAtTheEndOfBlock.java";
         verify(checkConfig, getPath(testInputFile), expected);
@@ -93,6 +100,7 @@ public void testCommentIsInsideSwitchBlock() throws Exception {
         final DefaultConfiguration checkConfig =
             createCheckConfig(CommentsIndentationCheck.class);
         final String[] expected = {
+            "19: " + getCheckMessage(MSG_KEY_BLOCK, 20, 12, 16),
             "25: " + getCheckMessage(MSG_KEY_SINGLE, "24, 26", 19, "16, 12"),
             "31: " + getCheckMessage(MSG_KEY_SINGLE, "30, 32", 19, "16, 12"),
             "48: " + getCheckMessage(MSG_KEY_SINGLE, 49, 6, 16),
@@ -109,6 +117,8 @@ public void testCommentIsInsideSwitchBlock() throws Exception {
             "204: " + getCheckMessage(MSG_KEY_SINGLE, 205, 20, 17),
             "205: " + getCheckMessage(MSG_KEY_SINGLE, "202, 206", 17, "16, 12"),
             "229: " + getCheckMessage(MSG_KEY_SINGLE, "228, 230", 6, "12, 12"),
+            "276: " + getCheckMessage(MSG_KEY_BLOCK, "275, 279", 11, "16, 12"),
+            "281: " + getCheckMessage(MSG_KEY_SINGLE, "280, 282", 11, "16, 12"),
         };
         final String testInputFile = "InputCommentsIndentationInSwitchBlock.java";
         verify(checkConfig, getPath(testInputFile), expected);
@@ -143,6 +153,9 @@ public void testSurroundingCode() throws Exception {
             "90: " + getCheckMessage(MSG_KEY_SINGLE, 91, 14, 8),
             "98: " + getCheckMessage(MSG_KEY_SINGLE, 99, 13, 8),
             "108: " + getCheckMessage(MSG_KEY_SINGLE, 109, 33, 8),
+            "130: " + getCheckMessage(MSG_KEY_BLOCK, 131, 12, 8),
+            "135: " + getCheckMessage(MSG_KEY_BLOCK, 136, 4, 8),
+            "141: " + getCheckMessage(MSG_KEY_BLOCK, 140, 4, 8),
         };
         final String testInputFile = "InputCommentsIndentationSurroundingCode.java";
         verify(checkConfig, getPath(testInputFile), expected);
@@ -183,7 +196,10 @@ public void testCheckOnlyBlockComments() throws Exception {
             "25: " + getCheckMessage(MSG_KEY_BLOCK, 27, 16, 12),
             "28: " + getCheckMessage(MSG_KEY_BLOCK, 31, 16, 12),
             "51: " + getCheckMessage(MSG_KEY_BLOCK, 53, 23, 36),
-            };
+            "130: " + getCheckMessage(MSG_KEY_BLOCK, 131, 12, 8),
+            "135: " + getCheckMessage(MSG_KEY_BLOCK, 136, 4, 8),
+            "141: " + getCheckMessage(MSG_KEY_BLOCK, 140, 4, 8),
+        };
         final String testInputFile = "InputCommentsIndentationSurroundingCode.java";
         verify(checkConfig, getPath(testInputFile), expected);
     }

diff --git a/...tools/checkstyle/checks/indentation/InputCommentsIndentationCommentIsAtTheEndOfBlock.java b/...tools/checkstyle/checks/indentation/InputCommentsIndentationCommentIsAtTheEndOfBlock.java
@@ -397,11 +397,127 @@ void foo58() {
         /*
            comment
            */
-        // comment
+        // violation
           foo1();
           // comment
     }
 
+    void foo59() {
+        foo1();
+        /*
+         comment */
+        // comment
+    }
+
+
+    void foo61() {
+        foo1();
+        /*
+         * comment
+         */
+        /*
+         * comment
+         */
+    }
+
+    void foo62() {
+        if (true) {
+            System.out.println();
+          }
+        else {
+
+          }
+        /*
+         comment
+         */
+        /*
+         comment
+         */
+    }
+
+    void foo63() {
+        try {
+            System.out.println();
+          }
+        catch (Exception e){
+
+          }
+
+        /*
+         comment
+         */
+        /*
+         comment
+         */
+    }
+
+    void foo64()  {
+        foo1();
+
+//  violation
+    }
+
+    void foo65() {
+        int i = 1
+            + 1
+            + 1;
+        // comment
+        // comment
+    }
+
+    void foo66()  {
+        if (true) {
+            getClass();
+        }
+
+          /* violation */
+    }
+
+    void foo67()  {
+        try {
+            getClass();
+        } finally {
+            hashCode();
+        }
+
+          /* violation */
+    }
+
+    void foo68()  {
+        for (int i = 0; i < 0; i++) {
+            getClass();
+        }
+
+          /* violation */
+    }
+
+    void foo69()  {
+        while (true) {
+            getClass();
+        }
+
+          /* violation */
+    }
+
+    void foo70()  {
+        do {
+            getClass();
+        } while (true);
+
+          /* violation */
+    }
+
+    void foo71() {
+        switch("") {
+            case "!":
+                break;
+            default:
+                break;
+        }
+
+          // violation
+    }
+
     // We almost reached the end of the class here.
 }
 // The END of the class.
diff --git a/...puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationInSwitchBlock.java b/...puppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationInSwitchBlock.java
@@ -16,7 +16,7 @@ private static void fooSwitch() {
                 // comment
                 break;
             case "3":
-            /* com */
+            /* violation */
                 foo1();
                 /* com */
                 break;
@@ -265,4 +265,41 @@ public void foo12() {
             case 1:
         }
     }
+
+    public void foo13() {
+        int a = 5;
+        switch (a) {
+            case 1:
+                /* comment */
+            case 2:
+                hashCode();
+           /*
+            violation
+            */
+            case 3: // comment
+                hashCode();
+           // violation
+            case 4: // comment
+                if (true) {
+
+                }
+                else {
+
+                }
+                // comment
+            case 5:
+                String s = ""
+                    + 1
+                    + "123";
+                break;
+                // comment
+            case 6:
+                String q = ""
+                    + 1
+                    + "123";
+                // comment
+            case 7:
+                break;
+        }
+    }
 }
diff --git a/...ppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationSurroundingCode.java b/...ppycrawl/tools/checkstyle/checks/indentation/InputCommentsIndentationSurroundingCode.java
@@ -125,5 +125,21 @@ public void foo10()
         };
     }
 
+    public void foo11() {
+
+            /* empty */
+        hashCode();
+    }
+
+    public void foo12() {
+    /* empty */
+        hashCode();
+    }
+
+    public void foo13() {
+        hashCode();
+    /* empty */
+    }
+
 } // The Check should not throw NPE here!
 // The Check should not throw NPE here!