Update for existing Check: DeclarationOrder #4

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

@@ -18,6 +18,8 @@
 ////////////////////////////////////////////////////////////////////////////////
 package com.puppycrawl.tools.checkstyle.checks.coding;
 
+import java.util.HashMap;
+
 import com.puppycrawl.tools.checkstyle.api.Check;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.FastStack;
@@ -92,7 +94,9 @@ private static class ScopeState
     private boolean mIgnoreMethods;
     /** If true, ignore the check to modifiers (fields, ...). */
     private boolean mIgnoreModifiers;
-
+    /** If true, avoid splitting overload methods */
+    private boolean mGroupOverloadMethods;
+
     @Override
     public int[] getDefaultTokens()
     {
@@ -113,6 +117,11 @@ public void visitToken(DetailAST aAST)
         switch(aAST.getType()) {
         case TokenTypes.OBJBLOCK:
             mScopeStates.push(new ScopeState());
+            if(mGroupOverloadMethods && (parentType == TokenTypes.CLASS_DEF
+            		|| parentType == TokenTypes.ENUM_DEF
+            		|| parentType == TokenTypes.INTERFACE_DEF)) {
+                checkGroupingOverloadMethods(aAST); 
+            }
             break;
 
         case TokenTypes.CTOR_DEF:
@@ -205,6 +214,33 @@ public void leaveToken(DetailAST aAST)
         }
     }
 
+    /**
+     * Check the grouping of overload. Overload methods
+     * should not be separated from each other.
+     * @param aObjectBlock is a class, interface or enum object block.
+     */
+    private void checkGroupingOverloadMethods(DetailAST aObjectBlock)
+    {
+        final int allowedDistance = 1;
+        DetailAST currentToken = aObjectBlock.getFirstChild();
+        HashMap <String, Integer> methodCollector = new HashMap<String, Integer>();
+    	int methodCounter = 0;
+    	while (currentToken != null) {
+            if (currentToken.getType() == TokenTypes.METHOD_DEF) {
+                methodCounter++;
+                String methodName = currentToken.findFirstToken(TokenTypes.IDENT).getText();
+                if (methodCollector.containsKey(methodName)) {
+                    Integer methodNumber = methodCollector.get(methodName);
+					if (methodCounter - methodNumber > allowedDistance) {
+                        log(currentToken, "declaration.order.overloads");
+                    }
+                }
+                methodCollector.put(methodName, methodCounter);
+            }
+            currentToken = currentToken.getNextSibling();
+        }
+    }
+
     /**
      * Sets whether to ignore constructors.
      * @param aIgnoreConstructors whether to ignore constructors.
@@ -231,4 +267,13 @@ public void setIgnoreModifiers(boolean aIgnoreModifiers)
     {
         mIgnoreModifiers = aIgnoreModifiers;
     }
+
+    /**
+     * Sets whether to group overload methods. 
+     * @param aGroupOverloadMethods whether to group overload methods.
+     */
+    public void setGroupOverloadMethods(boolean aGroupOverloadMethods)
+    {
+    	mGroupOverloadMethods = aGroupOverloadMethods;
+    }
 }

src/main/resources/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties

@@ -8,6 +8,7 @@ declaration.order.method=Method definition in wrong order.
 declaration.order.static=Static variable definition in wrong order.
 declaration.order.instance=Instance variable definition in wrong order.
 declaration.order.access=Variable access definition in wrong order.
+declaration.order.overloads=Overload methods never split.
 default.comes.last=Default should be last label in the switch.
 empty.statement=Empty statement.
 equals.avoid.null=String literal expressions should be on the left side of an equals comparison.

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

@@ -70,6 +70,7 @@ public void testOnlyConstructors() throws Exception
         checkConfig.addAttribute("ignoreConstructors", "false");
         checkConfig.addAttribute("ignoreMethods", "true");
         checkConfig.addAttribute("ignoreModifiers", "true");
