diff --git a/src/main/java/com/pablissimo/sonar/TsLintExecutorConfig.java b/src/main/java/com/pablissimo/sonar/TsLintExecutorConfig.java index 1681032..f1889de 100644 --- a/src/main/java/com/pablissimo/sonar/TsLintExecutorConfig.java +++ b/src/main/java/com/pablissimo/sonar/TsLintExecutorConfig.java @@ -1,10 +1,37 @@ package com.pablissimo.sonar; +import org.sonar.api.batch.sensor.SensorContext; +import org.sonar.api.config.Settings; + public class TsLintExecutorConfig { + public static final String CONFIG_FILENAME = "tslint.json"; + public static final String TSLINT_FALLBACK_PATH = "node_modules/tslint/bin/tslint"; + private String pathToTsLint; private String configFile; private String rulesDir; + private String pathToTsConfig; + private boolean shouldPerformTypeCheck; + private Integer timeoutMs; + + public static TsLintExecutorConfig fromSettings(Settings settings, SensorContext ctx, PathResolver resolver) { + TsLintExecutorConfig toReturn = new TsLintExecutorConfig(); + + toReturn.setPathToTsLint(resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_PATH, TSLINT_FALLBACK_PATH)); + toReturn.setConfigFile(resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH, CONFIG_FILENAME)); + toReturn.setRulesDir(resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_RULES_DIR, null)); + toReturn.setPathToTsConfig(resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_PROJECT_PATH, null)); + + toReturn.setTimeoutMs(Math.max(5000, settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT))); + toReturn.setShouldPerformTypeCheck(settings.getBoolean(TypeScriptPlugin.SETTING_TS_LINT_TYPECHECK)); + + return toReturn; + } + + public Boolean useTsConfigInsteadOfFileList() { + return this.pathToTsConfig != null && !this.pathToTsConfig.isEmpty(); + } public String getPathToTsLint() { return pathToTsLint; @@ -37,4 +64,20 @@ public Integer getTimeoutMs() { public void setTimeoutMs(Integer timeoutMs) { this.timeoutMs = timeoutMs; } + + public String getPathToTsConfig() { + return pathToTsConfig; + } + + public void setPathToTsConfig(String pathToTsConfig) { + this.pathToTsConfig = pathToTsConfig; + } + + public boolean shouldPerformTypeCheck() { + return this.shouldPerformTypeCheck; + } + + public void setShouldPerformTypeCheck(boolean performTypeCheck) { + this.shouldPerformTypeCheck = performTypeCheck; + } } diff --git a/src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java b/src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java index e9093de..ce2bacf 100644 --- a/src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java +++ b/src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java @@ -14,6 +14,7 @@ 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 { @@ -63,8 +64,20 @@ private Command getBaseCommand(TsLintExecutorConfig config, String tempPath) { command .addArgument("--config") - .addArgument(this.preparePath(config.getConfigFile())) - .setNewShell(false); + .addArgument(this.preparePath(config.getConfigFile())); + + if (config.useTsConfigInsteadOfFileList()) { + command + .addArgument("--project") + .addArgument(this.preparePath(config.getPathToTsConfig())); + } + + if (config.shouldPerformTypeCheck()) { + command + .addArgument("--type-check"); + } + + command.setNewShell(false); return command; } @@ -73,85 +86,106 @@ public List execute(TsLintExecutorConfig config, List files) { if (config == null) { throw new IllegalArgumentException("config"); } - - if (files == null) { + else if (files == null) { throw new IllegalArgumentException("files"); } // 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 + // We'll use this as our reference for chunking up files, if we need to File tslintOutputFile = this.tempFolder.newFile(); String tslintOutputFilePath = tslintOutputFile.getAbsolutePath(); + Command baseCommand = getBaseCommand(config, tslintOutputFilePath); LOG.debug("Using a temporary path for TsLint output: " + tslintOutputFilePath); - - int baseCommandLength = getBaseCommand(config, tslintOutputFilePath).toCommandLine().length(); - int availableForBatching = MAX_COMMAND_LENGTH - baseCommandLength; - List> batches = new ArrayList>(); - List currentBatch = new ArrayList(); - batches.add(currentBatch); - - int currentBatchLength = 0; - for (int i = 0; i < files.size(); i++) { - String nextPath = this.preparePath(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(); - currentBatchLength = 0; - batches.add(currentBatch); - } - - currentBatch.add(nextPath); - currentBatchLength += nextPath.length() + 1; - } - - LOG.debug("Split " + files.size() + " files into " + batches.size() + " batches for processing"); - StringStreamConsumer stdOutConsumer = new StringStreamConsumer(); StringStreamConsumer stdErrConsumer = new StringStreamConsumer(); List toReturn = new ArrayList(); - for (int i = 0; i < batches.size(); i++) { - StringBuilder outputBuilder = new StringBuilder(); - - List thisBatch = batches.get(i); - - Command thisCommand = getBaseCommand(config, tslintOutputFilePath); - - 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, config.getTimeoutMs() * thisBatch.size()); + if (config.useTsConfigInsteadOfFileList()) { + LOG.debug("Running against a single project JSON file"); - try { - BufferedReader reader = this.getBufferedReaderForFile(tslintOutputFile); - - String str; - while ((str = reader.readLine()) != null) { - outputBuilder.append(str); + // If we're being asked to use a tsconfig.json file, it'll contain + // the file list to lint - so don't batch, and just run with it + toReturn.add(this.getCommandOutput(baseCommand, stdOutConsumer, stdErrConsumer, tslintOutputFile, config.getTimeoutMs())); + } + else { + int baseCommandLength = baseCommand.toCommandLine().length(); + int availableForBatching = MAX_COMMAND_LENGTH - baseCommandLength; + + List> batches = new ArrayList>(); + List currentBatch = new ArrayList(); + batches.add(currentBatch); + + int currentBatchLength = 0; + for (int i = 0; i < files.size(); i++) { + String nextPath = this.preparePath(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(); + currentBatchLength = 0; + batches.add(currentBatch); } - - reader.close(); - - toReturn.add(outputBuilder.toString()); + + currentBatch.add(nextPath); + currentBatchLength += nextPath.length() + 1; } - catch (IOException ex) { - LOG.error("Failed to re-read TsLint output from " + tslintOutputFilePath, ex); + + LOG.debug("Split " + files.size() + " files into " + batches.size() + " batches for processing"); + + for (int i = 0; i < batches.size(); i++) { + StringBuilder outputBuilder = new StringBuilder(); + + List thisBatch = batches.get(i); + + Command thisCommand = getBaseCommand(config, tslintOutputFilePath); + + 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 + toReturn.add(this.getCommandOutput(thisCommand, stdOutConsumer, stdErrConsumer, tslintOutputFile, config.getTimeoutMs() * thisBatch.size())); } } return toReturn; } + private String getCommandOutput(Command thisCommand, StreamConsumer stdOutConsumer, StreamConsumer stdErrConsumer, File tslintOutputFile, Integer timeoutMs) { + 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); + + StringBuilder outputBuilder = new StringBuilder(); + + try { + BufferedReader reader = this.getBufferedReaderForFile(tslintOutputFile); + + String str; + while ((str = reader.readLine()) != null) { + outputBuilder.append(str); + } + + reader.close(); + + return outputBuilder.toString(); + } + catch (IOException ex) { + LOG.error("Failed to re-read TsLint output", ex); + } + + return ""; + } + protected BufferedReader getBufferedReaderForFile(File file) throws IOException { return new BufferedReader( new InputStreamReader( diff --git a/src/main/java/com/pablissimo/sonar/TsLintSensor.java b/src/main/java/com/pablissimo/sonar/TsLintSensor.java index 5b4eaf5..222e65d 100644 --- a/src/main/java/com/pablissimo/sonar/TsLintSensor.java +++ b/src/main/java/com/pablissimo/sonar/TsLintSensor.java @@ -16,9 +16,6 @@ import java.util.*; public class TsLintSensor implements Sensor { - public static final String CONFIG_FILENAME = "tslint.json"; - public static final String TSLINT_FALLBACK_PATH = "node_modules/tslint/bin/tslint"; - private static final Logger LOG = LoggerFactory.getLogger(TsLintExecutorImpl.class); private Settings settings; @@ -48,18 +45,14 @@ public void execute(SensorContext ctx) { return; } - String pathToTsLint = this.resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_PATH, TSLINT_FALLBACK_PATH); - String pathToTsLintConfig = this.resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH, CONFIG_FILENAME); - String rulesDir = this.resolver.getPath(ctx, TypeScriptPlugin.SETTING_TS_LINT_RULES_DIR, null); + TsLintExecutorConfig config = TsLintExecutorConfig.fromSettings(this.settings, ctx, this.resolver); - Integer tsLintTimeoutMs = Math.max(5000, settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)); - - if (pathToTsLint == null) { + if (config.getPathToTsLint() == null) { LOG.warn("Path to tslint not defined or not found. Skipping tslint analysis."); return; } - else if (pathToTsLintConfig == null) { - LOG.warn("Path to tslint.json configuration file not defined or not found. Skipping tslint analysis."); + else if (config.getConfigFile() == null && config.getPathToTsConfig() == null) { + LOG.warn("Path to tslint.json and tsconfig.json configuration files either not defined or not found - at least one is required. Skipping tslint analysis."); return; } @@ -83,12 +76,6 @@ else if (pathToTsLintConfig == null) { paths.add(pathAdjusted); fileMap.put(pathAdjusted, file); } - - TsLintExecutorConfig config = new TsLintExecutorConfig(); - config.setPathToTsLint(pathToTsLint); - config.setConfigFile(pathToTsLintConfig); - config.setRulesDir(rulesDir); - config.setTimeoutMs(tsLintTimeoutMs); List jsonResults = this.executor.execute(config, paths); diff --git a/src/main/java/com/pablissimo/sonar/TypeScriptPlugin.java b/src/main/java/com/pablissimo/sonar/TypeScriptPlugin.java index 28e8d91..841ec46 100644 --- a/src/main/java/com/pablissimo/sonar/TypeScriptPlugin.java +++ b/src/main/java/com/pablissimo/sonar/TypeScriptPlugin.java @@ -123,7 +123,7 @@ public class TypeScriptPlugin implements Plugin { public static final String SETTING_LCOV_REPORT_PATH = "sonar.ts.lcov.reportpath"; public static final String SETTING_TS_RULE_CONFIGS = "sonar.ts.ruleconfigs"; public static final String SETTING_TS_LINT_TYPECHECK = "sonar.ts.tslinttypecheck"; - public static final String SETTING_TS_LINT_TSCONFIG_PATH = "sonar.ts.tslinttsconfigpath"; + public static final String SETTING_TS_LINT_PROJECT_PATH = "sonar.ts.tslintprojectpath"; @Override public void define(Context ctx) { diff --git a/src/test/java/com/pablissimo/sonar/TsLintExecutorConfigTest.java b/src/test/java/com/pablissimo/sonar/TsLintExecutorConfigTest.java new file mode 100644 index 0000000..d22f36f --- /dev/null +++ b/src/test/java/com/pablissimo/sonar/TsLintExecutorConfigTest.java @@ -0,0 +1,133 @@ +package com.pablissimo.sonar; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import java.io.File; + +import org.junit.Test; +import org.sonar.api.batch.sensor.SensorContext; +import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.api.config.Settings; + +public class TsLintExecutorConfigTest { + + private TsLintExecutorConfig getNewConfig() { + return new TsLintExecutorConfig(); + } + + @Test + public void canGetSetPathToTsLint() { + TsLintExecutorConfig config = getNewConfig(); + config.setPathToTsLint("My path"); + + assertEquals("My path", config.getPathToTsLint()); + } + + @Test + public void canGetSetPathToTsLintConfig() { + TsLintExecutorConfig config = getNewConfig(); + config.setConfigFile("My path"); + + assertEquals("My path", config.getConfigFile()); + } + + @Test + public void canGetSetRulesDir() { + TsLintExecutorConfig config = getNewConfig(); + config.setRulesDir("My path"); + + assertEquals("My path", config.getRulesDir()); + } + + @Test + public void canGetSetTimeout() { + TsLintExecutorConfig config = getNewConfig(); + config.setTimeoutMs(12); + + assertEquals((Integer) 12, config.getTimeoutMs()); + } + + @Test + public void canGetSetTsConfigPath() { + TsLintExecutorConfig config = getNewConfig(); + config.setPathToTsConfig("My path"); + + assertEquals("My path", config.getPathToTsConfig()); + } + + @Test + public void canGetSetTypeCheck() { + TsLintExecutorConfig config = getNewConfig(); + config.setShouldPerformTypeCheck(true); + + assertTrue(config.shouldPerformTypeCheck()); + } + + @Test + public void useTsConfigInsteadOfFileList_returnsTrue_ifPathToTsConfigSet() { + TsLintExecutorConfig config = getNewConfig(); + config.setPathToTsConfig("My path"); + + assertTrue(config.useTsConfigInsteadOfFileList()); + } + + @Test + public void useTsConfigInsteadOfFileList_returnsFalse_ifPathToTsConfigNotSet() { + TsLintExecutorConfig config = getNewConfig(); + config.setPathToTsConfig(""); + + assertFalse(config.useTsConfigInsteadOfFileList()); + } + + @Test + public void fromSettings_initialisesFromSettingsAndResolver() { + Settings settings = new Settings(); + settings.setProperty(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT, 12000); + settings.setProperty(TypeScriptPlugin.SETTING_TS_LINT_TYPECHECK, true); + + PathResolver resolver = mock(PathResolver.class); + + when(resolver.getPath(any(SensorContext.class), + eq(TypeScriptPlugin.SETTING_TS_LINT_PATH), + eq(TsLintExecutorConfig.TSLINT_FALLBACK_PATH)) + ).thenReturn("tslint"); + + when(resolver.getPath(any(SensorContext.class), + eq(TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH), + eq(TsLintExecutorConfig.CONFIG_FILENAME)) + ).thenReturn("tslint.json"); + + when(resolver.getPath(any(SensorContext.class), + eq(TypeScriptPlugin.SETTING_TS_LINT_RULES_DIR), + eq((String) null)) + ).thenReturn("rulesdir"); + + when(resolver.getPath(any(SensorContext.class), + eq(TypeScriptPlugin.SETTING_TS_LINT_PROJECT_PATH), + eq((String) null)) + ).thenReturn("tsconfig.json"); + + TsLintExecutorConfig config = TsLintExecutorConfig.fromSettings(settings, SensorContextTester.create(new File("")), resolver); + + assertEquals("tslint", config.getPathToTsLint()); + assertEquals("tslint.json", config.getConfigFile()); + assertEquals("rulesdir", config.getRulesDir()); + assertEquals("tsconfig.json", config.getPathToTsConfig()); + + assertTrue(config.shouldPerformTypeCheck()); + assertEquals((Integer) 12000, config.getTimeoutMs()); + } + + @Test + public void fromSettings_setsTimeoutTo5000msMinimum_ifSetToLess() { + Settings settings = new Settings(); + settings.setProperty(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT, 1000); + + PathResolver resolver = mock(PathResolver.class); + + TsLintExecutorConfig config = TsLintExecutorConfig.fromSettings(settings, SensorContextTester.create(new File("")), resolver); + + assertEquals((Integer) 5000, config.getTimeoutMs()); + } +} diff --git a/src/test/java/com/pablissimo/sonar/TsLintExecutorImplTest.java b/src/test/java/com/pablissimo/sonar/TsLintExecutorImplTest.java index 3f9572b..2ae6c55 100644 --- a/src/test/java/com/pablissimo/sonar/TsLintExecutorImplTest.java +++ b/src/test/java/com/pablissimo/sonar/TsLintExecutorImplTest.java @@ -80,6 +80,61 @@ public Integer answer(InvocationOnMock invocation) throws Throwable { assertEquals(2 * 40000, theTimeout); } + @Test + public void doesNotSendFileListToTsLint_ifConfigSaysToUseProjectFile() { + final ArrayList capturedCommands = new ArrayList(); + final ArrayList capturedTimeouts = new ArrayList(); + + Answer captureCommand = new Answer() { + @Override + public Integer answer(InvocationOnMock invocation) throws Throwable { + capturedCommands.add((Command) invocation.getArguments()[0]); + capturedTimeouts.add((long) invocation.getArguments()[3]); + return 0; + } + }; + + this.config.setPathToTsConfig("path/to/tsconfig.json"); + + when(this.commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), any(long.class))).then(captureCommand); + this.executorImpl.execute(this.config, Arrays.asList(new String[] { "path/to/file", "path/to/another" })); + + assertEquals(1, capturedCommands.size()); + + Command theCommand = capturedCommands.get(0); + long theTimeout = capturedTimeouts.get(0); + + assertEquals("node path/to/tslint --format json --rules-dir path/to/rules --out path/to/temp --config path/to/config --project path/to/tsconfig.json", theCommand.toCommandLine()); + // Timeout should be just what we specified since we're not batching + + assertEquals(40000, theTimeout); + } + + @Test + public void usesTypeCheckParameter_ifConfigSaysToUseTypeCheck() { + final ArrayList capturedCommands = new ArrayList(); + + Answer captureCommand = new Answer() { + @Override + public Integer answer(InvocationOnMock invocation) throws Throwable { + capturedCommands.add((Command) invocation.getArguments()[0]); + return 0; + } + }; + + this.config.setPathToTsConfig("path/to/tsconfig.json"); + this.config.setShouldPerformTypeCheck(true); + + when(this.commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), any(long.class))).then(captureCommand); + this.executorImpl.execute(this.config, Arrays.asList(new String[] { "path/to/file", "path/to/another" })); + + assertEquals(1, capturedCommands.size()); + + Command theCommand = capturedCommands.get(0); + + assertEquals("node path/to/tslint --format json --rules-dir path/to/rules --out path/to/temp --config path/to/config --project path/to/tsconfig.json --type-check", theCommand.toCommandLine()); + } + @Test public void DoesNotAddRulesDirParameter_IfNull() { final ArrayList capturedCommands = new ArrayList();