From 6cd89ebebaec931a33ffde42b295eb6a20dc07e1 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 19 Jun 2014 17:21:13 +0400 Subject: [PATCH] Update for existing Check: DeclarationOrder #4 --- .../checks/coding/DeclarationOrderCheck.java | 65 +++++++++++++++++++ .../checks/coding/messages.properties | 1 + .../coding/DeclarationOrderCheckTest.java | 45 +++++++++++++ .../coding/InputDeclarationOrder.java | 37 +++++++++++ src/xdocs/config_coding.xml | 23 +++++++ 5 files changed, 171 insertions(+) diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java index 25fc07be0..1103a3a07 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java @@ -18,6 +18,9 @@ //////////////////////////////////////////////////////////////////////////////// package com.puppycrawl.tools.checkstyle.checks.coding; +import java.util.HashMap; +import java.util.Map; + import com.puppycrawl.tools.checkstyle.api.Check; import com.puppycrawl.tools.checkstyle.api.DetailAST; import com.puppycrawl.tools.checkstyle.api.FastStack; @@ -46,13 +49,29 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * *

*

+ * It can check that overload methods are grouped together. Example: + *

+ * 

+ * public void foo(int i) {}
+ * public void foo(String s) {}
+ * public void notFoo() {} // Have to be after foo(int i, String s)
+ * public void foo(int i, String s) {}
+ *
+ *

* An example of how to configure the check is: *

* 
  * <module name="DeclarationOrder"/>
  *
+ * An example of a check's configuration for grouping overload methods: + * 
+ * <module name="DeclarationOrder">
+ *     <property name="groupOverloadMethods" value="true">
+ * </module>
+ *
* * @author r_auckenthaler + * @author maxvetrenko */ public class DeclarationOrderCheck extends Check { @@ -92,6 +111,8 @@ public class DeclarationOrderCheck extends Check 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 +134,12 @@ public class DeclarationOrderCheck extends Check switch(aAST.getType()) { case TokenTypes.OBJBLOCK: mScopeStates.push(new ScopeState()); + if (mGroupOverloadMethods && (parentType == TokenTypes.CLASS_DEF + || parentType == TokenTypes.ENUM_DEF + || parentType == TokenTypes.INTERFACE_DEF)) + { + checkOverloadMethodsGrouping(aAST); + } break; case TokenTypes.CTOR_DEF: @@ -205,6 +232,35 @@ public class DeclarationOrderCheck extends Check } } + /** + * Checks that if overload methods are grouped together they + * should not be separated from each other. + * @param aObjectBlock is a class, interface or enum object block. + */ + private void checkOverloadMethodsGrouping(DetailAST aObjectBlock) + { + final int allowedDistance = 1; + DetailAST currentToken = aObjectBlock.getFirstChild(); + final Map methodIndexMap = + new HashMap(); + int currentIndex = 0; + while (currentToken != null) { + if (currentToken.getType() == TokenTypes.METHOD_DEF) { + currentIndex++; + final String methodName = + currentToken.findFirstToken(TokenTypes.IDENT).getText(); + if (methodIndexMap.containsKey(methodName)) { + final int priviousIndex = methodIndexMap.get(methodName); + if (currentIndex - priviousIndex > allowedDistance) { + log(currentToken, "declaration.order.overloads"); + } + } + methodIndexMap.put(methodName, currentIndex); + } + currentToken = currentToken.getNextSibling(); + } + } + /** * Sets whether to ignore constructors. * @param aIgnoreConstructors whether to ignore constructors. @@ -231,4 +287,13 @@ public class DeclarationOrderCheck extends Check { mIgnoreModifiers = aIgnoreModifiers; } + + /** + * Sets whether to group overload methods. + * @param aGroupOverloadMethods whether to group overload methods. + */ + public void setGroupOverloadMethods(boolean aGroupOverloadMethods) + { + mGroupOverloadMethods = aGroupOverloadMethods; + } } diff --git a/src/main/resources/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties b/src/main/resources/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties index 239d7354c..871722a71 100644 --- a/src/main/resources/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties +++ b/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 should not be 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. diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheckTest.java index bd4bf0242..0534d16f7 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheckTest.java @@ -70,6 +70,7 @@ public class DeclarationOrderCheckTest 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 class DeclarationOrderCheckTest 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 class DeclarationOrderCheckTest }; verify(checkConfig, getPath("coding/InputDeclarationOrder.java"), expected); } + + @Test + public void testOverloadMethodsGrouping() 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 should not be split.", + "215:9: Overload methods should not be split.", + }; + verify(checkConfig, getPath("coding/InputDeclarationOrder.java"), expected); + } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputDeclarationOrder.java b/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputDeclarationOrder.java index 502749946..9d0219597 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputDeclarationOrder.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputDeclarationOrder.java @@ -176,4 +176,41 @@ enum InputDeclarationOrderEnum // 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); +} \ No newline at end of file diff --git a/src/xdocs/config_coding.xml b/src/xdocs/config_coding.xml index b301b000d..1dd733480 100644 --- a/src/xdocs/config_coding.xml +++ b/src/xdocs/config_coding.xml @@ -2002,6 +2002,15 @@ if ("something".equals(x))
  • Constructors
  • Methods
    • +

    + It can check that overload methods are grouped together. Example: +

    + +public void foo(int i) {} +public void foo(String s) {} +public void notFoo() {} // Have to be after foo(int i, String s) +public void foo(int i, String s) {} + @@ -2030,6 +2039,12 @@ if ("something".equals(x)) Boolean false + + groupOverloadMethods + whether to avoid splitting overload methods + Boolean + false + @@ -2040,6 +2055,14 @@ if ("something".equals(x)) <module name="DeclarationOrder"/> +

    + An example of a check's configuration for grouping overload methods +

    + +<module name="DeclarationOrder"> + <property name="groupOverloadMethods" value="true"> +</module> +