Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,38 +49,42 @@ public void import_report() {
@Test
public void missing_report() {
analyseProjectWithReport("missing");
assertThat(issues()).hasSize(0);
assertThat(issues()).isEmpty();
}

@Test
public void invalid_report() {
BuildResult result = analyseProjectWithReport("invalid.txt");
assertThat(result.getLogs()).contains("Cannot parse the line: trash");
assertThat(issues()).hasSize(0);
assertThat(issues()).isEmpty();
}

@Test
public void unknown_rule() {
BuildResult result = analyseProjectWithReport("rule-unknown.txt");
assertThat(result.getLogs()).doesNotContain("Pylint rule 'C0102' is unknown");
assertThat(result.getLogs()).containsOnlyOnce("Pylint rule 'C8888' is unknown");
assertThat(result.getLogs()).containsOnlyOnce("Pylint rule 'C9999' is unknown");
assertThat(issues()).hasSize(0);
assertThat(issues()).hasSize(4);
}

@Test
public void multiple_reports() {
analyseProjectWithReport("pylint-report.txt, rule-unknown.txt");
assertThat(issues()).hasSize(8);
}

private static List<Issue> issues() {
return newWsClient().issues().search(new SearchRequest().setProjects(singletonList(PROJECT))).getIssuesList();
}

private static BuildResult analyseProjectWithReport(String reportPath) {
private static BuildResult analyseProjectWithReport(String reportPaths) {
ORCHESTRATOR.resetData();
ORCHESTRATOR.getServer().provisionProject(PROJECT, PROJECT);
ORCHESTRATOR.getServer().associateProjectToQualityProfile(PROJECT, "py", "pylint-rules");
ORCHESTRATOR.getServer().associateProjectToQualityProfile(PROJECT, "py", "no_rule");

return ORCHESTRATOR.executeBuild(
SonarScanner.create()
.setDebugLogs(true)
.setProjectDir(new File("projects/pylint_project"))
.setProperty("sonar.python.pylint.reportPath", reportPath));
.setProperty("sonar.python.pylint.reportPaths", reportPaths));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
@RunWith(Suite.class)
@Suite.SuiteClasses({
BanditReportTest.class,
PylintReportTest.class,
MetricsTest.class,
CPDTest.class,
CoverageTest.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,27 @@


import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.sonar.api.utils.log.Logger;
import java.util.stream.Collectors;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.rule.Severity;
import org.sonar.api.batch.sensor.Sensor;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.batch.sensor.issue.NewExternalIssue;
import org.sonar.api.batch.sensor.issue.NewIssueLocation;
import org.sonar.api.rules.RuleType;
import org.sonar.api.utils.log.Logger;
import org.sonarsource.analyzer.commons.ExternalReportProvider;
import org.sonarsource.analyzer.commons.internal.json.simple.parser.ParseException;

public abstract class ExternalIssuesSensor implements Sensor {

private static final int MAX_LOGGED_FILE_NAMES = 20;
private static final Long DEFAULT_CONSTANT_DEBT_MINUTES = 5L;

@Override
public void describe(SensorDescriptor descriptor) {
Expand All @@ -47,10 +55,18 @@ public void describe(SensorDescriptor descriptor) {
public void execute(SensorContext context) {
Set<String> unresolvedInputFiles = new HashSet<>();
List<File> reportFiles = ExternalReportProvider.getReportFiles(context, reportPathKey());
reportFiles.forEach(report -> importReport(report, context, unresolvedInputFiles));
reportFiles.forEach(report -> importExternalReport(report, context, unresolvedInputFiles));
logUnresolvedInputFiles(unresolvedInputFiles);
}

private void importExternalReport(File reportPath, SensorContext context, Set<String> unresolvedInputFiles) {
try {
importReport(reportPath, context, unresolvedInputFiles);
} catch (IOException | ParseException | RuntimeException e) {
logFileCantBeRead(e, reportPath);
}
}

private void logUnresolvedInputFiles(Set<String> unresolvedInputFiles) {
if (unresolvedInputFiles.isEmpty()) {
return;
Expand All @@ -62,7 +78,40 @@ private void logUnresolvedInputFiles(Set<String> unresolvedInputFiles) {
logger().warn("Failed to resolve {} file path(s) in " + linterName() + " report. No issues imported related to file(s): {}", unresolvedInputFiles.size(), fileList);
}

protected abstract void importReport(File reportPath, SensorContext context, Set<String> unresolvedInputFiles);
private void logFileCantBeRead(Exception e, File reportPath) {
logger().error("No issues information will be saved as the report file '{}' can't be read. {}: {}"
, reportPath, e.getClass().getSimpleName(), e.getMessage());
}

protected void saveIssue(SensorContext context, TextReportReader.Issue issue, Set<String> unresolvedInputFiles, String linterKey) {
InputFile inputFile = context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(issue.filePath));
if (inputFile == null) {
unresolvedInputFiles.add(issue.filePath);
return;
}

NewExternalIssue newExternalIssue = context.newExternalIssue();
newExternalIssue
.type(RuleType.CODE_SMELL)
.severity(Severity.MAJOR)
.remediationEffortMinutes(DEFAULT_CONSTANT_DEBT_MINUTES);

NewIssueLocation primaryLocation = newExternalIssue.newLocation()
.message(issue.message)
.on(inputFile);
if (issue.columnNumber != null && issue.columnNumber < inputFile.selectLine(issue.lineNumber).end().lineOffset()) {
primaryLocation.at(inputFile.newRange(issue.lineNumber, issue.columnNumber, issue.lineNumber, issue.columnNumber + 1));
} else {
// Pylint formatted issues might not provide column information
primaryLocation.at(inputFile.selectLine(issue.lineNumber));
}

newExternalIssue.at(primaryLocation);
newExternalIssue.engineId(linterKey).ruleId(issue.ruleKey);
newExternalIssue.save();
}

protected abstract void importReport(File reportPath, SensorContext context, Set<String> unresolvedInputFiles) throws IOException, ParseException;

protected abstract String linterName();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.sonar.plugins.python.coverage.PythonCoverageSensor;
import org.sonar.plugins.python.flake8.Flake8RulesDefinition;
import org.sonar.plugins.python.flake8.Flake8Sensor;
import org.sonar.plugins.python.pylint.PylintRulesDefinition;
import org.sonar.plugins.python.pylint.PylintSensor;
import org.sonar.plugins.python.warnings.DefaultAnalysisWarningsWrapper;
import org.sonar.plugins.python.xunit.PythonXUnitSensor;

Expand Down Expand Up @@ -75,6 +77,7 @@ public void define(Context context) {
context.addExtension(DefaultAnalysisWarningsWrapper.class);
addCoberturaExtensions(context);
addXUnitExtensions(context);
addPylintExtensions(context);
addBanditExtensions(context);
addFlake8Extensions(context);
}
Expand Down Expand Up @@ -148,6 +151,20 @@ private static void addBanditExtensions(Context context) {
}
}

private static void addPylintExtensions(Context context) {
context.addExtension(PylintSensor.class);
context.addExtensions(
PropertyDefinition.builder(PylintSensor.REPORT_PATH_KEY)
.name("Pylint Report Files")
.description("Paths (absolute or relative) to report files with Pylint issues.")
.category(EXTERNAL_ANALYZERS_CATEGORY)
.subCategory(PYTHON_CATEGORY)
.onQualifiers(Qualifiers.PROJECT)
.multiValues(true)
.build(),
PylintRulesDefinition.class);
}

private static void addFlake8Extensions(Context context) {
context.addExtension(Flake8Sensor.class);
context.addExtensions(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* SonarQube Python Plugin
* Copyright (C) 2011-2020 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.plugins.python;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;

/**
* Common implementation to parse Flake8 and Pylint reports
*/
public class TextReportReader {

protected static final Pattern DEFAULT_PATTERN = Pattern.compile("(.+):(\\d+):(\\d+): (\\S+[^:]):? (.*)");
protected static final Pattern LEGACY_PATTERN = Pattern.compile("(.+):(\\d+): \\[(.*)\\] (.*)");
private static final Logger LOG = Loggers.get(TextReportReader.class);
public static final int COLUMN_ZERO_BASED = 0;
public static final int COLUMN_ONE_BASED = 1;

private final int reportOffset;

public TextReportReader(int columnStartIndex) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, subjective Having to call this constructor with some magic numbers seems a little off to me. I think it would look nicer if we introduced integer constants and used them something like this:

new TextReportReader(TextReportReader.COLUMN_ZERO_BASED)
new TextReportReader(TextReportReader.COLUMN_ONE_BASED)

Feel free to disregard if you feel that this is too much.

this.reportOffset = columnStartIndex;
}

public List<Issue> parse(File report, FileSystem fileSystem) throws IOException {
List<Issue> issues = new ArrayList<>();
try (Scanner scanner = new Scanner(report.toPath(), fileSystem.encoding().name())) {
while (scanner.hasNextLine()) {
Issue issue = parseLine(scanner.nextLine());
if (issue != null) {
issues.add(issue);
}
}
}
return issues;
}

private Issue parseLine(String line) {
if (line.length() > 0) {
Matcher m = TextReportReader.DEFAULT_PATTERN.matcher(line);
if (m.matches()) {
return extractDefaultStyleIssue(m, reportOffset);
}
m = TextReportReader.LEGACY_PATTERN.matcher(line);
if (m.matches()) {
return extractLegacyStyleIssue(m);
}
LOG.debug("Cannot parse the line: {}", line);
}
return null;
}

private static Issue extractDefaultStyleIssue(Matcher m, int columnOffset) {
String filePath = m.group(1);
int lineNumber = Integer.parseInt(m.group(2));
int columnNumber = Integer.parseInt(m.group(3));
// Flake8 column numbering starts at 1
columnNumber -= columnOffset;
String ruleKey = m.group(4);
String message = m.group(5);
return new Issue(filePath, ruleKey, message, lineNumber, columnNumber);
}

private static Issue extractLegacyStyleIssue(Matcher m) {
String filePath = m.group(1);
int lineNumber = Integer.parseInt(m.group(2));
String ruleKey = m.group(3);
int lastIndex = Math.min(ruleKey.indexOf(","), ruleKey.indexOf("("));
if (lastIndex > 0) {
ruleKey = ruleKey.substring(0, lastIndex);
}
String message = m.group(4);
return new Issue(filePath, ruleKey, message, lineNumber, null);
}

public static class Issue {

public final String filePath;

public final String ruleKey;

public final String message;

public final Integer lineNumber;

public final Integer columnNumber;

public Issue(String filePath, String ruleKey, String message, Integer lineNumber, @Nullable Integer columnNumber) {
this.filePath = filePath;
this.ruleKey = ruleKey;
this.message = message;
this.lineNumber = lineNumber;
this.columnNumber = columnNumber;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,11 @@ public class BanditSensor extends ExternalIssuesSensor {
private static final Long DEFAULT_CONSTANT_DEBT_MINUTES = 5L;

@Override
protected void importReport(File reportPath, SensorContext context, Set<String> unresolvedInputFiles) {
try (InputStream in = new FileInputStream(reportPath)) {
LOG.info("Importing {}", reportPath);
boolean engineIdIsSupported = context.getSonarQubeVersion().isGreaterThanOrEqual(Version.create(7, 4));
BanditJsonReportReader.read(in, issue -> saveIssue(context, issue, unresolvedInputFiles, engineIdIsSupported));
} catch (IOException | ParseException | RuntimeException e) {
LOG.error("No issues information will be saved as the report file '{}' can't be read. " +
e.getClass().getSimpleName() + ": " + e.getMessage(), reportPath, e);
}
protected void importReport(File reportPath, SensorContext context, Set<String> unresolvedInputFiles) throws IOException, ParseException {
InputStream in = new FileInputStream(reportPath);
LOG.info("Importing {}", reportPath);
boolean engineIdIsSupported = context.getSonarQubeVersion().isGreaterThanOrEqual(Version.create(7, 4));
BanditJsonReportReader.read(in, issue -> saveIssue(context, issue, unresolvedInputFiles, engineIdIsSupported));
}

private static void saveIssue(SensorContext context, Issue issue, Set<String> unresolvedInputFiles, boolean engineIdIsSupported) {
Expand Down
Loading