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

Commit

Permalink
Fix issue #69 - long output from TsLint no longer truncated on Linux
Browse files Browse the repository at this point in the history
  • Loading branch information
Pablissimo committed Nov 19, 2016
1 parent 1c2ac3a commit ebe1754
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 34 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<groupId>com.pablissimo.sonar</groupId>
<artifactId>sonar-typescript-plugin</artifactId>
<packaging>sonar-plugin</packaging>
<version>0.94-SNAPSHOT</version>
<version>0.95-SNAPSHOT</version>

<name>TypeScript</name>
<description>Analyse TypeScript projects</description>
Expand Down
80 changes: 54 additions & 26 deletions src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java
Original file line number Diff line number Diff line change
@@ -1,26 +1,31 @@
package com.pablissimo.sonar;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.List;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.utils.System2;
import org.sonar.api.utils.TempFolder;
import org.sonar.api.utils.command.Command;
import org.sonar.api.utils.command.CommandExecutor;
import org.sonar.api.utils.command.StreamConsumer;
import org.sonar.api.utils.command.StringStreamConsumer;

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;

private boolean mustQuoteSpaceContainingPaths = false;
private TempFolder tempFolder;

public TsLintExecutorImpl(System2 system) {
public TsLintExecutorImpl(System2 system, TempFolder tempFolder) {
this.mustQuoteSpaceContainingPaths = system.isOsWindows();
this.tempFolder = tempFolder;
}

public String preparePath(String path) {
Expand All @@ -35,7 +40,7 @@ else if (path.contains(" ") && this.mustQuoteSpaceContainingPaths) {
}
}

private Command getBaseCommand(String pathToTsLint, String configFile, String rulesDir) {
private Command getBaseCommand(String pathToTsLint, String configFile, String rulesDir, String tempPath) {
Command command =
Command
.create("node")
Expand All @@ -48,6 +53,12 @@ private Command getBaseCommand(String pathToTsLint, String configFile, String ru
.addArgument("--rules-dir")
.addArgument(this.preparePath(rulesDir));
}

if (tempPath != null && tempPath.length() > 0) {
command
.addArgument("--out")
.addArgument(this.preparePath(tempPath));
}

command
.addArgument("--config")
Expand All @@ -60,7 +71,12 @@ private Command getBaseCommand(String pathToTsLint, String configFile, String ru
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();
File tslintOutputFile = this.tempFolder.newFile();
String tslintOutputFilePath = tslintOutputFile.getAbsolutePath();

LOG.debug("Using a temporary path for TsLint output: " + tslintOutputFilePath);

int baseCommandLength = getBaseCommand(pathToTsLint, configFile, rulesDir, tslintOutputFilePath).toCommandLine().length();
int availableForBatching = MAX_COMMAND_LENGTH - baseCommandLength;

List<List<String>> batches = new ArrayList<List<String>>();
Expand All @@ -84,26 +100,16 @@ public String execute(String pathToTsLint, String configFile, String rulesDir, L
}

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) {
stdOut.append(line);
}
};

StreamConsumer stdErrConsumer = new StreamConsumer() {
public void consumeLine(String line) {
LOG.error("TsLint Err: " + line);
stdErr.append(line + "\n");
}
};


StringStreamConsumer stdOutConsumer = new StringStreamConsumer();
StringStreamConsumer stdErrConsumer = new StringStreamConsumer();

StringBuilder outputBuilder = new StringBuilder();

for (int i = 0; i < batches.size(); i++) {
List<String> thisBatch = batches.get(i);
Command thisCommand = getBaseCommand(pathToTsLint, configFile, rulesDir);

Command thisCommand = getBaseCommand(pathToTsLint, configFile, rulesDir, tslintOutputFilePath);

for (int fileIndex = 0; fileIndex < thisBatch.size(); fileIndex++) {
thisCommand.addArgument(thisBatch.get(fileIndex));
Expand All @@ -114,14 +120,36 @@ public void consumeLine(String line) {
// 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());

try {
BufferedReader reader = this.getBufferedReaderForFile(tslintOutputFile);

String str;
while ((str = reader.readLine()) != null) {
outputBuilder.append(str);
}

reader.close();
}
catch (IOException ex) {
LOG.error("Failed to re-read TsLint output from " + tslintOutputFilePath, ex);
}
}

String rawOutput = stdOut.toString();
String rawOutput = outputBuilder.toString();

LOG.debug("Got " + rawOutput.length() + " chars of TsLint output");

// 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 BufferedReader getBufferedReaderForFile(File file) throws IOException {
return new BufferedReader(
new InputStreamReader(
new FileInputStream(file), "UTF8"));
}

protected CommandExecutor createExecutor() {
return CommandExecutor.create();
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/com/pablissimo/sonar/TsLintSensor.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.sonar.api.config.Settings;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.utils.System2;
import org.sonar.api.utils.TempFolder;

import java.util.*;

Expand All @@ -24,18 +25,20 @@ public class TsLintSensor implements Sensor {

private Settings settings;
private System2 system;

public TsLintSensor(Settings settings, System2 system) {
private TempFolder tempFolder;

public TsLintSensor(Settings settings, System2 system, TempFolder tempFolder) {
this.settings = settings;
this.system = system;
this.tempFolder = tempFolder;
}

protected PathResolver getPathResolver() {
return new PathResolverImpl();
}

protected TsLintExecutor getTsLintExecutor() {
return new TsLintExecutorImpl(this.system);
return new TsLintExecutorImpl(this.system, this.tempFolder);
}

protected TsLintParser getTsLintParser() {
Expand Down
18 changes: 15 additions & 3 deletions src/test/java/com/pablissimo/sonar/TsLintExecutorImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

import java.io.BufferedReader;
import java.io.File;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -11,6 +13,7 @@
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.sonar.api.utils.System2;
import org.sonar.api.utils.TempFolder;
import org.sonar.api.utils.command.Command;
import org.sonar.api.utils.command.CommandExecutor;
import org.sonar.api.utils.command.StreamConsumer;
Expand All @@ -20,16 +23,25 @@
public class TsLintExecutorImplTest {
TsLintExecutorImpl executorImpl;
CommandExecutor commandExecutor;
TempFolder tempFolder;
File tempOutputFile;

System2 system;

@Before
public void setUp() throws Exception {
this.system = mock(System2.class);
this.tempFolder = mock(TempFolder.class);

this.tempOutputFile = mock(File.class);
when(this.tempOutputFile.getAbsolutePath()).thenReturn("path/to/temp");
when(this.tempFolder.newFile()).thenReturn(this.tempOutputFile);

this.commandExecutor = mock(CommandExecutor.class);
this.executorImpl = spy(new TsLintExecutorImpl(this.system));

this.executorImpl = spy(new TsLintExecutorImpl(this.system, this.tempFolder));
when(this.executorImpl.createExecutor()).thenReturn(this.commandExecutor);
doReturn(mock(BufferedReader.class)).when(this.executorImpl).getBufferedReaderForFile(any(File.class));
}

@Test
Expand All @@ -54,7 +66,7 @@ public Integer answer(InvocationOnMock invocation) throws Throwable {
Command theCommand = capturedCommands.get(0);
long theTimeout = capturedTimeouts.get(0);

assertEquals("node path/to/tslint --format json --rules-dir path/to/rules --config path/to/config path/to/file path/to/another", theCommand.toCommandLine());
assertEquals("node path/to/tslint --format json --rules-dir path/to/rules --out path/to/temp --config path/to/config path/to/file path/to/another", theCommand.toCommandLine());
// Expect one timeout period per file processed
assertEquals(2 * 40000, theTimeout);
}
Expand Down Expand Up @@ -105,7 +117,7 @@ public Integer answer(InvocationOnMock invocation) throws Throwable {
public void BatchesExecutions_IfTooManyFilesForCommandLine() {
List<String> filenames = new ArrayList<String>();
int currentLength = 0;
int standardCmdLength = "node path/to/tslint --format json --rules-dir path/to/rules --config path/to/config".length();
int standardCmdLength = "node path/to/tslint --format json --rules-dir path/to/rules --out path/to/temp --config path/to/config".length();

String firstBatch = "first batch";
while (currentLength + 12 < TsLintExecutorImpl.MAX_COMMAND_LENGTH - standardCmdLength) {
Expand Down
5 changes: 4 additions & 1 deletion src/test/java/com/pablissimo/sonar/TsLintSensorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import org.sonar.api.rule.RuleKey;
import org.sonar.api.utils.System2;
import org.sonar.api.utils.TempFolder;

public class TsLintSensorTest {
Settings settings;
Expand All @@ -39,6 +40,7 @@ public class TsLintSensorTest {
HashMap<String, String> fakePathResolutions;

System2 system;
TempFolder tempFolder;

@Before
public void setUp() throws Exception {
Expand All @@ -49,12 +51,13 @@ public void setUp() throws Exception {

this.settings = mock(Settings.class);
this.system = mock(System2.class);
this.tempFolder = mock(TempFolder.class);

when(this.settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)).thenReturn(45000);
this.executor = mock(TsLintExecutor.class);
this.parser = mock(TsLintParser.class);
this.resolver = mock(PathResolver.class);
this.sensor = spy(new TsLintSensor(settings, this.system));
this.sensor = spy(new TsLintSensor(settings, this.system, this.tempFolder));

this.file = new DefaultInputFile("", "path/to/file")
.setLanguage(TypeScriptLanguage.LANGUAGE_KEY)
Expand Down

0 comments on commit ebe1754

Please sign in to comment.