Improved UT coverage for CustomImportOrder #1128

checkstyle · Jul 8, 2015 · 6f51fa7 · 6f51fa7
1 parent d0e8581
commit 6f51fa7
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 29 deletions.
diff --git a/pom.xml b/pom.xml
@@ -803,12 +803,10 @@
           <regex><pattern>.*.checks.header.RegexpHeaderCheck</pattern><branchRate>87</branchRate><lineRate>93</lineRate></regex>
 
 
-          <regex><pattern>.*.checks.imports.CustomImportOrderCheck</pattern><branchRate>93</branchRate><lineRate>91</lineRate></regex>
-          <regex><pattern>.*.checks.imports.Guard</pattern><branchRate>86</branchRate><lineRate>100</lineRate></regex>
+          <regex><pattern>.*.checks.imports.CustomImportOrderCheck</pattern><branchRate>98</branchRate><lineRate>100</lineRate></regex>
           <regex><pattern>.*.checks.imports.ImportControlCheck</pattern><branchRate>85</branchRate><lineRate>73</lineRate></regex>
           <regex><pattern>.*.checks.imports.ImportControlLoader</pattern><branchRate>72</branchRate><lineRate>88</lineRate></regex>
           <regex><pattern>.*.checks.imports.ImportOrderCheck</pattern><branchRate>91</branchRate><lineRate>99</lineRate></regex>
