Merge pull request #44 from uber/add-checkstyle

Add checkstyle and lint
uber-archive · Jul 9, 2015 · 28d4de8 · 28d4de8
2 parents 2099bc0 + 95f3961
commit 28d4de8
Show file tree

Hide file tree

Showing 15 changed files with 159 additions and 46 deletions.
diff --git a/config/checkstyle.xml b/config/checkstyle.xml
@@ -0,0 +1,44 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+        "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
+        "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+
+<module name="Checker">
+  <!-- Double blank lines -->
+  <module name="RegexpMultiline">
+    <property name="format" value="\n\n\n"/>
+    <property name="message" value="Double blank lines."/>
+  </module>
+
+  <module name="TreeWalker">
+    <module name="OneTopLevelClass"/>
+    <module name="NoLineWrap"/>
+    <module name="EmptyBlock"/>
+    <module name="Indentation"/>
+    <module name="MethodName"/>
+    <module name="EmptyLineSeparator">
+      <property name="allowNoEmptyLineBetweenFields" value="true"/>
+    </module>
+
+    <module name="AvoidEscapedUnicodeCharacters"/>
+    <module name="NeedBraces"/>
+    <module name="LeftCurly"/>
+    <module name="RightCurly"/>
+    <module name="LineLength">
+      <property name="max" value="120"/>
+      <property name="ignorePattern" value="^ *"/>
+    </module>
+    <module name="WhitespaceAround">
+      <property name="allowEmptyConstructors" value="true"/>
+      <property name="allowEmptyMethods" value="true"/>
+      <property name="allowEmptyTypes" value="true"/>
+      <property name="allowEmptyLoops" value="true"/>
+    </module>
+    <module name="OneStatementPerLine"/>
+    <module name="MultipleVariableDeclarations"/>
+    <module name="ArrayTypeStyle"/>
+    <module name="MissingSwitchDefault"/>
+    <module name="FallThrough"/>
+    <module name="GenericWhitespace"/>
+  </module>
+</module>
diff --git a/pom.xml b/pom.xml
@@ -81,9 +81,9 @@
   <build>
     <plugins>
       <plugin>
-      <groupId>org.eluder.coveralls</groupId>
-      <artifactId>coveralls-maven-plugin</artifactId>
-      <version>3.1.0</version>
+        <groupId>org.eluder.coveralls</groupId>
+        <artifactId>coveralls-maven-plugin</artifactId>
+        <version>3.1.0</version>
       </plugin>
 
       <plugin>
@@ -100,6 +100,28 @@
           <aggregate>true</aggregate>
         </configuration>
       </plugin>
+
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <version>2.15</version>
+        <configuration>
+          <configLocation>config/checkstyle.xml</configLocation>
+          <encoding>UTF-8</encoding>
+          <consoleOutput>true</consoleOutput>
+          <failsOnError>true</failsOnError>
+          <includeTestSourceDirectory>true</includeTestSourceDirectory>
+        </configuration>
+        <executions>
+          <execution>
+            <id>validate</id>
+            <phase>validate</phase>
+            <goals>
+              <goal>check</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
     </plugins>
   </build>
 

diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapperDescriptor.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapperDescriptor.java
@@ -26,7 +26,6 @@
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.StaplerRequest;
 
-
 @SuppressWarnings("UnusedDeclaration")
 @Extension // This indicates to Jenkins that this is an implementation of an extension point.
 public final class PhabricatorBuildWrapperDescriptor extends BuildWrapperDescriptor {
@@ -63,18 +62,23 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc
     public String getConduitURL() {
         return conduitURL;
     }
-    public void setConduitURL(String value) { conduitURL = value; }
+
+    public void setConduitURL(String value) {
+        conduitURL = value;
+    }
 
     public String getConduitToken() {
         return conduitToken;
     }
+
     public void setConduitToken(String conduitToken) {
         this.conduitToken = conduitToken;
     }
 
     public String getArcPath() {
         return arcPath;
     }
+
     public void setArcPath(String arcPath) {
         this.arcPath = arcPath;
     }

diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
@@ -193,7 +193,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
                     comment = String.format("%s\n\n```\n%s\n```\n", comment, customComment);
                 }
             }
-        } catch(InterruptedException e) {
+        } catch (InterruptedException e) {
             e.printStackTrace(logger.getStream());
         } catch (IOException e) {
             Util.displayIOException(e, listener);
@@ -214,7 +214,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
             } catch (ArcanistUsageException e) {
                 logger.info("arcanist", "unable to post comment");
             }