+        checkConfig.addAttribute("groupOverloadMethods", "false");
 
         final String[] expected = {
             "45:9: Static variable definition in wrong order.",
@@ -91,6 +92,7 @@ public void testOnlyModifiers() throws Exception
         checkConfig.addAttribute("ignoreConstructors", "true");
         checkConfig.addAttribute("ignoreMethods", "true");
         checkConfig.addAttribute("ignoreModifiers", "false");
+        checkConfig.addAttribute("groupOverloadMethods", "false");
 
         final String[] expected = {
             "8:5: Variable access definition in wrong order.",
@@ -120,4 +122,47 @@ public void testOnlyModifiers() throws Exception
         };
         verify(checkConfig, getPath("coding/InputDeclarationOrder.java"), expected);
     }
+
+    @Test
+    public void testOverloadMethods() throws Exception
+    {
+        final DefaultConfiguration checkConfig =
+            createCheckConfig(DeclarationOrderCheck.class);
+        checkConfig.addAttribute("ignoreConstructors", "false");
+        checkConfig.addAttribute("ignoreMethods", "false");
+        checkConfig.addAttribute("ignoreModifiers", "false");
+        checkConfig.addAttribute("groupOverloadMethods", "true");
+
+        final String[] expected = {
+        		"8:5: Variable access definition in wrong order.",
+                "13:5: Variable access definition in wrong order.",
+                "18:5: Variable access definition in wrong order.",
+                "21:5: Variable access definition in wrong order.",
+                "27:5: Static variable definition in wrong order.",
+                "27:5: Variable access definition in wrong order.",
+                "34:9: Variable access definition in wrong order.",
+                "45:9: Static variable definition in wrong order.",
+                "45:9: Variable access definition in wrong order.",
+                "54:5: Constructor definition in wrong order.",
+                "80:5: Instance variable definition in wrong order.",
+
+                "92:9: Variable access definition in wrong order.",
+                "100:9: Static variable definition in wrong order.",
+                "100:9: Variable access definition in wrong order.",
+                "106:5: Variable access definition in wrong order.",
+                "111:5: Variable access definition in wrong order.",
+                "116:5: Variable access definition in wrong order.",
+                "119:5: Variable access definition in wrong order.",
+                "125:5: Static variable definition in wrong order.",
+                "125:5: Variable access definition in wrong order.",
+                "132:9: Variable access definition in wrong order.",
+                "143:9: Static variable definition in wrong order.",
+                "143:9: Variable access definition in wrong order.",
+                "152:5: Constructor definition in wrong order.",
+                "178:5: Instance variable definition in wrong order.",
+                "203:9: Overload methods never split.",
+                "215:9: Overload methods never split.",
+        };
+        verify(checkConfig, getPath("coding/InputDeclarationOrder.java"), expected);
+    }
 }

src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputDeclarationOrder.java

@@ -176,4 +176,41 @@ private static int getFoo21()
 
     // error member variables should be before methods or ctors
     private int mFoo = 0;
+
+    class overloadInput
+    {
+        public void overloadMethod(int i)
+        {
+        	//some foo code
+        }
+
+        public void overloadMethod(String s)
+        {
+        	//some foo code
+        }
+
+        public void overloadMethod(boolean b)
+        {
+        	//some foo code
+        }
+
+        public void fooMethod()
+        {
+
+        }
+
+        //error because overloads never split
+        public void overloadMethod(String s, Boolean b, int i)
+        {
+        	//some foo code
+        }
+    }
 }
+
+interface Fooable
+{
+	public abstract void foo(int i);
+	public abstract void foo(String s);
+	public abstract void noFoo();
+	public abstract void foo(String s, Boolean b, int i);
+}

src/xdocs/config_coding.xml

@@ -2030,6 +2030,12 @@ if ("something".equals(x))
             <td><a href="property_types.html#boolean">Boolean</a></td>
             <td><code>false</code></td>
           </tr>
+          <tr>
+            <td>groupOverloadMethods</td>
+            <td>whether to avoid splitting overload methods</td>
+            <td><a href="property_types.html#boolean">Boolean</a></td>
+            <td><code>false</code></td>
+          </tr>
         </table>
       </subsection>