From eb4857c2f631c1d40d461de33d7b679b650028bf Mon Sep 17 00:00:00 2001 From: Guillaume Dequenne Date: Thu, 9 Jul 2020 14:40:54 +0200 Subject: [PATCH] SONARPY-754 Support old reportPath property for Pylint import --- .../python/it/plugin/PylintReportTest.java | 22 +++++++--- .../plugins/python/ExternalIssuesSensor.java | 15 ++++++- .../python/flake8/Flake8SensorTest.java | 11 +++++ .../python/pylint/PylintSensorTest.java | 44 +++++++++++++++---- 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/PylintReportTest.java b/its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/PylintReportTest.java index 153294b9d1..f63cf55f2a 100644 --- a/its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/PylintReportTest.java +++ b/its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/PylintReportTest.java @@ -36,38 +36,46 @@ 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); } @@ -75,7 +83,7 @@ private static List 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"); @@ -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)); } } diff --git a/sonar-python-plugin/src/main/java/org/sonar/plugins/python/ExternalIssuesSensor.java b/sonar-python-plugin/src/main/java/org/sonar/plugins/python/ExternalIssuesSensor.java index 42f384262c..1e63439309 100644 --- a/sonar-python-plugin/src/main/java/org/sonar/plugins/python/ExternalIssuesSensor.java +++ b/sonar-python-plugin/src/main/java/org/sonar/plugins/python/ExternalIssuesSensor.java @@ -33,8 +33,10 @@ 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; @@ -42,19 +44,30 @@ 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 unresolvedInputFiles = new HashSet<>(); List 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); } diff --git a/sonar-python-plugin/src/test/java/org/sonar/plugins/python/flake8/Flake8SensorTest.java b/sonar-python-plugin/src/test/java/org/sonar/plugins/python/flake8/Flake8SensorTest.java index 801c54abf0..37a8fec5c7 100644 --- a/sonar-python-plugin/src/test/java/org/sonar/plugins/python/flake8/Flake8SensorTest.java +++ b/sonar-python-plugin/src/test/java/org/sonar/plugins/python/flake8/Flake8SensorTest.java @@ -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"); @@ -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 diff --git a/sonar-python-plugin/src/test/java/org/sonar/plugins/python/pylint/PylintSensorTest.java b/sonar-python-plugin/src/test/java/org/sonar/plugins/python/pylint/PylintSensorTest.java index 9317d4aaee..e344af11af 100644 --- a/sonar-python-plugin/src/test/java/org/sonar/plugins/python/pylint/PylintSensorTest.java +++ b/sonar-python-plugin/src/test/java/org/sonar/plugins/python/pylint/PylintSensorTest.java @@ -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"); @@ -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 externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_DEFAULT_FORMAT); + List externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_DEFAULT_FORMAT, false); assertThat(externalIssues).hasSize(10); ExternalIssue first = externalIssues.get(0); @@ -106,7 +122,7 @@ public void issues_default_format() throws IOException { @Test public void issues_no_column_info() throws IOException { - List externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_NO_COLUMN); + List externalIssues = executeSensorImporting(7, 9, PYLINT_REPORT_NO_COLUMN, false); assertThat(externalIssues).hasSize(3); ExternalIssue first = externalIssues.get(0); @@ -153,7 +169,7 @@ public void issues_no_column_info() throws IOException { @Test public void issues_other_formats() throws IOException { - List externalIssues = executeSensorImporting(7, 9, "pylint_brackets.txt"); + List externalIssues = executeSensorImporting(7, 9, "pylint_brackets.txt", false); assertThat(externalIssues).hasSize(10); ExternalIssue first = externalIssues.get(0); assertThat(first.ruleKey()).hasToString("external_pylint:E1300"); @@ -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"); @@ -178,20 +194,28 @@ public void issues_other_formats() throws IOException { @Test public void no_issues_unknown_files() throws IOException { - List externalIssues = executeSensorImporting(7, 9, "pylint_report_unknown_files.txt"); + List 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 externalIssues = executeSensorImporting(7, 9, "invalid-path.txt"); + List 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 executeSensorImporting(int majorVersion, int minorVersion, @Nullable String fileName) throws IOException { + @Test + public void import_with_legacy_property() throws IOException { + List 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 executeSensorImporting(int majorVersion, int minorVersion, @Nullable String fileName, boolean useLegacyKey) throws IOException { Path baseDir = PROJECT_DIR.getParent(); SensorContextTester context = SensorContextTester.create(baseDir); try (Stream fileStream = Files.list(PROJECT_DIR)) { @@ -199,7 +223,11 @@ private static List executeSensorImporting(int majorVersion, int 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());