Issue35 (#39)

Fix #35 with batching of files issued to TsLint

.gitignore

@@ -47,4 +47,5 @@ Desktop.ini
 hs_err_pid*
 
 #Custom stuff
-sample.tslint.json
+sample.tslint.json
+.idea

pom.xml

@@ -5,7 +5,7 @@
   <groupId>com.pablissimo.sonar</groupId>
   <artifactId>sonar-typescript-plugin</artifactId>
   <packaging>sonar-plugin</packaging>
-  <version>0.4-SNAPSHOT</version>
+  <version>0.6-SNAPSHOT</version>
 
   <name>TypeScript</name>
   <description>Analyse TypeScript projects</description>
@@ -14,7 +14,7 @@
     <connection>scm:git:[email protected]:Pablissimo/SonarTsPlugin.git</connection>
     <developerConnection>scm:git:[email protected]:Pablissimo/SonarTsPlugin.git</developerConnection>
     <url>https://github.com/Pablissimo/SonarTsPlugin</url>
-    <tag>0.4</tag>
+    <tag>0.6</tag>
   </scm>
 
   <properties>

src/main/java/com/pablissimo/sonar/TsLintExecutor.java

@@ -1,5 +1,7 @@
 package com.pablissimo.sonar;
 
+import java.util.List;
+
 public interface TsLintExecutor {
-    String execute(String pathToTsLint, String configFile, String rulesDir, String file, Integer timeoutMs);
+    String execute(String pathToTsLint, String configFile, String rulesDir, List<String> files, Integer timeoutMs);
 }

src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java

@@ -1,41 +1,78 @@
 package com.pablissimo.sonar;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.utils.command.Command;
 import org.sonar.api.utils.command.CommandExecutor;
 import org.sonar.api.utils.command.StreamConsumer;
 
 public class TsLintExecutorImpl implements TsLintExecutor {
+    public static final int MAX_COMMAND_LENGTH = 4096;
     private static final Logger LOG = LoggerFactory.getLogger(TsLintExecutorImpl.class);
 
     private StringBuilder stdOut;
     private StringBuilder stdErr;
 
-    public String execute(String pathToTsLint, String configFile, String rulesDir, String file, Integer timeoutMs) {
-        LOG.info("TsLint executing for " + file);
-        Command command = Command.create("node");
-        command.addArgument(pathToTsLint);
-        command.addArgument("--format");
-        command.addArgument("json");
-
+    private static Command getBaseCommand(String pathToTsLint, String configFile, String rulesDir) {
+        Command command =
+                Command
+                .create("node")
+                .addArgument(pathToTsLint)
+                .addArgument("--format")
+                .addArgument("json");
+
         if (rulesDir != null && rulesDir.length() > 0) {
-            command.addArgument("--rules-dir");
-            command.addArgument(rulesDir);
+            command
+                .addArgument("--rules-dir")
+                .addArgument(rulesDir);
+        }
+
+        command
+            .addArgument("--config")
+            .addArgument(configFile)
+            .setNewShell(false);
+
+        return command;
+    }
+
+    public String execute(String pathToTsLint, String configFile, String rulesDir, List<String> files, Integer timeoutMs) {
+        // New up a command that's everything we need except the files to process
+        // We'll use this as our reference for chunking up files
+        int baseCommandLength = getBaseCommand(pathToTsLint, configFile, rulesDir).toCommandLine().length();
+        int availableForBatching = MAX_COMMAND_LENGTH - baseCommandLength;
+
+        List<List<String>> batches = new ArrayList<List<String>>();
+        List<String> currentBatch = new ArrayList<String>();
+        batches.add(currentBatch);
+
+        int currentBatchLength = 0;
+        for (int i = 0; i < files.size(); i++) {
+            String nextPath = files.get(i).trim();
+
+            // +1 for the space we'll be adding between filenames
+            if (currentBatchLength + nextPath.length() + 1 > availableForBatching) {
+                // Too long to add to this batch, create new
+                currentBatch = new ArrayList<String>();
+                currentBatchLength = 0;
+                batches.add(currentBatch);
+            }
+
+            currentBatch.add(nextPath);
+            currentBatchLength += nextPath.length() + 1;
         }
-
-        command.addArgument("--config");
-        command.addArgument(configFile);
-        command.addArgument(file.trim());
-        command.setNewShell(false);
+
+        LOG.debug("Split " + files.size() + " files into " + batches.size() + " batches for processing");
 
         this.stdOut = new StringBuilder();
         this.stdErr = new StringBuilder();
 
         StreamConsumer stdOutConsumer = new StreamConsumer() {
             public void consumeLine(String line) {
                 LOG.trace("TsLint Out: " + line);
-                stdOut.append(line + "\n");
+                stdOut.append(line);
             }
         };
 
@@ -46,11 +83,28 @@ public void consumeLine(String line) {
             }
         };
 
-        this.createExecutor().execute(command, stdOutConsumer, stdErrConsumer, timeoutMs);
+        for (int i = 0; i < batches.size(); i++) {
+            List<String> thisBatch = batches.get(i);
+            Command thisCommand = getBaseCommand(pathToTsLint, configFile, rulesDir);
+
+            for (int fileIndex = 0; fileIndex < thisBatch.size(); fileIndex++) {
+                thisCommand.addArgument(thisBatch.get(fileIndex));
+            }
+
+            LOG.debug("Executing TsLint with command: " + thisCommand.toCommandLine());
+
+            // Timeout is specified per file, not per batch (which can vary a lot)
+            // so multiply it up
+            this.createExecutor().execute(thisCommand, stdOutConsumer, stdErrConsumer, timeoutMs * thisBatch.size());
+        }
 
-        return stdOut.toString();
+        String rawOutput = stdOut.toString();
+
+        // TsLint returns nonsense for its JSON output when faced with multiple files
+        // so we need to fix it up before we do anything else
+        return "[" + rawOutput.replaceAll("\\]\\[", "],[") + "]";
     }
-    
+
     protected CommandExecutor createExecutor() {
         return CommandExecutor.create();
     }

src/main/java/com/pablissimo/sonar/TsLintParser.java

@@ -3,5 +3,5 @@
 import com.pablissimo.sonar.model.TsLintIssue;
 
 public interface TsLintParser {
-    TsLintIssue[] parse(String toParse);
+    TsLintIssue[][] parse(String toParse);
 }

src/main/java/com/pablissimo/sonar/TsLintParserImpl.java

@@ -5,10 +5,10 @@
 import com.pablissimo.sonar.model.TsLintIssue;
 
 public class TsLintParserImpl implements TsLintParser {
-    public TsLintIssue[] parse(String toParse) {
+    public TsLintIssue[][] parse(String toParse) {
         GsonBuilder builder = new GsonBuilder();
         Gson gson = builder.create();
 
-        return gson.fromJson(toParse, TsLintIssue[].class);
+        return gson.fromJson(toParse, TsLintIssue[][].class);
     }
 }

src/main/java/com/pablissimo/sonar/TsLintSensor.java

@@ -5,6 +5,7 @@
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 
@@ -88,37 +89,62 @@ else if (pathToTsLintConfig == null) {
             ruleNames.add(rule.getKey());
         }
 
+        List<String> paths = new ArrayList<String>();
+        HashMap<String, File> fileMap = new HashMap<String, File>();
+
         for (File file : fileSystem.files(this.filePredicates.hasLanguage(TypeScriptLanguage.LANGUAGE_KEY))) {
             if (skipTypeDefFiles && file.getName().toLowerCase().endsWith("." + TypeScriptLanguage.LANGUAGE_DEFINITION_EXTENSION)) {
                 continue;
             }
-
+
+            String pathAdjusted = file.getAbsolutePath().replace('\\', '/');
+            paths.add(pathAdjusted);
+            fileMap.put(pathAdjusted, file);
+        }
+
+        String jsonResult = executor.execute(pathToTsLint, pathToTsLintConfig, rulesDir, paths, tsLintTimeoutMs);
+
+        TsLintIssue[][] issues = parser.parse(jsonResult);
+
+        if (issues == null) {
+            LOG.warn("TsLint returned no result at all");
+            return;
+        }
+
+        // Each issue bucket will contain info about a single file
+        for (TsLintIssue[] batchIssues : issues) {
+            if (batchIssues == null || batchIssues.length == 0) {
+                continue;
+            }
+
+            String filePath = batchIssues[0].getName();
+
+            if (!fileMap.containsKey(filePath)) {
+                LOG.warn("TsLint reported issues against a file that wasn't sent to it - will be ignored: " + filePath);
+                continue;
+            }
+
+            File file = fileMap.get(filePath);
             Resource resource = this.getFileFromIOFile(file, project);
             Issuable issuable = perspectives.as(Issuable.class, resource);
-
-            String jsonResult = executor.execute(pathToTsLint, pathToTsLintConfig, rulesDir, file.getAbsolutePath(), tsLintTimeoutMs);
-
-            TsLintIssue[] issues = parser.parse(jsonResult);
-
-            if (issues != null) {
-                for (TsLintIssue issue : issues) {
-                    // Make sure the rule we're violating is one we recognise - if not, we'll
-                    // fall back to the generic 'tslint-issue' rule
-                    String ruleName = issue.getRuleName();
-                    if (!ruleNames.contains(ruleName)) {
-                        ruleName = TsRulesDefinition.RULE_TSLINT_ISSUE;
-                    }
-
-                    issuable.addIssue
-                    (
-                            issuable
-                            .newIssueBuilder()
-                            .line(issue.getStartPosition().getLine() + 1)
-                            .message(issue.getFailure())
-                            .ruleKey(RuleKey.of(TsRulesDefinition.REPOSITORY_NAME, ruleName))
-                            .build()
-                            );
+
+            for (TsLintIssue issue : batchIssues) {
+                // Make sure the rule we're violating is one we recognise - if not, we'll
+                // fall back to the generic 'tslint-issue' rule
+                String ruleName = issue.getRuleName();
+                if (!ruleNames.contains(ruleName)) {
+                    ruleName = TsRulesDefinition.RULE_TSLINT_ISSUE;
                 }
+
+                issuable.addIssue
+                (
+                        issuable
+                        .newIssueBuilder()
+                        .line(issue.getStartPosition().getLine() + 1)
+                        .message(issue.getFailure())
+                        .ruleKey(RuleKey.of(TsRulesDefinition.REPOSITORY_NAME, ruleName))
+                        .build()
+                        );
             }
         }
     }