Finding findbugsmessages and correcting codenarc

* Was finding findbugs messages xml incorrectly in classpath. * Was not handling codenarc reports with empty line numbers.
tomasbjerre · Feb 16, 2017 · 60d19fe · 60d19fe
1 parent 84e5053
commit 60d19fe
Show file tree

Hide file tree

Showing 4 changed files with 165 additions and 145 deletions.
diff --git a/src/main/java/se/bjurr/violations/lib/parsers/CodeNarcParser.java b/src/main/java/se/bjurr/violations/lib/parsers/CodeNarcParser.java
@@ -80,7 +80,11 @@ public List<Violation> parseReportOutput(String string) throws Exception {
      if (xmlr.getLocalName().equals("Violation")) {
       ruleName = getAttribute(xmlr, "ruleName");
       priority = getIntegerAttribute(xmlr, "priority");
-      lineNumber = getIntegerAttribute(xmlr, "lineNumber");
+      String lineNumberString = getAttribute(xmlr, "lineNumber");
+      lineNumber = 1;
+      if (lineNumberString != null && !lineNumberString.isEmpty()) {
+        lineNumber = Integer.parseInt(lineNumberString);
+      }
       String message = rules.get(ruleName);
       if (message == null) {
        message = ruleName;

diff --git a/src/main/java/se/bjurr/violations/lib/parsers/FindbugsParser.java b/src/main/java/se/bjurr/violations/lib/parsers/FindbugsParser.java
@@ -1,22 +1,24 @@
 package se.bjurr.violations.lib.parsers;
 
+import static java.util.logging.Level.SEVERE;
 import static se.bjurr.violations.lib.model.Violation.violationBuilder;
 import static se.bjurr.violations.lib.parsers.ViolationParserUtils.findIntegerAttribute;
 import static se.bjurr.violations.lib.parsers.ViolationParserUtils.getAttribute;
 import static se.bjurr.violations.lib.parsers.ViolationParserUtils.getChunks;
 import static se.bjurr.violations.lib.parsers.ViolationParserUtils.getContent;
 import static se.bjurr.violations.lib.parsers.ViolationParserUtils.getIntegerAttribute;
 import static se.bjurr.violations.lib.reports.Reporter.FINDBUGS;
-import static se.bjurr.violations.lib.util.Utils.getResource;
 import static se.bjurr.violations.lib.util.Utils.isNullOrEmpty;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Logger;
 
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamConstants;
@@ -29,126 +31,131 @@
 import se.bjurr.violations.lib.util.Utils;
 
 public class FindbugsParser implements ViolationsParser {
-
- /**
-  * Severity rank.
-  */
- public static final String FINDBUGS_SPECIFIC_RANK = "RANK";
- private static String findbugsMessagesXml;
-
- public static void setFindbugsMessagesXml(String findbugsMessagesXml) {
-  FindbugsParser.findbugsMessagesXml = findbugsMessagesXml;
- }
-
- private Map<String, String> getMessagesPerType() {
-  Map<String, String> messagesPerType = new HashMap<>();
-  try {
-   if (isNullOrEmpty(findbugsMessagesXml)) {
-    findbugsMessagesXml = Utils.toString(getResource("findbugs/messages.xml"));
-   }
-   List<String> bugPatterns = getChunks(findbugsMessagesXml, "<BugPattern", "</BugPattern>");
-   for (String bugPattern : bugPatterns) {
-    String type = getAttribute(bugPattern, "type");
-    String shortDescription = getContent(bugPattern, "ShortDescription");
-    String details = getContent(bugPattern, "Details");
-    messagesPerType.put(type, shortDescription + "\n\n" + details);
-   }
-  } catch (IOException e) {
-   throw new RuntimeException(e);
+private static Logger LOG = Logger.getLogger(FindbugsParser.class.getSimpleName());
+  /**
+   * Severity rank.
+   */
+  public static final String FINDBUGS_SPECIFIC_RANK = "RANK";
+  private static String findbugsMessagesXml;
+
+  public static void setFindbugsMessagesXml(String findbugsMessagesXml) {
+    FindbugsParser.findbugsMessagesXml = findbugsMessagesXml;
   }
-  return messagesPerType;
- }
-
- private void parseBugInstance(XMLStreamReader xmlr, List<Violation> violations, Map<String, String> messagesPerType)
-   throws XMLStreamException {
-  String type = getAttribute(xmlr, "type");
-  Integer rank = getIntegerAttribute(xmlr, "rank");
-  String message = messagesPerType.get(type);
-  if (message == null) {
-   message = type;
+
+  private Map<String, String> getMessagesPerType() {
+    Map<String, String> messagesPerType = new HashMap<>();
+    try {
+      if (isNullOrEmpty(findbugsMessagesXml)) {
+        String messagesResourceFilename = "/findbugs/messages.xml";
+        URL resource = FindbugsParser.class.getResource(messagesResourceFilename);
+        if (resource == null) {
+          throw new RuntimeException("Unable to find resource " + messagesResourceFilename);
+        }
+        findbugsMessagesXml = Utils.toString(resource);
+      }
+      List<String> bugPatterns = getChunks(findbugsMessagesXml, "<BugPattern", "</BugPattern>");
+      for (String bugPattern : bugPatterns) {
+        String type = getAttribute(bugPattern, "type");
+        String shortDescription = getContent(bugPattern, "ShortDescription");
+        String details = getContent(bugPattern, "Details");
+        messagesPerType.put(type, shortDescription + "\n\n" + details);
+      }
+    } catch (IOException e) {
+      LOG.log(SEVERE, e.getMessage(), e);
+    }
+    return messagesPerType;
   }
-  SEVERITY severity = toSeverity(rank);
-
-  List<Violation> candidates = new ArrayList<>();
-
-  while (xmlr.hasNext()) {
-   int eventType = xmlr.next();
-   if (eventType == XMLStreamConstants.START_ELEMENT) {
-    if (xmlr.getLocalName().equals("SourceLine")) {
-     Optional<Integer> startLine = findIntegerAttribute(xmlr, "start");
-     Optional<Integer> endLine = findIntegerAttribute(xmlr, "end");
-     if (!startLine.isPresent() || !endLine.isPresent()) {
-      continue;
-     }
-     String filename = getAttribute(xmlr, "sourcepath");
-     String classname = getAttribute(xmlr, "classname");
-     candidates.add(//
-       violationBuilder()//
-         .setReporter(FINDBUGS)//
-         .setMessage(message)//
-         .setFile(filename)//
-         .setStartLine(startLine.get())//
-         .setEndLine(endLine.get())//
-         .setRule(type)//
-         .setSeverity(severity)//
-         .setSource(classname)//
-         .setSpecific(FINDBUGS_SPECIFIC_RANK, rank)//
-         .build()//
-     );
+
+  private void parseBugInstance(XMLStreamReader xmlr, List<Violation> violations,
+      Map<String, String> messagesPerType) throws XMLStreamException {
+    String type = getAttribute(xmlr, "type");
+    Integer rank = getIntegerAttribute(xmlr, "rank");
+    String message = messagesPerType.get(type);
+    if (message == null) {
+      message = type;
     }
-   }
-   if (eventType == XMLStreamConstants.END_ELEMENT) {
-    if (xmlr.getLocalName().equals("BugInstance")) {
-     // End of the bug instance.
-     break;
+    SEVERITY severity = toSeverity(rank);
+
+    List<Violation> candidates = new ArrayList<>();
+
+    while (xmlr.hasNext()) {
+      int eventType = xmlr.next();
+      if (eventType == XMLStreamConstants.START_ELEMENT) {
+        if (xmlr.getLocalName().equals("SourceLine")) {
+          Optional<Integer> startLine = findIntegerAttribute(xmlr, "start");
+          Optional<Integer> endLine = findIntegerAttribute(xmlr, "end");
+          if (!startLine.isPresent() || !endLine.isPresent()) {
+            continue;
+          }
+          String filename = getAttribute(xmlr, "sourcepath");
+          String classname = getAttribute(xmlr, "classname");
+          candidates.add(//
+              violationBuilder()//
+                  .setReporter(FINDBUGS)//
+                  .setMessage(message)//
+                  .setFile(filename)//
+                  .setStartLine(startLine.get())//
+                  .setEndLine(endLine.get())//
+                  .setRule(type)//
+                  .setSeverity(severity)//
+                  .setSource(classname)//
+                  .setSpecific(FINDBUGS_SPECIFIC_RANK, rank)//
+                  .build()//
+          );
+        }
+      }
+      if (eventType == XMLStreamConstants.END_ELEMENT) {
+        if (xmlr.getLocalName().equals("BugInstance")) {
+          // End of the bug instance.
+          break;
+        }
+      }
     }
-   }
-  }
 
-  if (!candidates.isEmpty()) {
-   /**
-    * Last one is the most specific, first 2 may be class and method when the
-    * third is source line.
-    */
-   violations.add(candidates.get(candidates.size() - 1));
-  }
+    if (!candidates.isEmpty()) {
+      /**
+       * Last one is the most specific, first 2 may be class and method when the third is source
+       * line.
+       */
+      violations.add(candidates.get(candidates.size() - 1));
+    }
 
- }
+  }
 
- @Override
- public List<Violation> parseReportOutput(String string) throws Exception {
-  List<Violation> violations = new ArrayList<>();
-  Map<String, String> messagesPerType = getMessagesPerType();
+  @Override
+  public List<Violation> parseReportOutput(String string) throws Exception {
+    List<Violation> violations = new ArrayList<>();
+    Map<String, String> messagesPerType = getMessagesPerType();
 
-  try (InputStream input = new ByteArrayInputStream(string.getBytes())) {
+    try (InputStream input = new ByteArrayInputStream(string.getBytes())) {
 
-   XMLInputFactory factory = XMLInputFactory.newInstance();
-   XMLStreamReader xmlr = factory.createXMLStreamReader(input);
+      XMLInputFactory factory = XMLInputFactory.newInstance();
+      XMLStreamReader xmlr = factory.createXMLStreamReader(input);
 
-   while (xmlr.hasNext()) {
-    int eventType = xmlr.next();
-    if (eventType == XMLStreamConstants.START_ELEMENT) {
-     if (xmlr.getLocalName().equals("BugInstance")) {
-      parseBugInstance(xmlr, violations, messagesPerType);
-     }
+      while (xmlr.hasNext()) {
+        int eventType = xmlr.next();
+        if (eventType == XMLStreamConstants.START_ELEMENT) {
+          if (xmlr.getLocalName().equals("BugInstance")) {
+            parseBugInstance(xmlr, violations, messagesPerType);
+          }
+        }
+      }
     }
-   }
-  }
-  return violations;
- }
-
- /**
-  * Bugs are given a rank 1-20, and grouped into the categories scariest (rank
-  * 1-4), scary (rank 5-9), troubling (rank 10-14), and of concern (rank 15-20).
-  */
- private SEVERITY toSeverity(Integer rank) {
-  if (rank <= 9) {
-   return SEVERITY.ERROR;
+    return violations;
   }
-  if (rank <= 14) {
-   return SEVERITY.WARN;
+
+  /**
+   * Bugs are given a rank 1-20, and grouped into the categories scariest (rank 1-4), scary (rank
+   * 5-9), troubling (rank 10-14), and of concern (rank 15-20).
+   */
+  private SEVERITY toSeverity(Integer rank) {
+    if (rank <= 9) {
+      return SEVERITY.ERROR;
+    }
+    if (rank <= 14) {
+      return SEVERITY.WARN;
+    }
+    return SEVERITY.INFO;
   }
-  return SEVERITY.INFO;
- }
 
 }
diff --git a/src/main/java/se/bjurr/violations/lib/util/Utils.java b/src/main/java/se/bjurr/violations/lib/util/Utils.java
@@ -2,46 +2,55 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URL;
 import java.util.Scanner;
 
 public class Utils {
- public static <T> T checkNotNull(T reference, String errorMessage) {
-  if (reference == null) {
-   throw new NullPointerException(errorMessage);
+  public static <T> T checkNotNull(T reference, String errorMessage) {
+    if (reference == null) {
+      throw new NullPointerException(errorMessage);
+    }
+    return reference;
   }
-  return reference;
- }
 
- public static String emptyToNull(String str) {
-  if (str == null || str.isEmpty()) {
-   return null;
+  public static String emptyToNull(String str) {
+    if (str == null || str.isEmpty()) {
+      return null;
+    }
+    return str;
   }
-  return str;
- }
 
- public static <T> T firstNonNull(T f, T s) {
-  if (f != null) {
-   return f;
+  public static <T> T firstNonNull(T f, T s) {
+    if (f != null) {
+      return f;
+    }
+    return s;
+  }
+
+  @SuppressWarnings("static-access")
+  public static InputStream getResource(String filename) {
+    return Thread.currentThread().getContextClassLoader().getSystemResourceAsStream(filename);
+  }
+
+  public static boolean isNullOrEmpty(String findbugsMessagesXml) {
+    return findbugsMessagesXml == null || findbugsMessagesXml.isEmpty();
+  }
+
+  @SuppressWarnings("resource")
+  public static String toString(InputStream inputStream) throws IOException {
+    Scanner scanner = new Scanner(inputStream).useDelimiter("\\A");
+    String result = scanner.hasNext() ? scanner.next() : "";
+    scanner.close();
+    inputStream.close();
+
+    return result;
+  }
+
+  public static String toString(URL resource) throws IOException {
+    try {
+      return toString(resource.openStream());
+    } catch (IOException e) {
+      throw e;
+    }
   }
-  return s;
- }
-
- @SuppressWarnings("static-access")
- public static InputStream getResource(String filename) {
-  return Thread.currentThread().getContextClassLoader().getSystemResourceAsStream(filename);
- }
-
- public static boolean isNullOrEmpty(String findbugsMessagesXml) {
-  return findbugsMessagesXml == null || findbugsMessagesXml.isEmpty();
- }
-
- @SuppressWarnings("resource")
- public static String toString(InputStream inputStream) throws IOException {
-  Scanner scanner = new Scanner(inputStream).useDelimiter("\\A");
-  String result = scanner.hasNext() ? scanner.next() : "";
-  scanner.close();
-  inputStream.close();
-
-  return result;
- }
 }
diff --git a/src/test/java/se/bjurr/violations/lib/CodeNarcTest.java b/src/test/java/se/bjurr/violations/lib/CodeNarcTest.java
@@ -25,7 +25,7 @@ public void testThatViolationsCanBeParsed() {
     .violations();
 
   assertThat(actual)//
-    .hasSize(12);
+    .hasSize(32);
 
   assertThat(actual.get(0).getMessage())//
     .isEqualTo("In most cases, exceptions should not be caught and ignored (swallowed).");