Issue #2231: restore 'url' option for ImportControl

src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheck.java

@@ -20,6 +20,7 @@
 package com.puppycrawl.tools.checkstyle.checks.imports;
 
 import java.io.File;
+import java.net.URI;
 
 import org.apache.commons.beanutils.ConversionException;
 import org.apache.commons.lang3.StringUtils;
@@ -63,6 +64,11 @@ public class ImportControlCheck extends Check {
      */
     public static final String MSG_DISALLOWED = "import.control.disallowed";
 
+    /**
+     * A part of message for exception.
+     */
+    private static final String UNABLE_TO_LOAD = "Unable to load ";
+
     /** The root package controller. */
     private PkgControl root;
     /** The package doing the import. */
@@ -145,7 +151,33 @@ public class ImportControlCheck extends Check {
             root = ImportControlLoader.load(new File(name).toURI());
         }
         catch (final CheckstyleException ex) {
-            throw new ConversionException("Unable to load " + name, ex);
+            throw new ConversionException(UNABLE_TO_LOAD + name, ex);
+        }
+    }
+
+    /**
+     * Set the parameter for the url containing the import control
+     * configuration. It will cause the url to be loaded.
+     * @param url the url of the file to load.
+     * @throws ConversionException on error loading the file.
+     */
+    public void setUrl(final String url) {
+        // Handle empty param
+        if (StringUtils.isBlank(url)) {
+            return;
+        }
+        final URI uri;
+        try {
+            uri = URI.create(url);
+        }
+        catch (final IllegalArgumentException ex) {
+            throw new ConversionException("Syntax error in url " + url, ex);
+        }
+        try {
+            root = ImportControlLoader.load(uri);
+        }
+        catch (final CheckstyleException ex) {
+            throw new ConversionException(UNABLE_TO_LOAD + url, ex);
         }
     }
 }

src/test/java/com/puppycrawl/tools/checkstyle/BaseCheckTestSupport.java

@@ -93,6 +93,10 @@ public class BaseCheckTestSupport {
         return new File("src/test/resources/com/puppycrawl/tools/checkstyle/" + filename).getCanonicalPath();
     }
 
+    protected static String getUriString(String filename) {
+        return new File("src/test/resources/com/puppycrawl/tools/checkstyle/" + filename).toURI().toString();
+    }
+
     protected static String getSrcPath(String filename) throws IOException {
 
         return new File("src/test/java/com/puppycrawl/tools/checkstyle/" + filename).getCanonicalPath();

src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java

@@ -23,9 +23,11 @@ import static com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheck.
 import static com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheck.MSG_MISSING_FILE;
 import static com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheck.MSG_UNKNOWN_PKG;
 import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.File;
+import java.lang.reflect.InvocationTargetException;
 
 import org.apache.commons.lang3.ArrayUtils;
 import org.junit.Test;
@@ -109,10 +111,11 @@ public class ImportControlCheckTest extends BaseCheckTestSupport {
         try {
             verify(checkConfig, getPath("imports" + File.separator
                     + "InputImportControl.java"), expected);
-            fail("should fail");
+            fail("Test should fail if exception was not thrown");
         }
         catch (CheckstyleException ex) {
-            //do nothing
+            final String message = ((InvocationTargetException) ex.getCause().getCause()).getTargetException().getMessage();
+            assertTrue(message.startsWith("Unable to load "));
         }
     }
 
@@ -125,10 +128,11 @@ public class ImportControlCheckTest extends BaseCheckTestSupport {
         try {
             verify(checkConfig, getPath("imports" + File.separator
                     + "InputImportControl.java"), expected);
-            fail("should fail");
+            fail("Test should fail if exception was not thrown");
         }
         catch (CheckstyleException ex) {
-            //do nothing
+            final String message = ((InvocationTargetException) ex.getCause().getCause()).getTargetException().getMessage();
+            assertTrue(message.startsWith("Unable to load "));
         }
     }
 
@@ -172,14 +176,61 @@ public class ImportControlCheckTest extends BaseCheckTestSupport {
         assertArrayEquals(expected, actual);
     }
 
-    @Test(expected = CheckstyleException.class)
-    public void testWrongFormatURI() throws Exception {
+    @Test
+    public void testUrl() throws Exception {
         final DefaultConfiguration checkConfig = createCheckConfig(ImportControlCheck.class);
-        checkConfig.addAttribute("file",
-                "aaa://src");
-        final String[] expected = ArrayUtils.EMPTY_STRING_ARRAY;
+        checkConfig.addAttribute("url", getUriString("imports" + File.separator
+                + "import-control_one.xml"));
+        final String[] expected = {"5:1: " + getCheckMessage(MSG_DISALLOWED, "java.io.File")};
+
         verify(checkConfig, getPath("imports" + File.separator
                 + "InputImportControl.java"), expected);
     }
 
+    @Test
+    public void testUrlBlank() throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(ImportControlCheck.class);
+        checkConfig.addAttribute("url", "");
+        final String[] expected = {"1:40: " + getCheckMessage(MSG_MISSING_FILE)};
+
+        verify(checkConfig, getPath("imports" + File.separator
+                + "InputImportControl.java"), expected);
+    }
+
+    @Test
+    public void testUrlUnableToLoad() throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(ImportControlCheck.class);
+        checkConfig.addAttribute("url", "https://UnableToLoadThisURL");
+        final String[] expected = ArrayUtils.EMPTY_STRING_ARRAY;
+
+        try {
+            verify(checkConfig, getPath("imports" + File.separator
+                    + "InputImportControl.java"), expected);
+            fail("Test should fail if exception was not thrown");
+        }
+        catch (final CheckstyleException ex) {
+            final String message = ((InvocationTargetException) ex.getCause().getCause())
+                    .getTargetException().getMessage();
+            assertTrue(message.startsWith("Unable to load "));
+        }
+    }
+
+    @Test
+    public void testUrlIncorrectUrl() throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(ImportControlCheck.class);
+        checkConfig.addAttribute("url", "https://{WrongCharsInURL}");
+        final String[] expected = ArrayUtils.EMPTY_STRING_ARRAY;
+
+        try {
+            verify(checkConfig, getPath("imports" + File.separator
+                    + "InputImportControl.java"), expected);
+            fail("Test should fail if exception was not thrown");
+        }
+        catch (final CheckstyleException ex) {
+            final String message = ((InvocationTargetException) ex.getCause().getCause())
+                    .getTargetException().getMessage();
+            assertTrue(message.startsWith("Syntax error in url "));
+        }
+    }
+
 }

src/xdocs/releasenotes.xml

@@ -178,6 +178,9 @@
         <li>
           Change default value of property "format" to "^Abstract.*$" in <a href="http://checkstyle.sourceforge.net/config_naming.html#AbstractClassName">AbstractClassNameCheck</a>. Author: Bhavik Patel <a href="https://github.com/checkstyle/checkstyle/issues/1279">#1279</a>
         </li>
+        <li>
+          Removed property "url" in <a href="http://checkstyle.sourceforge.net/config_imports.html#ImportControl">ImportControlCheck</a>. Note: will be restored in release 6.11. Author: Aleksandr Ivanov <a href="https://github.com/checkstyle/checkstyle/issues/1128">#1128</a>
+        </li>
       </ul>
 
       <p>New:</p>