[FIXED JENKINS-19127] Improved JSLint parser to support checkstyle

format.
jenkinsci · Aug 14, 2013 · 390dc75 · 390dc75
1 parent d6ca01a
commit 390dc75
Show file tree

Hide file tree

Showing 5 changed files with 420 additions and 54 deletions.
diff --git a/pom.xml b/pom.xml
@@ -36,7 +36,7 @@
     <dependency>
       <groupId>org.jvnet.hudson.plugins</groupId>
       <artifactId>analysis-core</artifactId>
-      <version>1.51</version>
+      <version>1.52</version>
     </dependency>
     <dependency>
       <groupId>org.jvnet.hudson.plugins</groupId>

diff --git a/src/main/java/hudson/plugins/warnings/parser/JSLintXMLSaxParser.java b/src/main/java/hudson/plugins/warnings/parser/JSLintXMLSaxParser.java
@@ -1,7 +1,6 @@
 package hudson.plugins.warnings.parser;
 
 import java.util.List;
-import java.util.logging.Logger;
 
 import org.apache.commons.lang.StringUtils;
 import org.xml.sax.Attributes;
@@ -15,15 +14,14 @@
  * Handles parsing.
  */
 public class JSLintXMLSaxParser extends DefaultHandler {
-    private static final Logger LOGGER = Logger.getLogger(JSLintXMLSaxParser.class.toString());
     private final List<FileAnnotation> warnings;
     private String fileName;
     private final String type;
 
     /** Categories. */
-    private static final String CATEGORY_PARSING = "Parsing";
-    private static final String CATEGORY_UNDEFINED_VARIABLE = "Undefined Variable";
-    private static final String CATEGORY_FORMATTING = "Formatting";
+    static final String CATEGORY_PARSING = "Parsing";
+    static final String CATEGORY_UNDEFINED_VARIABLE = "Undefined Variable";
+    static final String CATEGORY_FORMATTING = "Formatting";
 
     /**
      * Creates a new instance of {@link JSLintXMLSaxParser}.
@@ -48,43 +46,57 @@ public void startElement(final String namespaceURI, final String localName, fina
         if (isLintDerivate(key)) {
             return; // Start element, good to skip
         }
+
         if ("file".equals(key)) {
             fileName = atts.getValue("name");
             return;
         }
-        if ("issue".equals(key)) {
-            String category = StringUtils.EMPTY;
-            Priority priority = Priority.NORMAL;
-
-            String message = atts.getValue("reason");
-            if (message.startsWith("Expected")) {
-                priority = Priority.HIGH;
-                category = CATEGORY_PARSING;
-            }
-            else if (message.endsWith(" is not defined.")) {
-                priority = Priority.HIGH;
-                category = CATEGORY_UNDEFINED_VARIABLE;
-            }
-            else if (message.contains("Mixed spaces and tabs")) {
-                priority = Priority.LOW;
-                category = CATEGORY_FORMATTING;
-            }
-            else if (message.contains("Unnecessary semicolon")) {
-                category = CATEGORY_FORMATTING;
-            }
-            else if (message.contains("is better written in dot notation")) {
-                category = CATEGORY_FORMATTING;
-            }
-
-            int lineNumber = AbstractWarningsParser.convertLineNumber(atts.getValue("line"));
-            Warning warning = new Warning(fileName, lineNumber, type, category, message, priority);
-
-            warnings.add(warning);
-            return;
+
+        if ("issue".equals(key) || "error".equals(key)) {
+            createWarning(atts);
+        }
+    }
+
+    private void createWarning(final Attributes attributes) {
+        String category = StringUtils.EMPTY;
+        Priority priority = Priority.NORMAL;
+
+        String message = extractFrom(attributes, "reason", "message");
+        if (message.startsWith("Expected")) {
+            priority = Priority.HIGH;
+            category = CATEGORY_PARSING;
+        }
+        else if (message.endsWith(" is not defined.")) {
+            priority = Priority.HIGH;
+            category = CATEGORY_UNDEFINED_VARIABLE;
         }
-        else {
-            LOGGER.info("Unknown jslint xml tag: " + key);
+        else if (message.contains("Mixed spaces and tabs")) {
+            priority = Priority.LOW;
+            category = CATEGORY_FORMATTING;
+        }
+        else if (message.contains("Unnecessary semicolon")) {
+            category = CATEGORY_FORMATTING;
+        }
+        else if (message.contains("is better written in dot notation")) {
+            category = CATEGORY_FORMATTING;
+        }
+
+        int lineNumber = AbstractWarningsParser.convertLineNumber(attributes.getValue("line"));
+        Warning warning = new Warning(fileName, lineNumber, type, category, message, priority);
+
+        String column = extractFrom(attributes, "column", "char");
+        if (StringUtils.isNotBlank(column)) {
+            warning.setColumnPosition(AbstractWarningsParser.convertLineNumber(column));
+        }
+        warnings.add(warning);
+    }
+
+    private String extractFrom(final Attributes atts, final String first, final String second) {
+        String value = atts.getValue(first);
+        if (StringUtils.isEmpty(value)) {
+            value = atts.getValue(second);
         }
+        return value;
     }
 
     private boolean isLintDerivate(final String key) {

diff --git a/src/test/java/hudson/plugins/warnings/parser/JSLintParserTest.java b/src/test/java/hudson/plugins/warnings/parser/JSLintParserTest.java
@@ -4,13 +4,18 @@
 
 import java.io.IOException;
 import java.util.Collection;
-import java.util.Set;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
 
 import org.junit.Test;
 
-import com.google.common.collect.Sets;
+import com.google.common.collect.Lists;
 
+import hudson.plugins.analysis.util.model.DefaultAnnotationContainer;
 import hudson.plugins.analysis.util.model.FileAnnotation;
+import hudson.plugins.analysis.util.model.Priority;
+import hudson.plugins.analysis.util.model.WorkspaceFile;
 
 /**
  * Tests the class {@link JSLintParser}.
@@ -20,6 +25,25 @@
 public class JSLintParserTest extends ParserTester {
     private static final String EXPECTED_FILE_NAME = "duckworth/hudson-jslint-freestyle/src/prototype.js";
 
+    /**
+     * Parses a file with one warning that are started by ant.
+     *
+     * @throws IOException
+     *      if the file could not be read
+     * @see <a href="http://issues.jenkins-ci.org/browse/JENKINS-19127">Issue 19127</a>
+     */
+    @Test
+    public void issue19127() throws IOException {
+        Collection<FileAnnotation> warnings = new JSLintParser().parse(openFile("jslint/jslint.xml"));
+
+        assertEquals(WRONG_NUMBER_OF_WARNINGS_DETECTED, 197, warnings.size());
+
+        Iterator<FileAnnotation> iterator = warnings.iterator();
+        checkWarning(iterator.next(), 3, 5,
+                "'window' is not defined.", "C:/DVR/lint_Mobile-Localization_ws/evWebService/WebClientApi/api-v1.js",
+                JSLintXMLSaxParser.CATEGORY_UNDEFINED_VARIABLE, Priority.HIGH);
+    }
+
     /**
      * Tests the JS-Lint parsing for warnings in different files.
      *
@@ -31,15 +55,23 @@ public void testParse() throws IOException {
         Collection<FileAnnotation> results = createParser().parse(openFile());
         assertEquals(WRONG_NUMBER_OF_WARNINGS_DETECTED, 102, results.size());
 
-        Set<String> files = Sets.newHashSet();
+        DefaultAnnotationContainer container = new DefaultAnnotationContainer(results);
+        Collection<WorkspaceFile> files = container.getFiles();
+        assertEquals("Wrong number of files", 2, files.size());
 
-        for (FileAnnotation warning : results) {
-            files.add(warning.getFileName());
-        }
+        List<WorkspaceFile> sortedFiles = Lists.newArrayList(files);
+        Collections.sort(sortedFiles);
 
-        assertEquals("Wrong number of files", 2, files.size());
-        assertTrue("File not found", files.contains(EXPECTED_FILE_NAME));
-        assertTrue("File not found", files.contains("duckworth/hudson-jslint-freestyle/src/scriptaculous.js"));
+        verifyFileName(sortedFiles, EXPECTED_FILE_NAME, 0);
+        verifyFileName(sortedFiles, "duckworth/hudson-jslint-freestyle/src/scriptaculous.js", 1);
+
+        FileAnnotation firstWarning = results.iterator().next();
+        checkWarning(firstWarning, 10, 3, "Expected 'Version' to have an indentation at 5 instead at 3.",
+                EXPECTED_FILE_NAME, JSLintXMLSaxParser.CATEGORY_PARSING, Priority.HIGH);
+    }
+
+    private void verifyFileName(final List<WorkspaceFile> sortedFiles, final String expectedName, final int position) {
+        assertEquals("Wrong file found: ", expectedName, sortedFiles.get(position).getName());
     }
 
     /**

diff --git a/src/test/java/hudson/plugins/warnings/parser/ParserTester.java b/src/test/java/hudson/plugins/warnings/parser/ParserTester.java
@@ -15,7 +15,7 @@
  * Base class for parser tests. Provides an assertion test for warnings.
  */
 public abstract class ParserTester {
-    static final String WRONG_NUMBER_OF_WARNINGS_DETECTED = "Wrong number of warnings detected.";
+    static final String WRONG_NUMBER_OF_WARNINGS_DETECTED = "Wrong number of warnings detected: ";
 
     /**
      * Checks the properties of the specified warning.
@@ -35,13 +35,41 @@ public abstract class ParserTester {
      */
     protected void checkWarning(final FileAnnotation annotation, final int lineNumber, final String message, final String fileName, final String category, final Priority priority) {
         Warning warning = castWarning(annotation);
-        assertEquals("Wrong priority detected.", priority, warning.getPriority());
-        assertEquals("Wrong category of warning detected.", category, warning.getCategory());
-        assertEquals("Wrong number of ranges detected.", 1, warning.getLineRanges().size());
-        assertEquals("Wrong ranges start detected.", lineNumber, warning.getLineRanges().iterator().next().getStart());
-        assertEquals("Wrong ranges end detected.", lineNumber, warning.getLineRanges().iterator().next().getEnd());
-        assertEquals("Wrong message detected.", StringEscapeUtils.escapeXml(message), warning.getMessage());
-        assertEquals("Wrong filename detected.", fileName, warning.getFileName());
+        assertEquals("Wrong priority detected: ", priority, warning.getPriority());
+        assertEquals("Wrong category of warning detected: ", category, warning.getCategory());
+        assertEquals("Wrong number of ranges detected: ", 1, warning.getLineRanges().size());
+        assertEquals("Wrong ranges start detected: ", lineNumber, warning.getLineRanges().iterator().next().getStart());
+        assertEquals("Wrong ranges end detected: ", lineNumber, warning.getLineRanges().iterator().next().getEnd());
+        assertEquals("Wrong message detected: ", StringEscapeUtils.escapeXml(message), warning.getMessage());
+        assertEquals("Wrong filename detected: ", fileName, warning.getFileName());
+    }
+
+    /**
+     * Checks the properties of the specified warning.
+     *
+     * @param annotation
+     *            the warning to check
+     * @param lineNumber
+     *            the expected line number
+     * @param column
+     *            the expected column
+     * @param message
+     *            the expected message
+     * @param fileName
+     *            the expected filename
+     * @param category
+     *            the expected category
+     * @param priority
+     *            the expected priority
+     */
+    protected void checkWarning(final FileAnnotation annotation, final int lineNumber, final int column, final String message, final String fileName, final String category, final Priority priority) {
+        checkWarning(annotation, lineNumber, message, fileName, category, priority);
+
+        checkColumn(annotation, column);
+    }
+
+    private void checkColumn(final FileAnnotation annotation, final int column) {
+        assertEquals("Wrong column detected: ", column, annotation.getColumnStart());
     }
 
     /**
@@ -68,6 +96,32 @@ protected void checkWarning(final FileAnnotation annotation, final int lineNumbe
         checkWarning(annotation, lineNumber, message, fileName, category, priority);
     }
 
+    /**
+     * Checks the properties of the specified warning.
+     *
+     * @param annotation
+     *            the warning to check
+     * @param lineNumber
+     *            the expected line number
+     * @param column
+     *            the expected column
+     * @param message
+     *            the expected message
+     * @param fileName
+     *            the expected filename
+     * @param type
+     *            the expected type
+     * @param category
+     *            the expected category
+     * @param priority
+     *            the expected priority
+     */
+    protected void checkWarning(final FileAnnotation annotation, final int lineNumber, final int column, final String message, final String fileName, final String type, final String category, final Priority priority) { // NOCHECKSTYLE
+        checkWarning(annotation, lineNumber, message, fileName, type, category, priority);
+
+        checkColumn(annotation, column);
+    }
+
     private Warning castWarning(final FileAnnotation annotation) {
         assertTrue("Annotation is of wrong type.", annotation instanceof Warning);
         return (Warning)annotation;