-          <regex><pattern>.*.checks.imports.PkgControl</pattern><branchRate>80</branchRate><lineRate>100</lineRate></regex>
 
 
           <regex><pattern>.*.checks.indentation.ArrayInitHandler</pattern><branchRate>83</branchRate><lineRate>97</lineRate></regex>

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java
@@ -296,22 +296,16 @@ public final void setSortImportsInGroupAlphabetically(boolean value) {
     /**
      * Sets a custom import order from the rules in the string format specified
      * by user.
-     * @param inputCustoimportOrder
+     * @param inputCustomImportOrder
      *        user value.
      */
-    public final void setCustomImportOrderRules(final String inputCustoimportOrder) {
+    public final void setCustomImportOrderRules(final String inputCustomImportOrder) {
         customImportOrderRules.clear();
-        try {
-            for (String currentState : inputCustoimportOrder
-                    .split("\\s*###\\s*")) {
-                addRuleastoList(currentState);
-            }
-            customImportOrderRules.add(NON_GROUP_RULE_GROUP);
-        }
-        catch (StringIndexOutOfBoundsException exp) {
-            //if the structure of the input rule isn't correct
-            throw new RuntimeException("Unable to parse input rule: " + exp);
+        for (String currentState : inputCustomImportOrder
+                .split("\\s*###\\s*")) {
+            addRuleastoList(currentState);
         }
+        customImportOrderRules.add(NON_GROUP_RULE_GROUP);
     }
 
     @Override
@@ -340,8 +334,7 @@ public void beginTree(DetailAST rootAST) {
     @Override
     public void visitToken(DetailAST ast) {
         if (ast.getType() == TokenTypes.PACKAGE_DEF) {
-            if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP)
-                    && samePackageMatchingDepth != -1) {
+            if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP)) {
                 samePackageDomainsRegExp = createSamePackageRegexp(
                         samePackageMatchingDepth, ast);
             }
@@ -640,12 +633,7 @@ else if (ruleStr.startsWith(SAME_PACKAGE_RULE_GROUP)) {
 
             final String rule = ruleStr.substring(ruleStr.indexOf('(') + 1,
                     ruleStr.indexOf(')'));
-            try {
-                samePackageMatchingDepth = Integer.parseInt(rule);
-            }
-            catch (NumberFormatException e) {
-                samePackageDomainsRegExp = rule;
-            }
+            samePackageMatchingDepth = Integer.parseInt(rule);
             if (samePackageMatchingDepth <= 0) {
                 throw new IllegalArgumentException(
                         "SAME_PACKAGE rule parameter should be positive integer: " + ruleStr);

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java
@@ -22,9 +22,11 @@
 import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport;
 import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
 import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 
 import java.io.File;
+import java.lang.reflect.Method;
 
 import org.junit.Test;
 
@@ -33,6 +35,7 @@
 import static com.puppycrawl.tools.checkstyle.checks.imports.CustomImportOrderCheck.MSG_NONGROUP_IMPORT;
 import static com.puppycrawl.tools.checkstyle.checks.imports.CustomImportOrderCheck.MSG_ORDER;
 import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
 
 public class CustomImportOrderCheckTest extends BaseCheckTestSupport {
     /**
@@ -73,7 +76,7 @@ public void testCustom() throws Exception {
      * @throws Exception
      */
     @Test
-    public void testDefaultPackage() throws Exception {
+    public void testGoogleStyleguideConfiguraiton() throws Exception {
         final DefaultConfiguration checkConfig =
                 createCheckConfig(CustomImportOrderCheck.class);
         checkConfig.addAttribute("thirdPartyPackageRegExp", "com.|org.");
@@ -326,6 +329,28 @@ public void testGetAcceptableTokens() {
         assertArrayEquals(expected, actual);
     }
 
+    @Test
+    // UT uses Reflection to avoid removing null-validation from static method,
+    // which is a candidate for utility method in the future
+    public void testGetFullImportIdent() {
+        Object actual;
+        try {
+            Class<?> c = Class.forName(
+                    "com.puppycrawl.tools.checkstyle.checks.imports.CustomImportOrderCheck");
+            Object t = c.newInstance();
+            Method m = c.getDeclaredMethod("getFullImportIdent", DetailAST.class);
+            m.setAccessible(true);
+            actual = m.invoke(t, (DetailAST) null);
+        }
+        catch (Exception e) {
+            e.printStackTrace();
+            actual = null;
+        }
+
+        String expected = "";
+        assertEquals(expected, (String) actual);
+    }
+
     @Test(expected = CheckstyleException.class)
     public void testSamePackageDepthNegative() throws Exception {
         final DefaultConfiguration checkConfig =
@@ -336,9 +361,8 @@ public void testSamePackageDepthNegative() throws Exception {
                 "SAME_PACKAGE(-1)");
         final String[] expected = {};
 
-        verify(checkConfig, new File("src/test/resources-noncompilable/com/puppycrawl/tools/"
-                + "checkstyle/imports/"
-                + "InputCustomImportOrderSamePackageDepth2-5.java").getCanonicalPath(), expected);
+        verify(checkConfig, getPath("imports" + File.separator
+                + "InputCustomImportOrder.java"), expected);
     }
 
     @Test(expected = CheckstyleException.class)
@@ -351,9 +375,50 @@ public void testSamePackageDepthZero() throws Exception {
                 "SAME_PACKAGE(0)");
         final String[] expected = {};
 
-        verify(checkConfig, new File("src/test/resources-noncompilable/com/puppycrawl/tools/"
+        verify(checkConfig, getPath("imports" + File.separator
+                + "InputCustomImportOrder.java"), expected);
+    }
+
+    @Test(expected = CheckstyleException.class)
+    public void testUnsupportedRule() throws Exception {
+        final DefaultConfiguration checkConfig =
+                createCheckConfig(CustomImportOrderCheck.class);
+        checkConfig.addAttribute("customImportOrderRules", "SAME_PACKAGE(3)###UNSUPPORTED_RULE"); //#AAA##BBBB###CCCC####DDDD
+        checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true");
+        final String[] expected = {
+            "4: " + getCheckMessage(MSG_ORDER, "SAME_PACKAGE"),
+            "6: " + getCheckMessage(MSG_ORDER, "SAME_PACKAGE"),
+            "7: " + getCheckMessage(MSG_ORDER, "SAME_PACKAGE"),
+            "8: " + getCheckMessage(MSG_ORDER, "SAME_PACKAGE"),
+            "9: " + getCheckMessage(MSG_ORDER, "SAME_PACKAGE"),
+        };
+
+        verify(checkConfig, getPath("imports" + File.separator
+                + "InputCustomImportOrder.java"), expected);
+    }
+
+    @Test(expected = CheckstyleException.class)
+    public void testSamePackageDepthNotInt() throws Exception {
+        final DefaultConfiguration checkConfig =
+                createCheckConfig(CustomImportOrderCheck.class);
+        checkConfig.addAttribute("customImportOrderRules", "SAME_PACKAGE(INT_IS_REQUIRED_HERE)");
+        checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true");
+        final String[] expected = {};
+
+        verify(checkConfig, getPath("imports" + File.separator
+                + "InputCustomImportOrder.java"), expected);
+    }
+
+    @Test
+    public void testNoImports() throws Exception {
+        final DefaultConfiguration checkConfig =
+                createCheckConfig(CustomImportOrderCheck.class);
+        checkConfig.addAttribute("customImportOrderRules", "SAME_PACKAGE(3)");
+        final String[] expected = {};
+
+        verify(checkConfig, new File("src/test/resources/com/puppycrawl/tools/"
                 + "checkstyle/imports/"
-                + "InputCustomImportOrderSamePackageDepth2-5.java").getCanonicalPath(), expected);
+                + "InputCustomImportOrder_NoImports.java").getCanonicalPath(), expected);
     }
 
 }
diff --git a/...rces/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderThirdPartyPackage.java b/...rces/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderThirdPartyPackage.java
@@ -9,6 +9,7 @@
 
 import static sun.tools.util.CommandLine.parse;
 import static sun.tools.util.ModifierFilter.ALL_ACCESS;
+import static sun.tools.util.ModifierFilter.ALL_ACCESS;
 
 public class InputCustomImportOrderThirdPartyPackage
 {

diff --git a/...t/resources/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrder_NoImports.java b/...t/resources/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrder_NoImports.java
@@ -0,0 +1,5 @@
+package com.puppycrawl.tools.checkstyle.imports;
+
+
+public class InputCustomImportOrder_NoImports {
+}