From 80acea0af26b6ce3340160341fec2219496ab8b8 Mon Sep 17 00:00:00 2001 From: Sascha Theves Date: Sat, 29 Oct 2016 20:26:59 +0200 Subject: [PATCH] remove dependency to sslr and fix LCOVParser --- pom.xml | 9 +- .../com/pablissimo/sonar/LCOVParserImpl.java | 356 +++++++++--------- .../pablissimo/sonar/TsLintSensorTest.java | 102 +++-- 3 files changed, 222 insertions(+), 245 deletions(-) diff --git a/pom.xml b/pom.xml index a94e0ca..314ae63 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ sonar-plugin 0.94-SNAPSHOT - SonarQube TypeScript Plugin + TypeScript Analyse TypeScript projects 2013 https://github.com/pablissimo/SonarTsPlugin @@ -61,11 +61,6 @@ ${sonar.buildVersion} provided - - org.sonarsource.sslr - sslr-core - ${sslr.version} - org.slf4j slf4j-api @@ -77,7 +72,6 @@ gson 2.3 - commons-io commons-io @@ -156,7 +150,6 @@ true -Xlint:all - -Werror diff --git a/src/main/java/com/pablissimo/sonar/LCOVParserImpl.java b/src/main/java/com/pablissimo/sonar/LCOVParserImpl.java index 2899b9c..33953d5 100644 --- a/src/main/java/com/pablissimo/sonar/LCOVParserImpl.java +++ b/src/main/java/com/pablissimo/sonar/LCOVParserImpl.java @@ -21,18 +21,6 @@ */ package com.pablissimo.sonar; -import com.google.common.base.Objects; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; -import javax.annotation.CheckForNull; - import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.coverage.CoverageType; @@ -40,199 +28,201 @@ import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.*; +import java.util.stream.Collectors; + /** * http://ltp.sourceforge.net/coverage/lcov/geninfo.1.php */ public class LCOVParserImpl implements LCOVParser { - private static final String SF = "SF:"; - private static final String DA = "DA:"; - private static final String BRDA = "BRDA:"; - - private final Map coverageByFile; - private final SensorContext context; - private final List unresolvedPaths = new ArrayList(); - - private static final Logger LOG = Loggers.get(LCOVParser.class); - - private LCOVParserImpl(List lines, SensorContext context) { - this.context = context; - this.coverageByFile = parse(lines); - } - - static LCOVParser create(SensorContext context, File... files) { - final List lines = new LinkedList<>(); - for(File file: files) { - try { - lines.addAll(Files.lines(file.toPath()).collect(Collectors.toList())); - } catch (IOException e) { - throw new IllegalArgumentException("Could not read content from file: " + file, e); - } + private static final String SF = "SF:"; + private static final String DA = "DA:"; + private static final String BRDA = "BRDA:"; + + private final Map coverageByFile; + private final SensorContext context; + private final List unresolvedPaths = new ArrayList<>(); + + private static final Logger LOG = Loggers.get(LCOVParser.class); + + private LCOVParserImpl(List lines, SensorContext context) { + this.context = context; + this.coverageByFile = parse(lines); } - - return new LCOVParserImpl(lines, context); - } - - Map coverageByFile() { - return coverageByFile; - } - - List unresolvedPaths() { - return unresolvedPaths; - } - - public Map parseFile(File file) { - final List lines; - try - { - lines = Files.readAllLines(file.toPath()); - } - catch (IOException e) - { - throw new IllegalArgumentException("Could not read content from file: " + file, e); - } - - return parse(lines); - } - - public Map parse(List lines) { - final Map files = new HashMap(); - FileData fileData = null; - - for (String line : lines) { - if (line.startsWith(SF)) { - // SF: - fileData = loadCurrentFileData(files, line); - } else if (fileData != null) { - if (line.startsWith(DA)) { - // DA:,[,] - String execution = line.substring(DA.length()); - String executionCount = execution.substring(execution.indexOf(',') + 1); - String lineNumber = execution.substring(0, execution.indexOf(',')); - - try { - fileData.addLine(Integer.valueOf(lineNumber), Integer.valueOf(executionCount)); - } catch (IllegalArgumentException e) { - logWrongDataWarning("DA", lineNumber, e); - } - } else if (line.startsWith(BRDA)) { - // BRDA:,,, - String[] tokens = line.substring(BRDA.length()).trim().split(","); - String lineNumber = tokens[0]; - String branchNumber = tokens[1] + tokens[2]; - String taken = tokens[3]; - - try { - fileData.addBranch(Integer.valueOf(lineNumber), branchNumber, "-".equals(taken) ? 0 : Integer.valueOf(taken)); - } catch (IllegalArgumentException e) { - logWrongDataWarning("BRDA", lineNumber, e); - } + + static LCOVParser create(SensorContext context, File... files) { + final List lines = new LinkedList<>(); + for (File file : files) { + try { + lines.addAll(Files.lines(file.toPath()).collect(Collectors.toList())); + } catch (IOException e) { + throw new IllegalArgumentException("Could not read content from file: " + file, e); + } } - } + return new LCOVParserImpl(lines, context); } - Map coveredFiles = new HashMap(); - for (Map.Entry e : files.entrySet()) { - NewCoverage newCoverage = context.newCoverage().onFile(e.getKey()).ofType(CoverageType.UNIT); - e.getValue().save(newCoverage); - coveredFiles.put(e.getKey(), newCoverage); + Map coverageByFile() { + return coverageByFile; } - return coveredFiles; - } - - private static void logWrongDataWarning(String dataType, String lineNumber, IllegalArgumentException e) { - LOG.warn(String.format("Problem during processing LCOV report: can't save %s data for line %s (%s).", dataType, lineNumber, e.getMessage())); - } - - @CheckForNull - private FileData loadCurrentFileData(final Map files, String line) { - String filePath = line.substring(SF.length()); - FileData fileData = null; - // some tools (like Istanbul, Karma) provide relative paths, so let's consider them relative to project directory - InputFile inputFile = context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(filePath)); - if (inputFile != null) { - fileData = files.get(inputFile); - if (fileData == null) { - fileData = new FileData(inputFile); - files.put(inputFile, fileData); - } - } else { - unresolvedPaths.add(filePath); - } - return fileData; - } - - private static class FileData { - /** - * line number -> branch number -> taken - */ - private Map> branches = new HashMap>(); - - /** - * line number -> execution count - */ - private Map hits = new HashMap(); - - /** - * Number of lines in the file - * Required to check if line exist in a file, see {@link #checkLine(Integer)} - */ - private final int linesInFile; - - private final String filename; - private static final String WRONG_LINE_EXCEPTION_MESSAGE = "Line with number %s doesn't belong to file %s"; - - FileData(InputFile inputFile) { - linesInFile = inputFile.lines(); - filename = inputFile.relativePath(); + + List unresolvedPaths() { + return unresolvedPaths; } - void addBranch(Integer lineNumber, String branchNumber, Integer taken) { - checkLine(lineNumber); + public Map parseFile(File file) { + final List lines; + try { + lines = Files.readAllLines(file.toPath()); + } catch (IOException e) { + throw new IllegalArgumentException("Could not read content from file: " + file, e); + } - Map branchesForLine = branches.get(lineNumber); - if (branchesForLine == null) { - branchesForLine = new HashMap(); - branches.put(lineNumber, branchesForLine); - } - Integer currentValue = branchesForLine.get(branchNumber); - branchesForLine.put(branchNumber, Objects.firstNonNull(currentValue, 0) + taken); + return parse(lines); } - void addLine(Integer lineNumber, Integer executionCount) { - checkLine(lineNumber); + public Map parse(List lines) { + final Map files = new HashMap<>(); + FileData fileData = null; + + for (String line : lines) { + if (line.startsWith(SF)) { + // SF: + fileData = loadCurrentFileData(files, line); + } else if (fileData != null) { + if (line.startsWith(DA)) { + // DA:,[,] + String execution = line.substring(DA.length()); + String executionCount = execution.substring(execution.indexOf(',') + 1); + String lineNumber = execution.substring(0, execution.indexOf(',')); + + try { + fileData.addLine(Integer.valueOf(lineNumber), Integer.valueOf(executionCount)); + } catch (IllegalArgumentException e) { + logWrongDataWarning("DA", lineNumber, e); + } + } else if (line.startsWith(BRDA)) { + // BRDA:,,, + String[] tokens = line.substring(BRDA.length()).trim().split(","); + String lineNumber = tokens[0]; + String branchNumber = tokens[1] + tokens[2]; + String taken = tokens[3]; + + try { + fileData.addBranch(Integer.valueOf(lineNumber), branchNumber, "-".equals(taken) ? 0 : Integer.valueOf(taken)); + } catch (IllegalArgumentException e) { + logWrongDataWarning("BRDA", lineNumber, e); + } + } + } - Integer currentValue = hits.get(lineNumber); - if (currentValue == null) { - currentValue = 0; - } - - hits.put(lineNumber, currentValue + executionCount); - } + } - void save(NewCoverage newCoverage) { - for (Map.Entry e : hits.entrySet()) { - newCoverage.lineHits(e.getKey(), e.getValue()); - } - for (Map.Entry> e : branches.entrySet()) { - int conditions = e.getValue().size(); - int covered = 0; - for (Integer taken : e.getValue().values()) { - if (taken > 0) { - covered++; - } + Map coveredFiles = new HashMap<>(); + for (Map.Entry e : files.entrySet()) { + NewCoverage newCoverage = context.newCoverage().onFile(e.getKey()).ofType(CoverageType.UNIT); + e.getValue().save(newCoverage); + coveredFiles.put(e.getKey(), newCoverage); } + return coveredFiles; + } - newCoverage.conditions(e.getKey(), conditions, covered); - } + private static void logWrongDataWarning(String dataType, String lineNumber, IllegalArgumentException e) { + LOG.warn(String.format("Problem during processing LCOV report: can't save %s data for line %s (%s).", dataType, lineNumber, e.getMessage())); } - private void checkLine(Integer lineNumber) { - if (lineNumber < 1 || lineNumber > linesInFile) { - throw new IllegalArgumentException(String.format(WRONG_LINE_EXCEPTION_MESSAGE, lineNumber, filename)); - } + private FileData loadCurrentFileData(final Map files, String line) { + String filePath = line.substring(SF.length()); + FileData fileData = null; + // some tools (like Istanbul, Karma) provide relative paths, so let's consider them relative to project directory + InputFile inputFile = context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(filePath)); + if (inputFile != null) { + fileData = files.get(inputFile); + if (fileData == null) { + fileData = new FileData(inputFile); + files.put(inputFile, fileData); + } + } else { + unresolvedPaths.add(filePath); + } + return fileData; } - } + private static class FileData { + /** + * line number -> branch number -> taken + */ + private Map> branches = new HashMap<>(); + + /** + * line number -> execution count + */ + private Map hits = new HashMap<>(); + + /** + * Number of lines in the file + * Required to check if line exist in a file, see {@link #checkLine(Integer)} + */ + private final int linesInFile; + + private final String filename; + private static final String WRONG_LINE_EXCEPTION_MESSAGE = "Line with number %s doesn't belong to file %s"; + + FileData(InputFile inputFile) { + linesInFile = inputFile.lines(); + filename = inputFile.relativePath(); + } + + void addBranch(Integer lineNumber, String branchNumber, Integer taken) { + checkLine(lineNumber); + + Map branchesForLine = branches.get(lineNumber); + if (branchesForLine == null) { + branchesForLine = new HashMap<>(); + branches.put(lineNumber, branchesForLine); + } + Integer currentValue = branchesForLine.get(branchNumber); + branchesForLine.put(branchNumber, currentValue == null ? 0 : currentValue + taken); + } + + void addLine(Integer lineNumber, Integer executionCount) { + checkLine(lineNumber); + + Integer currentValue = hits.get(lineNumber); + if (currentValue == null) { + currentValue = 0; + } + + hits.put(lineNumber, currentValue + executionCount); + } + + void save(NewCoverage newCoverage) { + for (Map.Entry e : hits.entrySet()) { + newCoverage.lineHits(e.getKey(), e.getValue()); + } + for (Map.Entry> e : branches.entrySet()) { + int conditions = e.getValue().size(); + int covered = 0; + for (Integer taken : e.getValue().values()) { + if (taken > 0) { + covered++; + } + } + + newCoverage.conditions(e.getKey(), conditions, covered); + } + } + + private void checkLine(Integer lineNumber) { + if (lineNumber < 1 || lineNumber > linesInFile) { + throw new IllegalArgumentException(String.format(WRONG_LINE_EXCEPTION_MESSAGE, lineNumber, filename)); + } + } + + } } \ No newline at end of file diff --git a/src/test/java/com/pablissimo/sonar/TsLintSensorTest.java b/src/test/java/com/pablissimo/sonar/TsLintSensorTest.java index f157c76..9a9aea5 100644 --- a/src/test/java/com/pablissimo/sonar/TsLintSensorTest.java +++ b/src/test/java/com/pablissimo/sonar/TsLintSensorTest.java @@ -1,16 +1,9 @@ package com.pablissimo.sonar; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - -import java.io.File; -import java.io.IOException; -import java.util.HashMap; -import java.util.List; - +import com.pablissimo.sonar.model.TsLintIssue; +import com.pablissimo.sonar.model.TsLintPosition; import org.junit.Before; import org.junit.Test; -import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; @@ -18,15 +11,21 @@ import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; import org.sonar.api.batch.sensor.internal.SensorContextTester; import org.sonar.api.config.Settings; -import com.pablissimo.sonar.model.TsLintIssue; -import com.pablissimo.sonar.model.TsLintPosition; - import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.System2; +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.*; + public class TsLintSensorTest { Settings settings; - + DefaultInputFile file; TsLintExecutor executor; @@ -34,22 +33,22 @@ public class TsLintSensorTest { TsLintSensor sensor; SensorContextTester context; - + PathResolver resolver; HashMap fakePathResolutions; - + System2 system; - + @Before public void setUp() throws Exception { - this.fakePathResolutions = new HashMap(); + this.fakePathResolutions = new HashMap<>(); 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); this.system = mock(System2.class); - + when(this.settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)).thenReturn(45000); this.executor = mock(TsLintExecutor.class); this.parser = mock(TsLintParser.class); @@ -57,31 +56,26 @@ public void setUp() throws Exception { this.sensor = spy(new TsLintSensor(settings, this.system)); this.file = new DefaultInputFile("", "path/to/file") - .setLanguage(TypeScriptLanguage.LANGUAGE_KEY) - .setLines(1) - .setLastValidOffset(999) - .setOriginalLineOffsets(new int[] { 5 }); - + .setLanguage(TypeScriptLanguage.LANGUAGE_KEY) + .setLines(1) + .setLastValidOffset(999) + .setOriginalLineOffsets(new int[]{5}); + doReturn(this.executor).when(this.sensor).getTsLintExecutor(); doReturn(this.parser).when(this.sensor).getTsLintParser(); doReturn(this.resolver).when(this.sensor).getPathResolver(); - + this.context = SensorContextTester.create(new File("")); - this.context.fileSystem().add(this.file); - + this.context.fileSystem().add(this.file); + 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 lookUpFakePath = new Answer() { - @Override - public String answer(InvocationOnMock invocation) throws Throwable { - return fakePathResolutions.get(invocation.getArgumentAt(1, String.class)); - } - }; - + Answer lookUpFakePath = invocation -> fakePathResolutions.get(invocation.getArgumentAt(1, String.class)); + doAnswer(lookUpFakePath).when(this.resolver).getPath(any(SensorContext.class), any(String.class), any(String.class)); } @@ -89,37 +83,37 @@ public String answer(InvocationOnMock invocation) throws Throwable { 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"); - issue.setName(this.file.absolutePath().replace("\\", "/")); + issue.setName(this.file.absolutePath().replace("\\", "/")); TsLintPosition startPosition = new TsLintPosition(); startPosition.setLine(0); issue.setStartPosition(startPosition); - TsLintIssue[][] issues = new TsLintIssue[][] { - new TsLintIssue[] { issue } + TsLintIssue[][] issues = new TsLintIssue[][]{ + new TsLintIssue[]{issue} }; - + when(this.parser.parse(any(String.class))).thenReturn(issues); this.sensor.execute(this.context); - + assertEquals(1, this.context.allIssues().size()); assertEquals("rule name", this.context.allIssues().iterator().next().ruleKey().rule()); } @@ -129,20 +123,20 @@ 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(String.class), any(String.class), any(String.class), any(List.class), any(Integer.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(String.class), any(String.class), any(String.class), any(List.class), any(Integer.class)); - + assertEquals(0, this.context.allIssues().size()); } @@ -156,7 +150,7 @@ 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(any(String.class), any(String.class), any(String.class), any(List.class), eq(5000)); @@ -165,7 +159,7 @@ public void execute_callsExecutorWithAtLeast5000msTimeout() throws IOException { @Test public void execute_callsExecutorWithConfiguredPaths() { this.sensor.execute(this.context); - + verify(this.executor, times(1)).execute(eq("/path/to/tslint"), eq("/path/to/tslint.json"), eq("/path/to/rules"), any(List.class), any(Integer.class)); } }