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
2 changes: 1 addition & 1 deletion its/plugin/it-python-plugin-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<artifactId>it-python-plugin</artifactId>
<groupId>org.sonarsource.python</groupId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>it-python-plugin-test</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,51 +36,63 @@
public class PylintReportTest {

private static final String PROJECT = "pylint_project";
private static final String DEFAULT_PROPERTY = "sonar.python.pylint.reportPaths";
private static final String LEGACY_PROPERTY = "sonar.python.pylint.reportPath";

@ClassRule
public static final Orchestrator ORCHESTRATOR = Tests.ORCHESTRATOR;

@Test
public void import_report() {
analyseProjectWithReport("pylint-report.txt");
analyseProjectWithReport(DEFAULT_PROPERTY, "pylint-report.txt");
assertThat(issues()).hasSize(4);
}

@Test
public void import_report_legacy_key() {
analyseProjectWithReport(LEGACY_PROPERTY, "pylint-report.txt");
assertThat(issues()).hasSize(4);
}

@Test
public void missing_report() {
analyseProjectWithReport("missing");
assertThat(issues()).hasSize(0);
analyseProjectWithReport(DEFAULT_PROPERTY, "missing");
assertThat(issues()).isEmpty();
}

@Test
public void invalid_report() {
BuildResult result = analyseProjectWithReport("invalid.txt");
BuildResult result = analyseProjectWithReport(DEFAULT_PROPERTY, "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);
BuildResult result = analyseProjectWithReport(DEFAULT_PROPERTY, "rule-unknown.txt");
assertThat(issues()).hasSize(4);
}

@Test
public void multiple_reports() {
analyseProjectWithReport(DEFAULT_PROPERTY, "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 property, 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(property, reportPaths));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@
@RunWith(Suite.class)
@Suite.SuiteClasses({
BanditReportTest.class,
PylintReportTest.class,
MetricsTest.class,
CPDTest.class,
CoverageTest.class,
PylintReportTest.class,
TestReportTest.class,
NoSonarTest.class,
SonarLintTest.class
Expand Down
2 changes: 1 addition & 1 deletion its/plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>python-its</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<modules>
Expand Down
2 changes: 1 addition & 1 deletion its/plugin/python-custom-rules-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>it-python-plugin</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>python-custom-rules-plugin</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion its/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>python</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>python-its</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion its/ruling/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>python-its</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>it-python-ruling</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

<groupId>org.sonarsource.python</groupId>
<artifactId>python</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Python</name>
Expand Down
2 changes: 1 addition & 1 deletion python-checks-testkit/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<parent>
<artifactId>python</artifactId>
<groupId>org.sonarsource.python</groupId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>python-checks-testkit</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion python-checks/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>python</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>python-checks</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion python-frontend/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>python</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>python-frontend</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion sonar-python-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.sonarsource.python</groupId>
<artifactId>python</artifactId>
<version>2.14-SNAPSHOT</version>
<version>3.0-SNAPSHOT</version>
</parent>

<artifactId>sonar-python-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,35 @@


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.config.Configuration;
import org.sonar.api.rules.RuleType;
import org.sonar.api.utils.log.Logger;
import org.sonar.plugins.python.pylint.PylintSensor;
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;
protected static final String PYLINT_LEGACY_KEY = "sonar.python.pylint.reportPath";

@Override
public void describe(SensorDescriptor descriptor) {
descriptor
.onlyWhenConfiguration(conf -> conf.hasKey(reportPathKey()))
.onlyWhenConfiguration(this::shouldExecute)
.onlyOnLanguage(Python.KEY)
.name("Import of " + linterName() + " issues");
}
Expand All @@ -47,10 +58,22 @@ 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));
if (reportFiles.isEmpty() && context.config().hasKey(PYLINT_LEGACY_KEY)) {
reportFiles = ExternalReportProvider.getReportFiles(context, PYLINT_LEGACY_KEY);
logger().warn("The use of '{}' is deprecated. Please use the '{}' property instead.", PYLINT_LEGACY_KEY, PylintSensor.REPORT_PATH_KEY);
}
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 +85,42 @@ 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 boolean shouldExecute(Configuration conf);

protected abstract String linterName();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@
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.PylintConfiguration;
import org.sonar.plugins.python.pylint.PylintImportSensor;
import org.sonar.plugins.python.pylint.PylintRuleRepository;
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 @@ -136,41 +134,6 @@ private static void addXUnitExtensions(Context context) {
PythonXUnitSensor.class);
}

private static void addPylintExtensions(Context context) {
context.addExtensions(
PropertyDefinition.builder(PylintConfiguration.PYLINT_CONFIG_KEY)
.index(30)
.name("Pylint configuration")
.description("Path to the pylint configuration file to use in pylint analysis. Set to empty to use the default.")
.category(PYTHON_CATEGORY)
.subCategory(PYLINT)
.onQualifiers(Qualifiers.PROJECT)
.defaultValue("")
.build(),
PropertyDefinition.builder(PylintConfiguration.PYLINT_KEY)
.index(31)
.name("Pylint executable")
.description("Path to the pylint executable to use in pylint analysis. Set to empty to use the default one.")
.category(PYTHON_CATEGORY)
.subCategory(PYLINT)
.onQualifiers(Qualifiers.PROJECT)
.defaultValue("")
.build(),
PropertyDefinition.builder(PylintImportSensor.REPORT_PATH_KEY)
.index(32)
.name("Pylint's reports")
.description("Path to Pylint's report file, relative to projects root")
.category(PYTHON_CATEGORY)
.subCategory(PYLINT)
.onQualifiers(Qualifiers.PROJECT)
.defaultValue("")
.build(),
PylintConfiguration.class,
PylintSensor.class,
PylintImportSensor.class,
PylintRuleRepository.class);
}

private static void addBanditExtensions(Context context) {
context.addExtension(BanditSensor.class);
boolean externalIssuesSupported = context.getSonarQubeVersion().isGreaterThanOrEqual(Version.create(7, 2));
Expand All @@ -188,9 +151,21 @@ private static void addBanditExtensions(Context context) {
}
}

private static void addPylintExtensions(Context context) {
context.addExtensions(PylintSensor.class,
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);
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.

Very minor: it's weird to have PylintRulesDefinition here and PylintSensor in a separate call to context.addExtension.

}

private static void addFlake8Extensions(Context context) {
context.addExtension(Flake8Sensor.class);
context.addExtensions(
context.addExtensions(Flake8Sensor.class,
PropertyDefinition.builder(Flake8Sensor.REPORT_PATH_KEY)
.name("Flake8 Report Files")
.description("Paths (absolute or relative) to report files with Flake8 issues.")
Expand Down
Loading