-            if(!(result.get("errorMessage") instanceof JSONNull)) {
+            if (!(result.get("errorMessage") instanceof JSONNull)) {
                 logger.info("arcanist", "Get error " + result.get("errorMessage") + " with action " +
                         commentAction + "; trying again with action 'none'");
                 try {

diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifierDescriptor.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifierDescriptor.java
@@ -27,7 +27,6 @@
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.StaplerRequest;
 
-
 /**
  * Descriptor for {@link PhabricatorNotifier}. Used as a singleton.
  * The class is marked as public so that it can be accessed from views.
@@ -85,15 +84,21 @@ public String getConduitURL() {
         }
         return null;
     }
-    public void setConduitURL(String value) { conduitURL = value; }
+
+    public void setConduitURL(String value) {
+        conduitURL = value;
+    }
 
     public String getUberallsURL() {
         if (uberallsURL != null && !"".equals(uberallsURL)) {
             return uberallsURL;
         }
         return null;
     }
-    public void setUberallsURL(String value) { uberallsURL = value; }
+
+    public void setUberallsURL(String value) {
+        uberallsURL = value;
+    }
 
     public void setCommentFile(String commentFile) {
         this.commentFile = commentFile;

diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java
@@ -37,8 +37,12 @@ public class PhabricatorPlugin extends Plugin {
     public static final String ARCANIST_PATH = "ARCANIST_PATH";
 
     public static String getIconPath(String icon) {
-        if(icon == null) return null;
-        if(icon.startsWith("/")) return icon;
+        if (icon == null) {
+            return null;
+        }
+        if (icon.startsWith("/")) {
+            return icon;
+        }
 
         // Try plugin images dir, fallback to Hudson images dir
         PluginWrapper wrapper = Jenkins.getInstance().getPluginManager().getPlugin(PhabricatorPlugin.class);

diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorPostbuildAction.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorPostbuildAction.java
@@ -46,17 +46,47 @@ public static PhabricatorPostbuildAction createShortText(String text, String lin
     }
 
     /* Action methods */
-    public String getUrlName() { return ""; }
-    public String getDisplayName() { return ""; }
-    public String getIconFileName() { return null; }
-
-    @Exported public boolean isTextOnly() { return (iconPath == null); }
-    @Exported public String getIconPath() { return iconPath; }
-    @Exported public String getText() { return text; }
-    @Exported public String getColor() { return color; }
-    @Exported public String getBackground() { return background; }
-    @Exported public String getBorder() { return border; }
-    @Exported public String getBorderColor() { return borderColor; }
-    @Exported public String getLink() { return link; }
+    public String getUrlName() {
+        return "";
+    }
+
+    public String getDisplayName() {
+        return "";
+    }
+
+    public String getIconFileName() {
+        return null;
+    }
+
+    @Exported public boolean isTextOnly() {
+        return (iconPath == null);
+    }
+
+    @Exported public String getIconPath() {
+        return iconPath;
+    }
+
+    @Exported public String getText() {
+        return text;
+    }
+
+    @Exported public String getColor() {
+        return color;
+    }
+
+    @Exported public String getBackground() {
+        return background;
+    }
 
+    @Exported public String getBorder() {
+        return border;
+    }
+
+    @Exported public String getBorderColor() {
+        return borderColor;
+    }
+
+    @Exported public String getLink() {
+        return link;
+    }
 }
diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorPostbuildSummaryAction.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorPostbuildSummaryAction.java
@@ -21,14 +21,13 @@
 package com.uber.jenkins.phabricator;
 
 import hudson.model.Action;
-import org.apache.commons.lang.StringEscapeUtils;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
 /**
  * Ripped from https://github.com/jenkinsci/groovy-postbuild-plugin/blob/master/src/main/java/org/jvnet/hudson/plugins/groovypostbuild/GroovyPostbuildSummaryAction.java
  */
-@ExportedBean(defaultVisibility=2)
+@ExportedBean(defaultVisibility = 2)
 public class PhabricatorPostbuildSummaryAction implements Action {
     private final String iconPath;
     private final StringBuilder textBuilder = new StringBuilder();
@@ -38,17 +37,27 @@ public PhabricatorPostbuildSummaryAction(String iconPath) {
     }
 
     /* Action methods */
-    public String getUrlName() { return ""; }
-    public String getDisplayName() { return ""; }
-    public String getIconFileName() { return null; }
+    public String getUrlName() {
+        return "";
+    }
+
+    public String getDisplayName() {
+        return "";
+    }
+
+    public String getIconFileName() {
+        return null;
+    }
+
+    @Exported public String getIconPath() {
+        return iconPath;
+    }
 
-    @Exported public String getIconPath() { return iconPath; }
-    @Exported public String getText() { return textBuilder.toString(); }
+    @Exported public String getText() {
+        return textBuilder.toString();
+    }
 
     public void appendText(String text) {
-        if(false) {
-            text = StringEscapeUtils.escapeHtml(text);
-        }
         textBuilder.append(text);
     }
 }
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java
@@ -96,7 +96,7 @@ public JSONObject parseConduit(Launcher.ProcStarter starter, PrintStream stderr)
         JsonSlurper jsonParser = new JsonSlurper();
         try {
             return (JSONObject) jsonParser.parseText(stdoutBuffer.toString());
-        } catch(net.sf.json.JSONException e) {
+        } catch (net.sf.json.JSONException e) {
             stderr.println("[arcanist] Unable to parse JSON from response: " + stdoutBuffer.toString());
             throw e;
         }

diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistUsageException.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistUsageException.java
@@ -18,7 +18,6 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-
 package com.uber.jenkins.phabricator.conduit;
 
 public class ArcanistUsageException extends Exception {

diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java b/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
@@ -34,7 +34,6 @@
 public class Differential {
     private JSONObject rawJSON;
 
-
     public Differential(JSONObject rawJSON) throws ArcanistUsageException, IOException, InterruptedException {
         this.rawJSON = rawJSON;
     }
@@ -115,7 +114,7 @@ public String getBranch() {
         }
         try {
             return (String) branchName;
-        } catch(ClassCastException e) {
+        } catch (ClassCastException e) {
             return "(unknown)";
         }
     }

diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java
@@ -100,7 +100,6 @@ public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws IOEx
         return this.callConduit("sendHarbormasterMessage.sendmessage", params);
     }
 
-
     /**
      * Post a comment on the differential
      * @param message the string message to post

diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/NonDifferentialBuildTask.java b/src/main/java/com/uber/jenkins/phabricator/tasks/NonDifferentialBuildTask.java
@@ -24,7 +24,6 @@
 import com.uber.jenkins.phabricator.uberalls.UberallsClient;
 import com.uber.jenkins.phabricator.utils.CommonUtils;
 import com.uber.jenkins.phabricator.utils.Logger;
-import hudson.plugins.cobertura.targets.CoverageResult;
 
 /**
  * Generic build task.
@@ -82,7 +81,7 @@ protected void setup() {
      */
     @Override
     protected void execute() {
-        if (result == Result.UNKNWON) {
+        if (result == Result.UNKNOWN) {
             if (!CommonUtils.isBlank(commitSha)) {
                 info(String.format("Sending coverage result for %s as %s", commitSha,
                         codeCoverageMetrics.toString()));

diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/Task.java b/src/main/java/com/uber/jenkins/phabricator/tasks/Task.java
@@ -35,10 +35,10 @@ public enum Result {
         FAILURE,
         IGNORED, // For incorrect input.
         SKIPPED, // For incorrect configuration.
-        UNKNWON
-    };
+        UNKNOWN
+    }
 
-    protected Result result = Result.UNKNWON;
+    protected Result result = Result.UNKNOWN;
     private Logger logger;
 
     /**

diff --git a/src/main/java/com/uber/jenkins/phabricator/uberalls/UberallsClient.java b/src/main/java/com/uber/jenkins/phabricator/uberalls/UberallsClient.java
@@ -23,7 +23,6 @@
 import com.uber.jenkins.phabricator.CodeCoverageMetrics;
 import com.uber.jenkins.phabricator.conduit.Differential;
 import com.uber.jenkins.phabricator.utils.Logger;
-import hudson.EnvVars;
 import net.sf.json.JSON;
 import net.sf.json.JSONNull;
 import net.sf.json.JSONObject;
@@ -40,7 +39,6 @@
 import org.apache.http.entity.ContentType;
 
 import java.io.IOException;
-import java.io.PrintStream;
 import java.net.URISyntaxException;
 
 public class UberallsClient {
@@ -132,6 +130,7 @@ public boolean recordCoverage(String sha, CodeCoverageMetrics codeCoverageMetric
                 e.printStackTrace();
             } catch (HttpResponseException e) {
                 // e.g. 404, pass
+                logger.info(TAG, "HTTP Response error recording metrics: " + e);
             } catch (ClientProtocolException e) {
                 e.printStackTrace();
             } catch (IOException e) {