Merge pull request #145 from rafalnowak/pylint

Processor for Pylint

README.md

@@ -163,6 +163,7 @@ gradle run -Dexec.args="--conf example.properties --changeId 1234 --revisionId 4
 - Henning Hoefer
 - Dominik Przybysz
 - Damian Szczepanik
+- Rafał Nowak
 
 ## License
 

build.gradle

@@ -122,6 +122,7 @@ dependencies {
 
     // Test dependencies
     testCompile 'junit:junit:4.11'
+    testCompile 'pl.pragmatists:JUnitParams:1.0.5'
     testCompile 'org.mockito:mockito-core:1.9.5'
     testCompile 'org.assertj:assertj-core:1.5.0'
     testCompile 'com.googlecode.catch-exception:catch-exception:1.2.0'

src/main/java/pl/touk/sputnik/configuration/GeneralOption.java

@@ -63,6 +63,9 @@ public enum GeneralOption implements ConfigurationOption {
     TSLINT_CONFIGURATION_FILE("tslint.configurationFile", "TSLint configuration file", "tslint.json"),
     TSLINT_SCRIPT("tslint.script", "TSLint script for validating files", "/usr/bin/tslint"),
 
+    PYLINT_ENABLED("pylint.enabled", "Pylint enabled", "false"),
+    PYLINT_RCFILE("pylint.rcfile", "Pylint rcfile", null),
+
     SONAR_ENABLED("sonar.enabled", "Sonar enabled", "false"),
     SONAR_PROPERTIES("sonar.configurationFiles", "Sonar base configuration", "sonar-project.properties"),
     SONAR_VERBOSE("sonar.verbose", "Run sonar in verbose mode", "false"),

src/main/java/pl/touk/sputnik/processor/pylint/PylintException.java

@@ -0,0 +1,11 @@
+package pl.touk.sputnik.processor.pylint;
+
+public class PylintException extends RuntimeException {
+    public PylintException(String message) {
+        super(message);
+    }
+
+    public PylintException(String message, Throwable t) {
+        super(message, t);
+    }
+}

src/main/java/pl/touk/sputnik/processor/pylint/PylintExecutor.java

@@ -0,0 +1,47 @@
+package pl.touk.sputnik.processor.pylint;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import lombok.extern.slf4j.Slf4j;
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.exec.ExternalProcess;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+@Slf4j
+public class PylintExecutor {
+    private static final String PYLINT_EXECUTABLE = "pylint";
+    private static final String PYLINT_OUTPUT_FORMAT = "--output-format=json";
+    private static final String PYLINT_RCFILE_NAME = "--rcfile=";
+
+    private String rcfileName;
+
+    public PylintExecutor(@Nullable String rcfileName) {
+        this.rcfileName = rcfileName;
+    }
+
+    public String runOnFile(String filePath) {
+        log.info("Review on file: " + filePath);
+        return new ExternalProcess().executeCommand(buildParams(filePath));
+    }
+
+    @NotNull
+    private String[] buildParams(String filePath) {
+        List<String> basicPylintArgs = ImmutableList.of(
+                PYLINT_EXECUTABLE,
+                PYLINT_OUTPUT_FORMAT);
+        List<String> rcfileNameArg = getRcfileNameAsList();
+        List<String> filePathArg = ImmutableList.of(filePath);
+        List<String> allArgs = Lists.newArrayList(Iterables.concat(basicPylintArgs, rcfileNameArg, filePathArg));
+        return allArgs.toArray(new String[allArgs.size()]);
+    }
+
+    private List<String> getRcfileNameAsList() {
+        if (rcfileName == null) {
+            return ImmutableList.of();
+        }
+        return ImmutableList.of(PYLINT_RCFILE_NAME + rcfileName);
+    }
+}

src/main/java/pl/touk/sputnik/processor/pylint/PylintProcessor.java

@@ -0,0 +1,45 @@
+package pl.touk.sputnik.processor.pylint;
+
+import lombok.extern.slf4j.Slf4j;
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.configuration.Configuration;
+import pl.touk.sputnik.configuration.GeneralOption;
+import pl.touk.sputnik.processor.tools.externalprocess.ExternalProcessResultParser;
+import pl.touk.sputnik.processor.tools.externalprocess.ProcessorRunningExternalProcess;
+import pl.touk.sputnik.review.filter.FileFilter;
+import pl.touk.sputnik.review.filter.PythonFilter;
+
+import java.io.File;
+
+@Slf4j
+public class PylintProcessor extends ProcessorRunningExternalProcess {
+
+    private PylintExecutor pylintExecutor;
+    private PylintResultParser pylintResultParser;
+
+    public PylintProcessor(Configuration configuration) {
+        pylintExecutor = new PylintExecutor(configuration.getProperty(GeneralOption.PYLINT_RCFILE));
+        pylintResultParser = new PylintResultParser();
+    }
+
+    @NotNull
+    @Override
+    public String getName() {
+        return "Pylint";
+    }
+
+    @Override
+    public FileFilter getReviewFileFilter() {
+        return new PythonFilter();
+    }
+
+    @Override
+    public ExternalProcessResultParser getParser() {
+        return pylintResultParser;
+    }
+
+    @Override
+    public String getOutputFromExternalProcess(File fileToReview) {
+        return pylintExecutor.runOnFile(fileToReview.getAbsolutePath());
+    }
+}

src/main/java/pl/touk/sputnik/processor/pylint/PylintProcessorFactory.java

@@ -0,0 +1,17 @@
+package pl.touk.sputnik.processor.pylint;
+
+import pl.touk.sputnik.configuration.Configuration;
+import pl.touk.sputnik.configuration.GeneralOption;
+import pl.touk.sputnik.processor.ReviewProcessorFactory;
+
+public class PylintProcessorFactory implements ReviewProcessorFactory<PylintProcessor> {
+    @Override
+    public boolean isEnabled(Configuration configuration) {
+        return Boolean.valueOf(configuration.getProperty(GeneralOption.PYLINT_ENABLED));
+    }
+
+    @Override
+    public PylintProcessor create(Configuration configuration) {
+        return new PylintProcessor(configuration);
+    }
+}

src/main/java/pl/touk/sputnik/processor/pylint/PylintResultParser.java

@@ -0,0 +1,68 @@
+package pl.touk.sputnik.processor.pylint;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.lang3.StringUtils;
+import pl.touk.sputnik.processor.tools.externalprocess.ExternalProcessResultParser;
+import pl.touk.sputnik.processor.pylint.json.PylintMessage;
+import pl.touk.sputnik.processor.pylint.json.PylintMessages;
+import pl.touk.sputnik.review.Severity;
+import pl.touk.sputnik.review.Violation;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class PylintResultParser implements ExternalProcessResultParser {
+
+    private final ObjectMapper objectMapper = new ObjectMapper();
+
+    private static final String PYLINT_ERROR = "error";
+    private static final String PYLINT_FATAL = "fatal";
+    private static final String PYLINT_WARNING = "warning";
+    private static final String PYLINT_CONVENTION = "convention";
+    private static final String PYLINT_REFACTOR = "refactor";
+
+    @Override
+    public List<Violation> parse(String pylintOutput) {
+        if (StringUtils.isEmpty(pylintOutput)) {
+            return Collections.emptyList();
+        }
+        try {
+            List<Violation> violations = new ArrayList<>();
+            PylintMessages messages = objectMapper.readValue(removeHeaderFromPylintOutput(pylintOutput), PylintMessages.class);
+            for (PylintMessage message : messages) {
+                Violation violation = new Violation(message.getPath(),
+                        message.getLine(),
+                        formatViolationMessageFromPylint(message),
+                        pylintMessageTypeToSeverity(message.getMessage(), message.getType()));
+                violations.add(violation);
+            }
+            return violations;
+        } catch (IOException e) {
+            throw new PylintException("Error when converting from json format", e);
+        }
+    }
+
+    private String removeHeaderFromPylintOutput(String pylintOutput) {
+        return pylintOutput.replace("No config file found, using default configuration", "");
+    }
+
+    private String formatViolationMessageFromPylint(PylintMessage message) {
+        return message.getMessage() + " [" + message.getSymbol() + "]";
+    }
+
+    private Severity pylintMessageTypeToSeverity(String message, String messageType) {
+        if (PYLINT_ERROR.equals(messageType)) {
+            return Severity.ERROR;
+        } else if (PYLINT_WARNING.equals(messageType)) {
+            return Severity.WARNING;
+        } else if (PYLINT_CONVENTION.equals(messageType) || PYLINT_REFACTOR.equals(messageType)) {
+            return Severity.INFO;
+        } else if (PYLINT_FATAL.equals(messageType)) {
+            throw new PylintException("Fatal error from pylint (" + message + ")");
+        } else {
+            throw new PylintException("Unknown message type returned by pylint (type = " + messageType + ")");
+        }
+    }
+}

src/main/java/pl/touk/sputnik/processor/pylint/json/PylintMessage.java

@@ -0,0 +1,17 @@
+package pl.touk.sputnik.processor.pylint.json;
+
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+@Data
+@NoArgsConstructor
+public class PylintMessage {
+    private String message;
+    private String obj;
+    private int column;
+    private String path;
+    private int line;
+    private String type;
+    private String symbol;
+    private String module;
+}

src/main/java/pl/touk/sputnik/processor/pylint/json/PylintMessages.java

@@ -0,0 +1,6 @@
+package pl.touk.sputnik.processor.pylint.json;
+
+import java.util.ArrayList;
+
+public class PylintMessages extends ArrayList<PylintMessage> {
+}

src/main/java/pl/touk/sputnik/processor/tools/externalprocess/ExternalProcessResultParser.java

@@ -0,0 +1,9 @@
+package pl.touk.sputnik.processor.tools.externalprocess;
+
+import pl.touk.sputnik.review.Violation;
+
+import java.util.List;
+
+public interface ExternalProcessResultParser {
+    List<Violation> parse(String output);
+}

src/main/java/pl/touk/sputnik/processor/tools/externalprocess/ProcessorRunningExternalProcess.java

@@ -0,0 +1,34 @@
+package pl.touk.sputnik.processor.tools.externalprocess;
+
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.review.Review;
+import pl.touk.sputnik.review.ReviewProcessor;
+import pl.touk.sputnik.review.ReviewResult;
+import pl.touk.sputnik.review.Violation;
+import pl.touk.sputnik.review.filter.FileFilter;
+import pl.touk.sputnik.review.transformer.IOFileTransformer;
+
+import java.io.File;
+import java.util.List;
+
+public abstract class ProcessorRunningExternalProcess implements ReviewProcessor {
+
+    @Override
+    @NotNull
+    public ReviewResult process(@NotNull Review review) {
+        ReviewResult result = new ReviewResult();
+        List<File> files = review.getFiles(getReviewFileFilter(), new IOFileTransformer());
+        for (File file : files) {
+            for (Violation violation : getParser().parse(getOutputFromExternalProcess(file))) {
+                result.add(violation);
+            }
+        }
+        return result;
+    }
+
+    public abstract FileFilter getReviewFileFilter();
+
+    public abstract ExternalProcessResultParser getParser();
+
+    public abstract String getOutputFromExternalProcess(File fileToReview);
+}

src/main/java/pl/touk/sputnik/processor/tslint/TSLintException.java

@@ -0,0 +1,11 @@
+package pl.touk.sputnik.processor.tslint;
+
+public class TSLintException extends RuntimeException {
+    public TSLintException(String message) {
+        super(message);
+    }
+
+    public TSLintException(String message, Throwable t) {
+        super(message, t);
+    }
+}

src/main/java/pl/touk/sputnik/processor/tslint/TSLintProcessor.java

@@ -4,18 +4,15 @@
 import org.jetbrains.annotations.NotNull;
 import pl.touk.sputnik.configuration.Configuration;
 import pl.touk.sputnik.configuration.GeneralOption;
-import pl.touk.sputnik.review.Review;
-import pl.touk.sputnik.review.ReviewProcessor;
-import pl.touk.sputnik.review.ReviewResult;
-import pl.touk.sputnik.review.Violation;
+import pl.touk.sputnik.processor.tools.externalprocess.ExternalProcessResultParser;
+import pl.touk.sputnik.processor.tools.externalprocess.ProcessorRunningExternalProcess;
+import pl.touk.sputnik.review.filter.FileFilter;
 import pl.touk.sputnik.review.filter.TypeScriptFilter;
-import pl.touk.sputnik.review.transformer.IOFileTransformer;
 
 import java.io.File;
-import java.util.List;
 
 @Slf4j
-public class TSLintProcessor implements ReviewProcessor {
+public class TSLintProcessor extends ProcessorRunningExternalProcess {
 
     private static final String SOURCE_NAME = "TSLint";
 
@@ -31,23 +28,24 @@ public TSLintProcessor(Configuration config) {
         resultParser = new TSLintResultParser();
     }
 
+    @NotNull
     @Override
     public String getName() {
         return SOURCE_NAME;
     }
 
     @Override
-    @NotNull
-    public ReviewResult process(Review review) {
-        ReviewResult result = new ReviewResult();
-
-        List<File> files = review.getFiles(new TypeScriptFilter(), new IOFileTransformer());
-        for (File file : files) {
-            for (Violation violation : resultParser.parse(tsLintScript.reviewFile(file.getAbsolutePath()))) {
-                result.add(violation);
-            }
-        }
-        return result;
+    public FileFilter getReviewFileFilter() {
+        return new TypeScriptFilter();
     }
 
+    @Override
+    public ExternalProcessResultParser getParser() {
+        return resultParser;
+    }
+
+    @Override
+    public String getOutputFromExternalProcess(File fileToReview) {
+        return tsLintScript.reviewFile(fileToReview.getAbsolutePath());
+    }
 }

src/main/java/pl/touk/sputnik/processor/tslint/TSLintResultParser.java

@@ -3,7 +3,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
-import pl.touk.sputnik.connector.gerrit.GerritException;
+import pl.touk.sputnik.processor.tools.externalprocess.ExternalProcessResultParser;
 import pl.touk.sputnik.processor.tslint.json.ListViolationsResponse;
 import pl.touk.sputnik.processor.tslint.json.TSLintFileInfo;
 import pl.touk.sputnik.review.Severity;
@@ -15,10 +15,11 @@
 import java.util.List;
 
 @Slf4j
-public class TSLintResultParser {
+public class TSLintResultParser implements ExternalProcessResultParser {
 
     private final ObjectMapper objectMapper = new ObjectMapper();
 
+    @Override
     public List<Violation> parse(String jsonViolations) {
         if (StringUtils.isEmpty(jsonViolations)) {
             return Collections.emptyList();
@@ -35,7 +36,7 @@ public List<Violation> parse(String jsonViolations) {
             }
             return result;
         } catch (IOException e) {
-            throw new GerritException("Error when converting from json format", e);
+            throw new TSLintException("Error when converting from json format", e);
         }
     }
 }

src/main/java/pl/touk/sputnik/processor/tslint/TSLintScript.java

@@ -41,7 +41,7 @@ public TSLintScript(String tsScript, String configFile) {
     public void validateConfiguration() throws ReviewException {
         // check if config file exist
         if (!new File(configFile).exists()) {
-            throw new ReviewException("Could not find tslint configuration file: " + configFile);
+            throw new TSLintException("Could not find tslint configuration file: " + configFile);
         }
     }