Skip to content
This repository was archived by the owner on Jul 8, 2019. It is now read-only.

Commit dc44ea1

Browse files
PablissimoPablissimo
authored
Issue35 (#39)
Fix #35 with batching of files issued to TsLint
1 parent 5e9b15a commit dc44ea1

File tree

10 files changed

+232
-60
lines changed

10 files changed

+232
-60
lines changed

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,5 @@ Desktop.ini
4747
hs_err_pid*
4848

4949
#Custom stuff
50-
sample.tslint.json
50+
sample.tslint.json
51+
.idea

‎pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<groupId>com.pablissimo.sonar</groupId>
66
<artifactId>sonar-typescript-plugin</artifactId>
77
<packaging>sonar-plugin</packaging>
8-
<version>0.4-SNAPSHOT</version>
8+
<version>0.6-SNAPSHOT</version>
99

1010
<name>TypeScript</name>
1111
<description>Analyse TypeScript projects</description>
@@ -14,7 +14,7 @@
1414
<connection>scm:git:git@github.com:Pablissimo/SonarTsPlugin.git</connection>
1515
<developerConnection>scm:git:git@github.com:Pablissimo/SonarTsPlugin.git</developerConnection>
1616
<url>https://github.com/Pablissimo/SonarTsPlugin</url>
17-
<tag>0.4</tag>
17+
<tag>0.6</tag>
1818
</scm>
1919

2020
<properties>

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.pablissimo.sonar;
22

3+
import java.util.List;
4+
35
public interface TsLintExecutor {
4-
String execute(String pathToTsLint, String configFile, String rulesDir, String file, Integer timeoutMs);
6+
String execute(String pathToTsLint, String configFile, String rulesDir, List<String> files, Integer timeoutMs);
57
}

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

Lines changed: 72 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,78 @@
11
package com.pablissimo.sonar;
22

3+
import java.util.ArrayList;
4+
import java.util.List;
5+
36
import org.slf4j.Logger;
47
import org.slf4j.LoggerFactory;
58
import org.sonar.api.utils.command.Command;
69
import org.sonar.api.utils.command.CommandExecutor;
710
import org.sonar.api.utils.command.StreamConsumer;
811

912
public class TsLintExecutorImpl implements TsLintExecutor {
13+
public static final int MAX_COMMAND_LENGTH = 4096;
1014
private static final Logger LOG = LoggerFactory.getLogger(TsLintExecutorImpl.class);
1115

1216
private StringBuilder stdOut;
1317
private StringBuilder stdErr;
1418

15-
public String execute(String pathToTsLint, String configFile, String rulesDir, String file, Integer timeoutMs) {
16-
LOG.info("TsLint executing for " + file);
17-
Command command = Command.create("node");
18-
command.addArgument(pathToTsLint);
19-
command.addArgument("--format");
20-
command.addArgument("json");
21-
19+
private static Command getBaseCommand(String pathToTsLint, String configFile, String rulesDir) {
20+
Command command =
21+
Command
22+
.create("node")
23+
.addArgument(pathToTsLint)
24+
.addArgument("--format")
25+
.addArgument("json");
26+
2227
if (rulesDir != null && rulesDir.length() > 0) {
23-
command.addArgument("--rules-dir");
24-
command.addArgument(rulesDir);
28+
command
29+
.addArgument("--rules-dir")
30+
.addArgument(rulesDir);
31+
}
32+
33+
command
34+
.addArgument("--config")
35+
.addArgument(configFile)
36+
.setNewShell(false);
37+
38+
return command;
39+
}
40+
41+
public String execute(String pathToTsLint, String configFile, String rulesDir, List<String> files, Integer timeoutMs) {
42+
// New up a command that's everything we need except the files to process
43+
// We'll use this as our reference for chunking up files
44+
int baseCommandLength = getBaseCommand(pathToTsLint, configFile, rulesDir).toCommandLine().length();
45+
int availableForBatching = MAX_COMMAND_LENGTH - baseCommandLength;
46+
47+
List<List<String>> batches = new ArrayList<List<String>>();
48+
List<String> currentBatch = new ArrayList<String>();
49+
batches.add(currentBatch);
50+
51+
int currentBatchLength = 0;
52+
for (int i = 0; i < files.size(); i++) {
53+
String nextPath = files.get(i).trim();
54+
55+
// +1 for the space we'll be adding between filenames
56+
if (currentBatchLength + nextPath.length() + 1 > availableForBatching) {
57+
// Too long to add to this batch, create new
58+
currentBatch = new ArrayList<String>();
59+
currentBatchLength = 0;
60+
batches.add(currentBatch);
61+
}
62+
63+
currentBatch.add(nextPath);
64+
currentBatchLength += nextPath.length() + 1;
2565
}
26-
27-
command.addArgument("--config");
28-
command.addArgument(configFile);
29-
command.addArgument(file.trim());
30-
command.setNewShell(false);
66+
67+
LOG.debug("Split " + files.size() + " files into " + batches.size() + " batches for processing");
3168

3269
this.stdOut = new StringBuilder();
3370
this.stdErr = new StringBuilder();
3471

3572
StreamConsumer stdOutConsumer = new StreamConsumer() {
3673
public void consumeLine(String line) {
3774
LOG.trace("TsLint Out: " + line);
38-
stdOut.append(line + "\n");
75+
stdOut.append(line);
3976
}
4077
};
4178

@@ -46,11 +83,28 @@ public void consumeLine(String line) {
4683
}
4784
};
4885

49-
this.createExecutor().execute(command, stdOutConsumer, stdErrConsumer, timeoutMs);
86+
for (int i = 0; i < batches.size(); i++) {
87+
List<String> thisBatch = batches.get(i);
88+
Command thisCommand = getBaseCommand(pathToTsLint, configFile, rulesDir);
89+
90+
for (int fileIndex = 0; fileIndex < thisBatch.size(); fileIndex++) {
91+
thisCommand.addArgument(thisBatch.get(fileIndex));
92+
}
93+
94+
LOG.debug("Executing TsLint with command: " + thisCommand.toCommandLine());
95+
96+
// Timeout is specified per file, not per batch (which can vary a lot)
97+
// so multiply it up
98+
this.createExecutor().execute(thisCommand, stdOutConsumer, stdErrConsumer, timeoutMs * thisBatch.size());
99+
}
50100

51-
return stdOut.toString();
101+
String rawOutput = stdOut.toString();
102+
103+
// TsLint returns nonsense for its JSON output when faced with multiple files
104+
// so we need to fix it up before we do anything else
105+
return "[" + rawOutput.replaceAll("\\]\\[", "],[") + "]";
52106
}
53-
107+
54108
protected CommandExecutor createExecutor() {
55109
return CommandExecutor.create();
56110
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@
33
import com.pablissimo.sonar.model.TsLintIssue;
44

55
public interface TsLintParser {
6-
TsLintIssue[] parse(String toParse);
6+
TsLintIssue[][] parse(String toParse);
77
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
import com.pablissimo.sonar.model.TsLintIssue;
66

77
public class TsLintParserImpl implements TsLintParser {
8-
public TsLintIssue[] parse(String toParse) {
8+
public TsLintIssue[][] parse(String toParse) {
99
GsonBuilder builder = new GsonBuilder();
1010
Gson gson = builder.create();
1111

12-
return gson.fromJson(toParse, TsLintIssue[].class);
12+
return gson.fromJson(toParse, TsLintIssue[][].class);
1313
}
1414
}

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

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.nio.charset.Charset;
66
import java.util.ArrayList;
77
import java.util.Collection;
8+
import java.util.HashMap;
89
import java.util.HashSet;
910
import java.util.List;
1011

@@ -88,37 +89,62 @@ else if (pathToTsLintConfig == null) {
8889
ruleNames.add(rule.getKey());
8990
}
9091

92+
List<String> paths = new ArrayList<String>();
93+
HashMap<String, File> fileMap = new HashMap<String, File>();
94+
9195
for (File file : fileSystem.files(this.filePredicates.hasLanguage(TypeScriptLanguage.LANGUAGE_KEY))) {
9296
if (skipTypeDefFiles && file.getName().toLowerCase().endsWith("." + TypeScriptLanguage.LANGUAGE_DEFINITION_EXTENSION)) {
9397
continue;
9498
}
95-
99+
100+
String pathAdjusted = file.getAbsolutePath().replace('\\', '/');
101+
paths.add(pathAdjusted);
102+
fileMap.put(pathAdjusted, file);
103+
}
104+
105+
String jsonResult = executor.execute(pathToTsLint, pathToTsLintConfig, rulesDir, paths, tsLintTimeoutMs);
106+
107+
TsLintIssue[][] issues = parser.parse(jsonResult);
108+
109+
if (issues == null) {
110+
LOG.warn("TsLint returned no result at all");
111+
return;
112+
}
113+
114+
// Each issue bucket will contain info about a single file
115+
for (TsLintIssue[] batchIssues : issues) {
116+
if (batchIssues == null || batchIssues.length == 0) {
117+
continue;
118+
}
119+
120+
String filePath = batchIssues[0].getName();
121+
122+
if (!fileMap.containsKey(filePath)) {
123+
LOG.warn("TsLint reported issues against a file that wasn't sent to it - will be ignored: " + filePath);
124+
continue;
125+
}
126+
127+
File file = fileMap.get(filePath);
96128
Resource resource = this.getFileFromIOFile(file, project);
97129
Issuable issuable = perspectives.as(Issuable.class, resource);
98-
99-
String jsonResult = executor.execute(pathToTsLint, pathToTsLintConfig, rulesDir, file.getAbsolutePath(), tsLintTimeoutMs);
100-
101-
TsLintIssue[] issues = parser.parse(jsonResult);
102-
103-
if (issues != null) {
104-
for (TsLintIssue issue : issues) {
105-
// Make sure the rule we're violating is one we recognise - if not, we'll
106-
// fall back to the generic 'tslint-issue' rule
107-
String ruleName = issue.getRuleName();
108-
if (!ruleNames.contains(ruleName)) {
109-
ruleName = TsRulesDefinition.RULE_TSLINT_ISSUE;
110-
}
111-
112-
issuable.addIssue
113-
(
114-
issuable
115-
.newIssueBuilder()
116-
.line(issue.getStartPosition().getLine() + 1)
117-
.message(issue.getFailure())
118-
.ruleKey(RuleKey.of(TsRulesDefinition.REPOSITORY_NAME, ruleName))
119-
.build()
120-
);
130+
131+
for (TsLintIssue issue : batchIssues) {
132+
// Make sure the rule we're violating is one we recognise - if not, we'll
133+
// fall back to the generic 'tslint-issue' rule
134+
String ruleName = issue.getRuleName();
135+
if (!ruleNames.contains(ruleName)) {
136+
ruleName = TsRulesDefinition.RULE_TSLINT_ISSUE;
121137
}
138+
139+
issuable.addIssue
140+
(
141+
issuable
142+
.newIssueBuilder()
143+
.line(issue.getStartPosition().getLine() + 1)
144+
.message(issue.getFailure())
145+
.ruleKey(RuleKey.of(TsRulesDefinition.REPOSITORY_NAME, ruleName))
146+
.build()
147+
);
122148
}
123149
}
124150
}

0 commit comments

Comments
 (0)