Repository: checkstyle/checkstyle

Problem statement:
CheckstyleAntTask: substitude setConfig(File) with setConfig(String)
Ant's configUrl requires a full URL - relative paths don't work

Background in Issue #3119:

As I understand it, you should be able to specify a relative path (to a resource on the classpath) in the `configUrl` attribute of the Ant task. However I'm getting an error when I attempt to do so, and instead have to specify the full resource URL (`jar:file:PATH_TO_JAR!/PATH_TO_RESOURCE`), which is brittle and overspecified.

---------------

```
$ ant -version
Apache Ant(TM) version 1.10.1 compiled on February 2 2017

$ hg diff
diff -r 57a890e81233 build.xml
--- a/build.xml Sat Jun 10 15:06:52 2017 -0700
+++ b/build.xml Tue Jun 13 09:18:42 2017 -0700
@@ -124,7 +124,7 @@

     <!-- Checkstyle -->
     <target name="checkstyle" depends="build-test" description="Style linter">
-      <checkstyle configUrl="jar:file:lib/checkstyle-7.8.1-all.jar!/google_checks.xml" maxWarnings="0">
+      <checkstyle configUrl="/google_checks.xml" maxWarnings="0">
         <fileset dir="${core.src}" includes="**/*.java"/>
       </checkstyle>
       <checkstyle configUrl="jar:file:lib/checkstyle-7.8.1-all.jar!/google_checks.xml" maxWarnings="0">

$ ant checkstyle
Buildfile: /.../build.xml

init:

build:

build-test:

checkstyle:

BUILD FAILED
/.../build.xml:127: java.net.MalformedURLException: no protocol: /google_checks.xml
	at java.net.URL.<init>(URL.java:593)
	at java.net.URL.<init>(URL.java:490)
	at java.net.URL.<init>(URL.java:439)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at org.apache.tools.ant.IntrospectionHelper$11.set(IntrospectionHelper.java:1206)
	at org.apache.tools.ant.IntrospectionHelper$AttributeSetter.setObject(IntrospectionHelper.java:1512)
	at org.apache.tools.ant.IntrospectionHelper.setAttribute(IntrospectionHelper.java:412)
	at org.apache.tools.ant.RuntimeConfigurable.maybeConfigure(RuntimeConfigurable.java:528)
	at org.apache.tools.ant.RuntimeConfigurable.maybeConfigure(RuntimeConfigurable.java:464)
	at org.apache.tools.ant.Task.maybeConfigure(Task.java:202)
	at org.apache.tools.ant.UnknownElement.configure(UnknownElement.java:200)
	at org.apache.tools.ant.UnknownElement.maybeConfigure(UnknownElement.java:164)
	at org.apache.tools.ant.Task.perform(Task.java:347)
	at org.apache.tools.ant.Target.execute(Target.java:435)
	at org.apache.tools.ant.Target.performTasks(Target.java:456)
	at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1405)
	at org.apache.tools.ant.Project.executeTarget(Project.java:1376)
	at org.apache.tools.ant.helper.DefaultExecutor.executeTargets(DefaultExecutor.java:41)
	at org.apache.tools.ant.Project.executeTargets(Project.java:1260)
	at org.apache.tools.ant.Main.runBuild(Main.java:857)
	at org.apache.tools.ant.Main.startAnt(Main.java:236)
	at org.apache.tools.ant.launch.Launcher.run(Launcher.java:287)
	at org.apache.tools.ant.launch.Launcher.main(Launcher.java:113)

Total time: 0 seconds
```

Checkstyle runs successfully without this local mod (i.e. with the fully-qualified URL). I see this same error on both OSX and Windows.


Hints:
interesting that I do not see any "checkstyle" in stacktrace , so resolving to URL is done by ANT completely.
We do load of in sequence that I described at - https://github.com/checkstyle/checkstyle/commit/20506e1ceb2de74c9a85367135be714f8208643f#diff-2be5da6521926564b0fd9efc2fc60390R124

