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

Commit

Permalink
Fixes #114. (#115)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverbrandt authored and Pablissimo committed Feb 15, 2017
1 parent 21d9b04 commit 106f19a
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 64 deletions.
29 changes: 16 additions & 13 deletions src/main/java/com/pablissimo/sonar/TsLintSensor.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class TsLintSensor implements Sensor {
private PathResolver resolver;
private TsLintExecutor executor;
private TsLintParser parser;

public TsLintSensor(Settings settings, PathResolver resolver, TsLintExecutor executor, TsLintParser parser) {
this.settings = settings;
this.resolver = resolver;
Expand All @@ -47,13 +47,16 @@ public void execute(SensorContext ctx) {

TsLintExecutorConfig config = TsLintExecutorConfig.fromSettings(this.settings, ctx, this.resolver);

if (config.getPathToTsLint() == null) {
LOG.warn("Path to tslint not defined or not found. Skipping tslint analysis.");
return;
}
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;
if (!config.useExistingTsLintOutput()) {
if (config.getPathToTsLint() == null) {
LOG.warn("Path to tslint not defined or not found. Skipping tslint analysis.");
return;
} 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;
}
}
}

boolean skipTypeDefFiles = settings.getBoolean(TypeScriptPlugin.SETTING_EXCLUDE_TYPE_DEFINITION_FILES);
Expand All @@ -74,7 +77,7 @@ else if (config.getConfigFile() == null && config.getPathToTsConfig() == null) {
String pathAdjusted = file.absolutePath();
paths.add(pathAdjusted);
}

List<String> jsonResults = this.executor.execute(config, paths);

Map<String, List<TsLintIssue>> issues = this.parser.parse(jsonResults);
Expand All @@ -94,11 +97,11 @@ else if (config.getConfigFile() == null && config.getPathToTsConfig() == null) {

File matchingFile = ctx.fileSystem().resolvePath(filePath);
InputFile inputFile = null;

if (shouldSkipFile(matchingFile, skipTypeDefFiles)) {
continue;
}

if (matchingFile != null) {
try {
inputFile = ctx.fileSystem().inputFile(ctx.fileSystem().predicates().is(matchingFile));
Expand All @@ -108,7 +111,7 @@ else if (config.getConfigFile() == null && config.getPathToTsConfig() == null) {
continue;
}
}

if (inputFile == null) {
LOG.warn("TsLint reported issues against a file that isn't in the analysis set - will be ignored: " + filePath);
continue;
Expand Down Expand Up @@ -142,7 +145,7 @@ else if (config.getConfigFile() == null && config.getPathToTsConfig() == null) {
}
}
}

private boolean shouldSkipFile(File f, boolean skipTypeDefFiles) {
return skipTypeDefFiles && f.getName().toLowerCase().endsWith("." + TypeScriptLanguage.LANGUAGE_DEFINITION_EXTENSION);
}
Expand Down
115 changes: 64 additions & 51 deletions src/test/java/com/pablissimo/sonar/TsLintSensorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

public class TsLintSensorTest {
Settings settings;

DefaultInputFile file;
DefaultInputFile typeDefFile;

Expand All @@ -37,19 +37,19 @@ public class TsLintSensorTest {
TsLintSensor sensor;

SensorContextTester context;

PathResolver resolver;
HashMap<String, String> fakePathResolutions;

ArgumentCaptor<TsLintExecutorConfig> configCaptor;

@Before
public void setUp() throws Exception {
this.fakePathResolutions = new HashMap<String, String>();
this.fakePathResolutions.put(TypeScriptPlugin.SETTING_TS_LINT_PATH, "/path/to/tslint");
this.fakePathResolutions.put(TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH, "/path/to/tslint.json");
this.fakePathResolutions.put(TypeScriptPlugin.SETTING_TS_LINT_RULES_DIR, "/path/to/rules");

this.settings = mock(Settings.class);
when(this.settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)).thenReturn(45000);
when(this.settings.getBoolean(TypeScriptPlugin.SETTING_TS_LINT_ENABLED)).thenReturn(true);
Expand All @@ -64,53 +64,53 @@ public void setUp() throws Exception {
.setLines(1)
.setLastValidOffset(999)
.setOriginalLineOffsets(new int[] { 5 });

this.typeDefFile = new DefaultInputFile("", "path/to/file.d.ts")
.setLanguage(TypeScriptLanguage.LANGUAGE_KEY)
.setLines(1)
.setLastValidOffset(999)
.setOriginalLineOffsets(new int[] { 5 });

this.context = SensorContextTester.create(new File(""));
this.context.fileSystem().add(this.file);
this.context.fileSystem().add(this.typeDefFile);

ActiveRulesBuilder rulesBuilder = new ActiveRulesBuilder();
rulesBuilder.create(RuleKey.of(TsRulesDefinition.REPOSITORY_NAME, "rule name")).activate();

this.context.setActiveRules(rulesBuilder.build());

// Pretend all paths are absolute
Answer<String> lookUpFakePath = new Answer<String>() {
@Override
public String answer(InvocationOnMock invocation) throws Throwable {
return fakePathResolutions.get(invocation.<String>getArgument(1));
}
}
};

doAnswer(lookUpFakePath).when(this.resolver).getPath(any(SensorContext.class), any(String.class), (String) any());

this.configCaptor = ArgumentCaptor.forClass(TsLintExecutorConfig.class);
}

@Test
public void describe_setsName() {
DefaultSensorDescriptor desc = new DefaultSensorDescriptor();
this.sensor.describe(desc);

assertNotNull(desc.name());
}

@Test
public void describe_setsLanguage() {
DefaultSensorDescriptor desc = new DefaultSensorDescriptor();
this.sensor.describe(desc);

assertEquals(TypeScriptLanguage.LANGUAGE_KEY, desc.languages().iterator().next());
}

@Test
public void execute_addsIssues() {
public void execute_addsIssues() {
TsLintIssue issue = new TsLintIssue();
issue.setFailure("failure");
issue.setRuleName("rule name");
Expand All @@ -126,14 +126,14 @@ public void execute_addsIssues() {

Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(issue.getName(), issueList);

when(this.parser.parse(any(List.class))).thenReturn(issues);
this.sensor.execute(this.context);

assertEquals(1, this.context.allIssues().size());
assertEquals("rule name", this.context.allIssues().iterator().next().ruleKey().rule());
}

@Test
public void execute_addsIssues_evenIfReportedAgainstRelativePaths() {
TsLintIssue issue = new TsLintIssue();
Expand All @@ -151,38 +151,38 @@ public void execute_addsIssues_evenIfReportedAgainstRelativePaths() {

Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(issue.getName(), issueList);

when(this.parser.parse(any(List.class))).thenReturn(issues);
this.sensor.execute(this.context);

assertEquals(1, this.context.allIssues().size());
assertEquals("rule name", this.context.allIssues().iterator().next().ruleKey().rule());
}

@Test
public void execute_doesNotThrow_ifParserReturnsNoResult() {
when(this.parser.parse(any(List.class))).thenReturn(null);

this.sensor.execute(this.context);
}

@Test
public void execute_doesNotThrow_ifFileIssuesNull() {
Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(this.file.absolutePath().replace("\\", "/"), null);
when(this.parser.parse(any(List.class))).thenReturn(issues);

this.sensor.execute(this.context);
}

@Test
public void execute_doesNotThrow_ifFileIssuesEmpty() {
Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(this.file.absolutePath().replace("\\", "/"), new ArrayList<TsLintIssue>());
when(this.parser.parse(any(List.class))).thenReturn(issues);

this.sensor.execute(this.context);
}
}

@Test
public void execute_addsToUnknownRuleBucket_whenRuleNameNotRecognised() {
Expand All @@ -201,14 +201,14 @@ public void execute_addsToUnknownRuleBucket_whenRuleNameNotRecognised() {

Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(issue.getName(), issueList);

when(this.parser.parse(any(List.class))).thenReturn(issues);
this.sensor.execute(this.context);

assertEquals(1, this.context.allIssues().size());
assertEquals(TsRulesDefinition.TSLINT_UNKNOWN_RULE.key, this.context.allIssues().iterator().next().ruleKey().rule());
}

@Test
public void execute_doesNotThrow_ifTsLintReportsAgainstFileNotInAnalysisSet() {
TsLintIssue issue = new TsLintIssue();
Expand All @@ -226,13 +226,13 @@ public void execute_doesNotThrow_ifTsLintReportsAgainstFileNotInAnalysisSet() {

Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(issue.getName(), issueList);

when(this.parser.parse(any(List.class))).thenReturn(issues);
this.sensor.execute(this.context);
this.sensor.execute(this.context);
}

@Test
public void execute_ignoresTypeDefinitionFilesIfConfigured() {
public void execute_ignoresTypeDefinitionFilesIfConfigured() {
TsLintIssue issue = new TsLintIssue();
issue.setFailure("failure");
issue.setRuleName("rule name");
Expand All @@ -248,10 +248,10 @@ public void execute_ignoresTypeDefinitionFilesIfConfigured() {

Map<String, List<TsLintIssue>> issues = new HashMap<String, List<TsLintIssue>>();
issues.put(issue.getName(), issueList);

when(this.parser.parse(any(List.class))).thenReturn(issues);
when(this.settings.getBoolean(TypeScriptPlugin.SETTING_EXCLUDE_TYPE_DEFINITION_FILES)).thenReturn(true);
this.sensor.execute(this.context);
this.sensor.execute(this.context);

assertEquals(0, this.context.allIssues().size());
}
Expand All @@ -261,31 +261,31 @@ public void execute_doesNothingWhenNotConfigured() throws IOException {
this.fakePathResolutions.remove(TypeScriptPlugin.SETTING_TS_LINT_PATH);

this.sensor.execute(this.context);

verify(this.executor, times(0)).execute(any(TsLintExecutorConfig.class), any(List.class));

assertEquals(0, this.context.allIssues().size());
}

@Test
public void analyse_doesNothingWhenDisabled() throws IOException {
when(this.settings.getBoolean(TypeScriptPlugin.SETTING_TS_LINT_ENABLED)).thenReturn(Boolean.FALSE);

this.sensor.execute(this.context);

verify(this.executor, times(0)).execute(any(TsLintExecutorConfig.class), any(List.class));

assertEquals(0, this.context.allIssues().size());
}

@Test
public void execute_doesNothingWhenNoConfigPathset() throws IOException {
this.fakePathResolutions.remove(TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH);

this.sensor.execute(this.context);

verify(this.executor, times(0)).execute(any(TsLintExecutorConfig.class), any(List.class));

assertEquals(0, this.context.allIssues().size());
}

Expand All @@ -300,20 +300,33 @@ public void execute_callsExecutorWithSuppliedTimeout() throws IOException {
@Test
public void execute_callsExecutorWithAtLeast5000msTimeout() throws IOException {
when(this.settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)).thenReturn(-500);

this.sensor.execute(this.context);

verify(this.executor, times(1)).execute(this.configCaptor.capture(), any(List.class));
assertEquals((Integer) 5000, this.configCaptor.getValue().getTimeoutMs());
}

@Test
public void execute_callsExecutorWithConfiguredPaths() {
this.sensor.execute(this.context);

verify(this.executor, times(1)).execute(this.configCaptor.capture(), any(List.class));
assertEquals("/path/to/tslint", this.configCaptor.getValue().getPathToTsLint());
assertEquals("/path/to/tslint.json", this.configCaptor.getValue().getConfigFile());
assertEquals("/path/to/rules", this.configCaptor.getValue().getRulesDir());
}

@Test
public void execute_callsExecutorWithTslintOutput() throws IOException {
this.fakePathResolutions.remove(TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH);
this.fakePathResolutions.put(TypeScriptPlugin.SETTING_TS_LINT_OUTPUT_PATH, "/path/to/output");

this.sensor.execute(this.context);

verify(this.executor, times(1)).execute(any(TsLintExecutorConfig.class), any(List.class));

assertEquals(0, this.context.allIssues().size());
}

}

0 comments on commit 106f19a

Please sign in to comment.