Issue sevntu-checkstyle#454: fixed NPE in ConfusingConditionCheck

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/ConfusingConditionCheck.java

@@ -48,7 +48,7 @@
  * and swapped code in "if" and "else" block:
  * </p>
  * <pre>
- *  if (a == b &amp;&amp; c == d)
+ *  if (a == b || c == d)
  *      {
  *          smth2();
  *      }
@@ -246,22 +246,50 @@ private boolean isRatioBetweenIfAndElseBlockSuitable(DetailAST literalIf) {
      * @return linesOfCodeInIfBlock line of code in block.
      */
     private static int getAmounOfCodeRowsInBlock(DetailAST detailAST) {
+        final DetailAST firstBrace = getFirstBrace(detailAST);
+        int linesOfCodeInIfBlock;
+
+        if (firstBrace == null) {
+            linesOfCodeInIfBlock = 0;
+        }
+        else {
+            final DetailAST lastBrace = firstBrace.getLastChild();
+            linesOfCodeInIfBlock = lastBrace.getLineNo()
+                    - firstBrace.getLineNo();
+            // If the closing brace on a separate line - ignore this line.
+            if (lastBrace.getLineNo() != lastBrace.getParent().getLineNo()) {
+                linesOfCodeInIfBlock -= 1;
+            }
+        }
+
+        return linesOfCodeInIfBlock;
+    }
+
+    /**
+     * Retrieves the first, opening brace of an {@code if} or {@code else} statement.
+     * @param detailAST The token to examine.
+     * @return The opening brace token or {@code null} if it doesn't exist.
+     */
+    private static DetailAST getFirstBrace(DetailAST detailAST) {
         DetailAST firstBrace = null;
+
         if (detailAST.getType() == TokenTypes.LITERAL_ELSE) {
             firstBrace = detailAST.getFirstChild();
+
+            if (firstBrace.getType() == TokenTypes.LITERAL_IF) {
+                firstBrace = getFirstBrace(firstBrace);
+            }
         }
         else {
             firstBrace = detailAST.getFirstChild().getNextSibling()
                     .getNextSibling().getNextSibling();
         }
-        final DetailAST lastBrace = firstBrace.getLastChild();
-        int linesOfCodeInIfBlock = lastBrace.getLineNo()
-                - firstBrace.getLineNo();
-        // If the closing brace on a separate line - ignore this line.
-        if (lastBrace.getLineNo() != lastBrace.getParent().getLineNo()) {
-            linesOfCodeInIfBlock -= 1;
+
+        if (firstBrace != null && firstBrace.getType() != TokenTypes.SLIST) {
+            firstBrace = null;
         }
-        return linesOfCodeInIfBlock;
+
+        return firstBrace;
     }
 
     /**

sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/ConfusingConditionCheckTest.java

@@ -37,12 +37,6 @@ public class ConfusingConditionCheckTest extends BaseCheckTestSupport {
     public void testDefault() throws Exception {
         final DefaultConfiguration checkConfig = createCheckConfig(ConfusingConditionCheck.class);
 
-        checkConfig.addAttribute("ignoreInnerIf", "true");
-        checkConfig.addAttribute("ignoreNullCaseInIf", "true");
-        checkConfig.addAttribute("ignoreSequentialIf", "true");
-        checkConfig.addAttribute("ignoreThrowInElse", "true");
-        checkConfig.addAttribute("multiplyFactorForElseBlocks", "4");
-
         final String[] expected = {
             "10: " + warningMessage,
             "13: " + warningMessage,
@@ -59,6 +53,8 @@ public void testDefault() throws Exception {
             "200: " + warningMessage,
             "215: " + warningMessage,
             "231: " + warningMessage,
+            "250: " + warningMessage,
+            "259: " + warningMessage,
         };
 
         verify(checkConfig, getPath("InputConfusingConditionCheck.java"),
@@ -99,6 +95,10 @@ public void testFalseProperties() throws Exception {
             "215: " + warningMessage,
             "227: " + warningMessage,
             "231: " + warningMessage,
+            "247: " + warningMessage,
+            "250: " + warningMessage,
+            "257: " + warningMessage,
+            "259: " + warningMessage,
         };
 
         verify(checkConfig, getPath("InputConfusingConditionCheck.java"),
@@ -121,9 +121,20 @@ public void testMultiplyFactor() throws Exception {
             "108: " + warningMessage,
             "111: " + warningMessage,
             "177: " + warningMessage,
+            "259: " + warningMessage,
         };
 
         verify(checkConfig, getPath("InputConfusingConditionCheck.java"),
                 expected);
     }
+
+    @Test
+    public void testExceptions() throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(ConfusingConditionCheck.class);
+
+        final String[] expected = {};
+
+        verify(checkConfig, getPath("InputConfusingConditionCheck2.java"),
+                expected);
+    }
 }

sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputConfusingConditionCheck.java

@@ -242,7 +242,24 @@ else if (!a) {}
         }
         else {
             a = true;
-        }       
+        }
+
+        if (!a) {
+            //
+        }
+        else if (!b) {
+            a = true;
+        }
+        else {
+            //
+        }
+
+        if (!a) 
+            ;
+        else if (!b) 
+            ;
+        else 
+            ;
 
     }
 }

sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputConfusingConditionCheck2.java

@@ -0,0 +1,17 @@
+package com.github.sevntu.checkstyle.checks.coding;
+
+public class InputConfusingConditionCheck2 {
+    class InnerEmptyBlocks {
+        void foo() {
+        }
+    }
+
+    InnerEmptyBlocks anon = new InnerEmptyBlocks() {
+        boolean flag = true;
+
+        void foo() {
+            if(flag);
+            else;
+        }
+    };
+}