As we use the same config file for multiple of usages (ant, cli, maven, eclipse, .......) ... we cannot change anything. Looks like nuance of ANT pre-processing of `URL` types (`public void setConfigUrl(URL url) {`([code](https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java#L255))), so such syntax might be required.

ToDo if that is feature of ANT, we need to have a link to specification and probably(I am not sure now!!!) make new setter for config with `String` type, to let ANT do nothing and trigger checkstyle logic on priority of files load.

@dimo414 , if you have a time please help us and your self to resolve this issue.
I don't think it's a nuance of Ant's preprocessing - Ant's just constructing a `URL` instance since that's the type the `setConfigUrl()` method expects. The exception is coming from `java.net.URL`'s constructor, so there's nothing Ant can really do about it.

I agree swapping `setConfigUrl()` to take a `String` instead seems like the right fix, I'll try to verify that works and send a pull request if it does.

Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java b/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
@@ -24,7 +24,6 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -93,7 +92,7 @@ public class CheckstyleAntTask extends Task {
     private String fileName;
 
     /** Config file containing configuration. */
-    private String configLocation;
+    private String config;
 
     /** Whether to fail build on violations. */
     private boolean failOnViolation = true;
@@ -230,42 +229,13 @@ public void setFile(File file) {
 
     /**
      * Sets configuration file.
-     * @param file the configuration file to use
+     * @param configuration the configuration file, URL, or resource to use
      */
-    public void setConfig(File file) {
-        setConfigLocation(file.getAbsolutePath());
-    }
-
-    /**
-     * Sets URL to the configuration.
-     * @param url the URL of the configuration to use
-     * @deprecated please use setConfigUrl instead
-     */
-    // -@cs[AbbreviationAsWordInName] Should be removed at 7.0 version,
-    // we keep for some time to avoid braking compatibility.
-    @Deprecated
-    public void setConfigURL(URL url) {
-        setConfigUrl(url);
-    }
-
-    /**
-     * Sets URL to the configuration.
-     * @param url the URL of the configuration to use
-     */
-    public void setConfigUrl(URL url) {
-        setConfigLocation(url.toExternalForm());
-    }
-
-    /**
-     * Sets the location of the configuration.
-     * @param location the location, which is either a
-     */
-    private void setConfigLocation(String location) {
-        if (configLocation != null) {
-            throw new BuildException("Attributes 'config' and 'configURL' "
-                    + "must not be set at the same time");
+    public void setConfig(String configuration) {
+        if (config != null) {
+            throw new BuildException("Attribute 'config' has already been set");
         }
-        configLocation = location;
+        config = configuration;
     }
 
     /**
@@ -316,7 +286,7 @@ public void execute() {
                         "Must specify at least one of 'file' or nested 'fileset' or 'path'.",
                         getLocation());
             }
-            if (configLocation == null) {
+            if (config == null) {
                 throw new BuildException("Must specify 'config'.", getLocation());
             }
             realExecute(version);
@@ -381,7 +351,7 @@ private void processFiles(RootModule rootModule, final SeverityLevelCounter warn
 
         log("Running Checkstyle " + checkstyleVersion + " on " + files.size()
                 + " files", Project.MSG_INFO);
-        log("Using configuration " + configLocation, Project.MSG_VERBOSE);
+        log("Using configuration " + config, Project.MSG_VERBOSE);
 
         final int numErrs;
 
@@ -421,9 +391,9 @@ private RootModule createRootModule() {
         final RootModule rootModule;
         try {
             final Properties props = createOverridingProperties();
-            final Configuration config =
+            final Configuration configuration =
                 ConfigurationLoader.loadConfiguration(
-                    configLocation,
+                    config,
                     new PropertiesExpander(props),
                     !executeIgnoredModules);
 
@@ -433,7 +403,7 @@ private RootModule createRootModule() {
             final ModuleFactory factory = new PackageObjectFactory(
                     Checker.class.getPackage().getName() + ".", moduleClassLoader);
 
-            rootModule = (RootModule) factory.createModule(config.getName());
+            rootModule = (RootModule) factory.createModule(configuration.getName());
             rootModule.setModuleClassLoader(moduleClassLoader);
 
             if (rootModule instanceof Checker) {
@@ -443,11 +413,11 @@ private RootModule createRootModule() {
                 ((Checker) rootModule).setClassLoader(loader);
             }
 
-            rootModule.configure(config);
+            rootModule.configure(configuration);
         }
         catch (final CheckstyleException ex) {
             throw new BuildException(String.format(Locale.ROOT, "Unable to create Root Module: "
-                    + "configLocation {%s}, classpath {%s}.", configLocation, classpath), ex);
+                    + "config {%s}, classpath {%s}.", config, classpath), ex);
         }
         return rootModule;
     }


Test patch:
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTaskTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTaskTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTaskTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTaskTest.java
@@ -73,6 +73,7 @@ public class CheckstyleAntTaskTest extends BaseCheckTestSupport {
     private static final String WARNING_INPUT =
         "ant/checkstyleanttask/InputCheckstyleAntTaskWarning.java";
     private static final String CONFIG_FILE = "ant/ant_task_test_checks.xml";
+    private static final String CONFIG_RESOURCE = "/com/puppycrawl/tools/checkstyle/" + CONFIG_FILE;
     private static final String CUSTOM_ROOT_CONFIG_FILE = "config-custom-root-module.xml";
     private static final String NOT_EXISTING_FILE = "target/not_existing.xml";
     private static final String FAILURE_PROPERTY_VALUE = "myValue";
@@ -83,7 +84,7 @@ private CheckstyleAntTask getCheckstyleAntTask() throws IOException {
 
     private CheckstyleAntTask getCheckstyleAntTask(String configFile) throws IOException {
         final CheckstyleAntTask antTask = new CheckstyleAntTask();
-        antTask.setConfig(new File(getPath(configFile)));
+        antTask.setConfig(getPath(configFile));
         antTask.setProject(new Project());
         return antTask;
     }
@@ -184,7 +185,7 @@ public final void testNoConfigFile() throws IOException {
     @Test
     public final void testNonExistingConfig() throws IOException {
         final CheckstyleAntTask antTask = new CheckstyleAntTask();
-        antTask.setConfig(new File(getPath(NOT_EXISTING_FILE)));
+        antTask.setConfig(getPath(NOT_EXISTING_FILE));
         antTask.setProject(new Project());
         antTask.setFile(new File(getPath(FLAWLESS_INPUT)));
         try {
@@ -193,14 +194,14 @@ public final void testNonExistingConfig() throws IOException {
         }
         catch (BuildException ex) {
             assertTrue("Error message is unexpected",
-                    ex.getMessage().startsWith("Unable to create Root Module: configLocation"));
+                    ex.getMessage().startsWith("Unable to create Root Module: config"));
         }
     }
 
     @Test
     public final void testEmptyConfigFile() throws IOException {
         final CheckstyleAntTask antTask = new CheckstyleAntTask();
-        antTask.setConfig(new File(getPath("ant/empty_config.xml")));
+        antTask.setConfig(getPath("ant/empty_config.xml"));
         antTask.setProject(new Project());
         antTask.setFile(new File(getPath(FLAWLESS_INPUT)));
         try {
@@ -209,7 +210,7 @@ public final void testEmptyConfigFile() throws IOException {
         }
         catch (BuildException ex) {
             assertTrue("Error message is unexpected",
-                    ex.getMessage().startsWith("Unable to create Root Module: configLocation"));
+                    ex.getMessage().startsWith("Unable to create Root Module: config"));
         }
     }
 
@@ -253,7 +254,7 @@ public final void testMaxErrors() throws IOException {
     @Test
     public final void testFailureProperty() throws IOException {
         final CheckstyleAntTask antTask = new CheckstyleAntTask();
-        antTask.setConfig(new File(getPath(CONFIG_FILE)));
+        antTask.setConfig(getPath(CONFIG_FILE));
         antTask.setFile(new File(getPath(VIOLATED_INPUT)));
 
         final Project project = new Project();
@@ -323,32 +324,29 @@ public final void testConfigurationByUrl() throws IOException {
         final CheckstyleAntTask antTask = new CheckstyleAntTask();
         antTask.setProject(new Project());
         final URL url = new File(getPath(CONFIG_FILE)).toURI().toURL();
-        antTask.setConfigURL(url);
+        antTask.setConfig(url.toString());
+        antTask.setFile(new File(getPath(FLAWLESS_INPUT)));
+        antTask.execute();
+    }
+
+    @Test
+    public final void testConfigurationByResource() throws IOException {
+        final CheckstyleAntTask antTask = new CheckstyleAntTask();
+        antTask.setProject(new Project());
+        antTask.setConfig(CONFIG_RESOURCE);
         antTask.setFile(new File(getPath(FLAWLESS_INPUT)));
         antTask.execute();
     }
 
     @Test
     public final void testSimultaneousConfiguration() throws IOException {
-        CheckstyleAntTask antTask;
         final File file = new File(getPath(CONFIG_FILE));
         final URL url = file.toURI().toURL();
-        final String expected =
-                "Attributes 'config' and 'configURL' must not be set at the same time";
-        try {
-            antTask = new CheckstyleAntTask();
-            antTask.setConfigUrl(url);
-            antTask.setConfig(file);
-            fail("Exception is expected");
-        }
-        catch (BuildException ex) {
-            assertEquals("Error message is unexpected",
-                    expected, ex.getMessage());
-        }
+        final String expected = "Attribute 'config' has already been set";
         try {
-            antTask = new CheckstyleAntTask();
-            antTask.setConfig(file);
-            antTask.setConfigUrl(url);
+            final CheckstyleAntTask antTask = new CheckstyleAntTask();
+            antTask.setConfig(url.toString());
+            antTask.setConfig(file.toString());
             fail("Exception is expected");
         }
         catch (BuildException ex) {
@@ -515,7 +513,7 @@ public void testSetClasspathRef() {
     @Test
     public void testCheckerException() throws IOException {
         final CheckstyleAntTask antTask = new CheckstyleAntTaskStub();
-        antTask.setConfig(new File(getPath(CONFIG_FILE)));
+        antTask.setConfig(getPath(CONFIG_FILE));
         antTask.setProject(new Project());
         antTask.setFile(new File(""));
         try {
@@ -533,7 +531,7 @@ public final void testExecuteLogOutput() throws Exception {
         final CheckstyleAntTaskLogStub antTask = new CheckstyleAntTaskLogStub();
         final URL url = new File(getPath(CONFIG_FILE)).toURI().toURL();
         antTask.setProject(new Project());
-        antTask.setConfigURL(url);
+        antTask.setConfig(url.toString());
         antTask.setFile(new File(getPath(FLAWLESS_INPUT)));
 
         mockStatic(System.class);