Issue checkstyle#3143: added maxForVoid option for ReturnCountCheck

src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ReturnCountCheck.java

@@ -34,6 +34,18 @@
  * (2 by default). Ignores specified methods ({@code equals()} by default).
  * </p>
  * <p>
+ * <b>max</b> property will only check returns in methods and lambdas that
+ * return a specific value (Ex: 'return 1;').
+ * </p>
+ * <p>
+ * <b>maxForVoid</b> property will only check returns in methods, constructors,
+ * and lambdas that have no return type (IE 'return;'). It will only count
+ * visible return statements. Return statements not normally written, but
+ * implied, at the end of the method/constructor definition will not be taken
+ * into account. To disallow "return;" in void return type methods, use a value
+ * of 0.
+ * </p>
+ * <p>
  * Rationale: Too many return points can be indication that code is
  * attempting to do too much or may be difficult to understand.
  * </p>
@@ -58,6 +70,8 @@ public final class ReturnCountCheck extends AbstractCheck {
 
     /** Maximum allowed number of return statements. */
     private int max = 2;
+    /** Maximum allowed number of return statements for void methods. */
+    private int maxForVoid = 1;
     /** Current method context. */
     private Context context;
 
@@ -112,6 +126,22 @@ public void setMax(int max) {
         this.max = max;
     }
 
+    /**
+     * Getter for max property.
+     * @return maximum allowed number of return statements.
+     */
+    public int getMaxForVoid() {
+        return maxForVoid;
+    }
+
+    /**
+     * Setter for maxForVoid property.
+     * @param maxForVoid maximum allowed number of return statements for void methods.
+     */
+    public void setMaxForVoid(int maxForVoid) {
+        this.maxForVoid = maxForVoid;
+    }
+
     @Override
     public void beginTree(DetailAST rootAST) {
         context = new Context(false);
@@ -129,7 +159,7 @@ public void visitToken(DetailAST ast) {
                 visitLambda();
                 break;
             case TokenTypes.LITERAL_RETURN:
-                context.visitLiteralReturn();
+                visitReturn(ast);
                 break;
             default:
                 throw new IllegalStateException(ast.toString());
@@ -180,39 +210,61 @@ private void visitLambda() {
         context = new Context(true);
     }
 
+    /**
+     * Examines the return statement and tells context about it.
+     * @param ast return statement to check.
+     */
+    private void visitReturn(DetailAST ast) {
+        // we can't identify which max to use for lambdas, so we can only assign
+        // after the first return statement is seen
+        if (ast.getFirstChild().getType() == TokenTypes.SEMI) {
+            context.visitLiteralReturn(maxForVoid);
+        }
+        else {
+            context.visitLiteralReturn(max);
+        }
+    }
+
     /**
      * Class to encapsulate information about one method.
      * @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
      */
     private class Context {
         /** Whether we should check this method or not. */
         private final boolean checking;
-        /** Counter for return statements. */
+        /** Counter for return expression statements. */
         private int count;
+        /** Maximum allowed number of return statements. */
+        private Integer max;
 
         /**
          * Creates new method context.
          * @param checking should we check this method or not.
          */
         Context(boolean checking) {
             this.checking = checking;
-            count = 0;
         }
 
-        /** Increase number of return statements. */
-        public void visitLiteralReturn() {
+        /**
+         * Increase the number of return statements.
+         * @param maxAllowed Maximum allowed number of return statements.
+         */
+        public void visitLiteralReturn(int maxAllowed) {
+            if (max == null) {
+                max = maxAllowed;
+            }
+
             ++count;
         }
 
         /**
-         * Checks if number of return statements in method more
+         * Checks if number of return statements in the method are more
          * than allowed.
          * @param ast method def associated with this context.
          */
         public void checkCount(DetailAST ast) {
-            if (checking && count > getMax()) {
-                log(ast.getLineNo(), ast.getColumnNo(), MSG_KEY,
-                    count, getMax());
+            if (checking && max != null && count > max) {
+                log(ast.getLineNo(), ast.getColumnNo(), MSG_KEY, count, max);
             }
         }
     }

src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/ReturnCountCheckTest.java

@@ -138,4 +138,18 @@ public void testImproperToken() {
             // it is OK
         }
     }
+
+    @Test
+    public void testMaxForVoid() throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(ReturnCountCheck.class);
+        checkConfig.addAttribute("max", "2");
+        checkConfig.addAttribute("maxForVoid", "0");
+        final String[] expected = {
+            "4:5: " + getCheckMessage(MSG_KEY, 1, 0),
+            "8:5: " + getCheckMessage(MSG_KEY, 1, 0),
+            "14:5: " + getCheckMessage(MSG_KEY, 2, 0),
+            "30:5: " + getCheckMessage(MSG_KEY, 3, 2),
+        };
+        verify(checkConfig, getPath("InputReturnCountVoid.java"), expected);
+    }
 }

src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/InputReturnCountVoid.java

@@ -0,0 +1,40 @@
+package com.puppycrawl.tools.checkstyle.checks.coding;
+
+class InputReturnCountVoid {
+    public InputReturnCountVoid() {
+        return;
+    }
+
+    public void method() {
+        if (true) {
+            return;
+        }
+    }
+
+    public void method2() {
+        if (true) {
+            return;
+        }
+
+        return;
+    }
+
+    public int method3() {
+        if (true) {
+            return 0;
+        }
+
+        return 0;
+    }
+
+    public int method4() {
+        if (true) {
+            return 0;
+        }
+        if (false) {
+            return 0;
+        }
+
+        return 0;
+    }
+}

src/xdocs/config_coding.xml

@@ -3467,6 +3467,18 @@ public static class B {
           (2 by default). Ignores specified methods (<code>equals()</code> by default).
         </p>
 
+        <p>
+          <b>max</b> property will only check returns in methods and lambdas that return a specific value (Ex: 'return 1;').
+        </p>
+
+        <p>
+          <b>maxForVoid</b> property will only check returns in methods, constructors, and lambdas that have no return type (IE
+          'return;').
+          It will only count visible return statements. Return statements not normally written, but implied, at
+          the end of the method/constructor definition will not be taken into account.
+          To disallow "return;" in void return type methods, use a value of 0.
+        </p>
+
         <p>
           Rationale: Too many return points can mean that code is
           attempting to do too much or may be difficult to understand.
@@ -3483,10 +3495,16 @@ public static class B {
           </tr>
           <tr>
             <td>max</td>
-            <td>maximum allowed number of return statements</td>
+            <td>maximum allowed number of return statements in non-void methods/lambdas</td>
             <td><a href="property_types.html#integer">Integer</a></td>
             <td><code>2</code></td>
           </tr>
+          <tr>
+            <td>maxForVoid</td>
+            <td>maximum allowed number of return statements in void methods/constructors/lambdas</td>
+            <td><a href="property_types.html#integer">Integer</a></td>
+            <td><code>1</code></td>
+          </tr>
           <tr>
             <td>format</td>
             <td>method names to ignore</td>
@@ -3523,6 +3541,28 @@ public static class B {
 &lt;/module&gt;
         </source>
 
+        <p>
+          To configure the check so that it doesn't allow any
+          return statements per void method:
+        </p>
+        <source>
+&lt;module name=&quot;ReturnCount&quot;&gt;
+    &lt;property name=&quot;maxForVoid&quot; value=&quot;0&quot;/&gt;
+&lt;/module&gt;
+        </source>
+
+        <p>
+          To configure the check so that it doesn't allow more than 2
+          return statements per method (ignoring the <code>equals()</code>
+          method) and more than 1 return statements per void method:
+        </p>
+        <source>
+&lt;module name=&quot;ReturnCount&quot;&gt;
+    &lt;property name=&quot;max&quot; value=&quot;2&quot;/&gt;
+    &lt;property name=&quot;maxForVoid&quot; value=&quot;1&quot;/&gt;
+&lt;/module&gt;
+        </source>
+
         <p>
           To configure the check so that it doesn't allow more than three
           return statements per method for all methods: