Skip to content

SONARPY-754 Support old reportPath property for Pylint import #809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gyula-sallai-sonarsource merged 1 commit into from
Jul 9, 2020
Merged

SONARPY-754 Support old reportPath property for Pylint import #809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 15 additions & 7 deletions ...ugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/PylintReportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,46 +36,54 @@
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");
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()).isEmpty();
}

@Test
public void unknown_rule() {
BuildResult result = analyseProjectWithReport("rule-unknown.txt");
BuildResult result = analyseProjectWithReport(DEFAULT_PROPERTY, "rule-unknown.txt");
assertThat(issues()).hasSize(4);
}

@Test
public void multiple_reports() {
analyseProjectWithReport("pylint-report.txt, rule-unknown.txt");
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 reportPaths) {
private static BuildResult analyseProjectWithReport(String property, String reportPaths) {
ORCHESTRATOR.resetData();
ORCHESTRATOR.getServer().provisionProject(PROJECT, PROJECT);
ORCHESTRATOR.getServer().associateProjectToQualityProfile(PROJECT, "py", "no_rule");
Expand All @@ -84,7 +92,7 @@ private static BuildResult analyseProjectWithReport(String reportPaths) {
SonarScanner.create()
.setDebugLogs(true)
.setProjectDir(new File("projects/pylint_project"))
.setProperty("sonar.python.pylint.reportPaths", reportPaths));
.setProperty(property, reportPaths));
}

}
15 changes: 14 additions & 1 deletion sonar-python-plugin/src/main/java/org/sonar/plugins/python/ExternalIssuesSensor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,41 @@
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;
private static final String PYLINT_LEGACY_KEY = "sonar.python.pylint.reportPath";

@Override
public void describe(SensorDescriptor descriptor) {
descriptor
.onlyWhenConfiguration(conf -> conf.hasKey(reportPathKey()))
.onlyWhenConfiguration(conf -> conf.hasKey(reportPathKey()) || hasPylintLegacyConfig(conf))
.onlyOnLanguage(Python.KEY)
.name("Import of " + linterName() + " issues");
}

private boolean hasPylintLegacyConfig(Configuration conf) {
// Only for PylintSensor
return reportPathKey().equals(PylintSensor.REPORT_PATH_KEY) &&
conf.hasKey(PYLINT_LEGACY_KEY);
}

@Override
public void execute(SensorContext context) {
Set<String> unresolvedInputFiles = new HashSet<>();
List<File> reportFiles = ExternalReportProvider.getReportFiles(context, reportPathKey());
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);
}
Expand Down
11 changes: 11 additions & 0 deletions sonar-python-plugin/src/test/java/org/sonar/plugins/python/flake8/Flake8SensorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public class Flake8SensorTest {
private static final String FLAKE8_FILE = "python-project:flake8/file1.py";
private static final String FLAKE8_F401 = "external_flake8:F401";
private static final String FLAKE_8_REPORT = "flake8-report.txt";
private static final String FLAKE_8_PROPERTY = "sonar.python.flake8.reportPaths";
private static final String FLAKE_8_REPORT_UNKNOWN_FILES = "flake8-report-unknown-files.txt";

private static final Path PROJECT_DIR = Paths.get("src", "test", "resources", "org", "sonar", "plugins", "python", "flake8");
Expand All @@ -70,6 +71,16 @@ public void test_descriptor() {
assertThat(sensorDescriptor.languages()).containsOnly("py");
assertThat(sensorDescriptor.configurationPredicate()).isNotNull();
assertNoErrorWarnDebugLogs(logTester);

Path baseDir = PROJECT_DIR.getParent();
SensorContextTester context = SensorContextTester.create(baseDir);
context.settings().setProperty(FLAKE_8_PROPERTY, "path/to/report");
assertThat(sensorDescriptor.configurationPredicate().test(context.config())).isTrue();

context = SensorContextTester.create(baseDir);
context.settings().setProperty("sonar.python.flake8.reportPath", "path/to/report");
// No support of "reportPath" property for Flake8
assertThat(sensorDescriptor.configurationPredicate().test(context.config())).isFalse();
}

@Test
Expand Down
44 changes: 36 additions & 8 deletions sonar-python-plugin/src/test/java/org/sonar/plugins/python/pylint/PylintSensorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class PylintSensorTest {
private static final String PYLINT_FILE = "python-project:pylint/file1.py";
private static final String PYLINT_REPORT_DEFAULT_FORMAT = "pylint_report_default_format.txt";
private static final String PYLINT_REPORT_NO_COLUMN = "pylint_report_no_column.txt";
private static final String DEFAULT_PROPERTY = "sonar.python.pylint.reportPaths";
private static final String LEGACY_PROPERTY = "sonar.python.pylint.reportPath";

private static final Path PROJECT_DIR = Paths.get("src", "test", "resources", "org", "sonar", "plugins", "python", "pylint");

Expand All @@ -69,11 +71,25 @@ public void test_descriptor() {
assertThat(sensorDescriptor.languages()).containsOnly("py");
assertThat(sensorDescriptor.configurationPredicate()).isNotNull();
assertNoErrorWarnLogs(logTester);

Path baseDir = PROJECT_DIR.getParent();
SensorContextTester context = SensorContextTester.create(baseDir);
context.settings().setProperty(DEFAULT_PROPERTY, "path/to/report");
assertThat(sensorDescriptor.configurationPredicate().test(context.config())).isTrue();

context = SensorContextTester.create(baseDir);
context.settings().setProperty(LEGACY_PROPERTY, "path/to/report");
// Support of legacy "reportPath" property for Pylint
assertThat(sensorDescriptor.configurationPredicate().test(context.config())).isTrue();

context = SensorContextTester.create(baseDir);
context.settings().setProperty("random.key", "path/to/report");
assertThat(sensorDescriptor.configurationPredicate().test(context.config())).isFalse();
}

@Test
public void issues_default_format() throws IOException {
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_DEFAULT_FORMAT);
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_DEFAULT_FORMAT, false);
assertThat(externalIssues).hasSize(10);

ExternalIssue first = externalIssues.get(0);
Expand Down Expand Up @@ -106,7 +122,7 @@ public void issues_default_format() throws IOException {

@Test
public void issues_no_column_info() throws IOException {
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_NO_COLUMN);
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_NO_COLUMN, false);
assertThat(externalIssues).hasSize(3);

ExternalIssue first = externalIssues.get(0);
Expand Down Expand Up @@ -153,7 +169,7 @@ public void issues_no_column_info() throws IOException {

@Test
public void issues_other_formats() throws IOException {
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "pylint_brackets.txt");
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "pylint_brackets.txt", false);
assertThat(externalIssues).hasSize(10);
ExternalIssue first = externalIssues.get(0);
assertThat(first.ruleKey()).hasToString("external_pylint:E1300");
Expand All @@ -164,7 +180,7 @@ public void issues_other_formats() throws IOException {
assertThat(firstPrimaryLoc.message())
.isEqualTo("message");

externalIssues = executeSensorImporting(7, 9, "pylint_names_in_brackets.txt");
externalIssues = executeSensorImporting(7, 9, "pylint_names_in_brackets.txt", false);
assertThat(externalIssues).hasSize(1);
first = externalIssues.get(0);
assertThat(first.ruleKey()).hasToString("external_pylint:C0111");
Expand All @@ -178,28 +194,40 @@ public void issues_other_formats() throws IOException {

@Test
public void no_issues_unknown_files() throws IOException {
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "pylint_report_unknown_files.txt");
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "pylint_report_unknown_files.txt", false);
assertThat(externalIssues).isEmpty();
}

@Test
public void no_issues_with_invalid_report_path() throws IOException {
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "invalid-path.txt");
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "invalid-path.txt", false);
assertThat(externalIssues).isEmpty();
assertThat(onlyOneLogElement(logTester.logs(LoggerLevel.ERROR)))
.startsWith("No issues information will be saved as the report file '")
.contains("invalid-path.txt' can't be read.");
}

private static List<ExternalIssue> executeSensorImporting(int majorVersion, int minorVersion, @Nullable String fileName) throws IOException {
@Test
public void import_with_legacy_property() throws IOException {
List<ExternalIssue> externalIssues = executeSensorImporting(7, 9, "pylint_brackets.txt", true);
assertThat(externalIssues).hasSize(10);
assertThat(onlyOneLogElement(logTester.logs())).hasToString("The use of 'sonar.python.pylint.reportPath' is deprecated." +
" Please use the 'sonar.python.pylint.reportPaths' property instead.");
}

private static List<ExternalIssue> executeSensorImporting(int majorVersion, int minorVersion, @Nullable String fileName, boolean useLegacyKey) throws IOException {
Path baseDir = PROJECT_DIR.getParent();
SensorContextTester context = SensorContextTester.create(baseDir);
try (Stream<Path> fileStream = Files.list(PROJECT_DIR)) {
fileStream.forEach(file -> addFileToContext(context, baseDir, file));
context.setRuntime(SonarRuntimeImpl.forSonarQube(Version.create(majorVersion, minorVersion), SonarQubeSide.SERVER, SonarEdition.DEVELOPER));
if (fileName != null) {
String path = PROJECT_DIR.resolve(fileName).toAbsolutePath().toString();
context.settings().setProperty("sonar.python.pylint.reportPaths", path);
if (useLegacyKey) {
context.settings().setProperty(LEGACY_PROPERTY, path);
} else {
context.settings().setProperty(DEFAULT_PROPERTY, path);
}
}
pylintSensor.execute(context);
return new ArrayList<>(context.allExternalIssues());
Expand Down