Skip to content

SONARPY-898 Avoid failing on older SonarLint #1002

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
guillaume-dequenne merged 3 commits into from
Nov 15, 2021
Merged

SONARPY-898 Avoid failing on older SonarLint #1002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d5ed07
SONARPY-898 Avoid failing on older SonarLint
guillaume-dequenne Nov 12, 2021
8009276
Fix after review
guillaume-dequenne Nov 15, 2021
d179a09
Fix indentation and log
guillaume-dequenne Nov 15, 2021
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
6 changes: 3 additions & 3 deletions sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import org.sonar.plugins.python.indexer.SonarLintPythonIndexer;
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.warnings.AnalysisWarningsWrapper;
import org.sonar.plugins.python.xunit.PythonXUnitSensor;

public class PythonPlugin implements Plugin {
Expand Down Expand Up @@ -72,11 +72,11 @@ public void define(Context context) {
PythonProfile.class,

PythonSensor.class,
PythonRuleRepository.class);
PythonRuleRepository.class,
AnalysisWarningsWrapper.class);

SonarRuntime sonarRuntime = context.getRuntime();
if (sonarRuntime.getProduct() != SonarProduct.SONARLINT) {
context.addExtension(DefaultAnalysisWarningsWrapper.class);
addCoberturaExtensions(context);
addXUnitExtensions(context);
addPylintExtensions(context);
Expand Down
2 changes: 1 addition & 1 deletion sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonReportSensor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void execute(SensorContext context) {
processReports(context, reports);
} catch (Exception e) {
LOG.warn("Cannot read report '{}', the following exception occurred: {}", reportPath, e.getMessage());
analysisWarnings.addWarning(String.format("An error occurred while trying to import %s report(s): '%s'", reportType, reportPath));
analysisWarnings.addUnique(String.format("An error occurred while trying to import %s report(s): '%s'", reportType, reportPath));
}
}

Expand Down
22 changes: 10 additions & 12 deletions sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonSensor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.issue.NoSonarFilter;
import org.sonar.api.measures.FileLinesContextFactory;
import org.sonar.api.notifications.AnalysisWarnings;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.plugins.python.api.ProjectPythonVersion;
Expand All @@ -49,6 +48,7 @@
import org.sonar.plugins.python.api.tree.FileInput;
import org.sonar.plugins.python.indexer.PythonIndexer;
import org.sonar.plugins.python.indexer.SonarQubePythonIndexer;
import org.sonar.plugins.python.warnings.AnalysisWarningsWrapper;
import org.sonar.python.checks.CheckList;
import org.sonar.python.parser.PythonParser;
import org.sonar.python.semantic.ProjectLevelSymbolTable;
Expand All @@ -67,8 +67,7 @@ public final class PythonSensor implements Sensor {
private final FileLinesContextFactory fileLinesContextFactory;
private final NoSonarFilter noSonarFilter;
private final PythonIndexer indexer;
@Nullable
private final AnalysisWarnings analysisWarnings;
private final AnalysisWarningsWrapper analysisWarnings;
private static final Logger LOG = Loggers.get(PythonSensor.class);
static final String UNSET_VERSION_WARNING =
"Your code is analyzed as compatible with python 2 and 3 by default. This will prevent the detection of issues specific to python 2 or python 3." +
Expand All @@ -77,22 +76,23 @@ public final class PythonSensor implements Sensor {
/**
* Constructor to be used by pico if neither PythonCustomRuleRepository nor PythonIndexer are to be found and injected.
*/
public PythonSensor(FileLinesContextFactory fileLinesContextFactory, CheckFactory checkFactory, NoSonarFilter noSonarFilter, AnalysisWarnings analysisWarnings) {
public PythonSensor(FileLinesContextFactory fileLinesContextFactory, CheckFactory checkFactory,
NoSonarFilter noSonarFilter, AnalysisWarningsWrapper analysisWarnings) {
this(fileLinesContextFactory, checkFactory, noSonarFilter, null, null, analysisWarnings);
}

public PythonSensor(FileLinesContextFactory fileLinesContextFactory, CheckFactory checkFactory, NoSonarFilter noSonarFilter,

@andrea-guarino-sonarsource andrea-guarino-sonarsource Nov 15, 2021

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.

In recent version of SonarLint, my understanding is that PythonIndexer will be injected, while PythonCustomRuleRepository won't be found. So what constructor will be called in that case?

@guillaume-dequenne guillaume-dequenne Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Indeed, due to the missing constructor, PythonIndexer ended up not being injected properly...

PythonIndexer indexer) {
this(fileLinesContextFactory, checkFactory, noSonarFilter, null, indexer, null);
PythonCustomRuleRepository[] customRuleRepositories, AnalysisWarningsWrapper analysisWarnings) {
this(fileLinesContextFactory, checkFactory, noSonarFilter, customRuleRepositories, null, analysisWarnings);
}

public PythonSensor(FileLinesContextFactory fileLinesContextFactory, CheckFactory checkFactory, NoSonarFilter noSonarFilter,
PythonCustomRuleRepository[] customRuleRepositories, AnalysisWarnings analysisWarnings) {
this(fileLinesContextFactory, checkFactory, noSonarFilter, customRuleRepositories, null, analysisWarnings);
PythonIndexer indexer, AnalysisWarningsWrapper analysisWarnings) {
this(fileLinesContextFactory, checkFactory, noSonarFilter, null, indexer, analysisWarnings);
}

public PythonSensor(FileLinesContextFactory fileLinesContextFactory, CheckFactory checkFactory, NoSonarFilter noSonarFilter,
@Nullable PythonCustomRuleRepository[] customRuleRepositories, @Nullable PythonIndexer indexer, @Nullable AnalysisWarnings analysisWarnings) {
@Nullable PythonCustomRuleRepository[] customRuleRepositories, @Nullable PythonIndexer indexer, AnalysisWarningsWrapper analysisWarnings) {
this.checks = new PythonChecks(checkFactory)
.addChecks(CheckList.REPOSITORY_KEY, CheckList.getChecks())
.addCustomChecks(customRuleRepositories);
Expand All @@ -118,9 +118,7 @@ public void execute(SensorContext context) {
Optional<String> pythonVersionParameter = context.config().get(PYTHON_VERSION_KEY);
if (!pythonVersionParameter.isPresent() && context.runtime().getProduct() != SonarProduct.SONARLINT) {
LOG.warn(UNSET_VERSION_WARNING);
if (analysisWarnings != null) {
analysisWarnings.addUnique(UNSET_VERSION_WARNING);
}
analysisWarnings.addUnique(UNSET_VERSION_WARNING);
}
pythonVersionParameter.ifPresent(value -> ProjectPythonVersion.setCurrentVersions(PythonVersionUtils.fromString(value)));
PythonIndexer pythonIndexer = this.indexer != null ? this.indexer : new SonarQubePythonIndexer(mainFiles);
Expand Down
2 changes: 1 addition & 1 deletion ...r-python-plugin/src/main/java/org/sonar/plugins/python/coverage/PythonCoverageSensor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private void warnDeprecatedPropertyUsage(Configuration config) {
if (config.hasKey(REPORT_PATH_KEY)) {
String msg = "Property 'sonar.python.coverage.reportPath' has been removed. Please use 'sonar.python.coverage.reportPaths' instead.";
LOG.warn(msg);
analysisWarnings.addWarning(msg);
analysisWarnings.addUnique(msg);
}
}

Expand Down
26 changes: 21 additions & 5 deletions ...ython-plugin/src/main/java/org/sonar/plugins/python/warnings/AnalysisWarningsWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
*/
package org.sonar.plugins.python.warnings;

import org.sonar.api.batch.InstantiationStrategy;
import org.sonar.api.batch.ScannerSide;
import javax.annotation.Nullable;
import org.sonar.api.notifications.AnalysisWarnings;
import org.sonar.api.scanner.ScannerSide;
import org.sonarsource.api.sonarlint.SonarLintSide;

/**
* As {@link org.sonar.api.notifications.AnalysisWarnings} has been added in SQ 7.4, previous version of the API
Expand All @@ -29,7 +31,21 @@
* warnings to the underlying {@link org.sonar.api.notifications.AnalysisWarnings} or do nothing when not available.
*/
@ScannerSide
@InstantiationStrategy("PER_BATCH")
public interface AnalysisWarningsWrapper {
void addWarning(String text);
@SonarLintSide(lifespan = SonarLintSide.MULTIPLE_ANALYSES)
public class AnalysisWarningsWrapper {
private final AnalysisWarnings analysisWarnings;

public AnalysisWarningsWrapper(@Nullable AnalysisWarnings analysisWarnings) {
this.analysisWarnings = analysisWarnings;
}

public AnalysisWarningsWrapper() {
this.analysisWarnings = null;
}

public void addUnique(String text) {
if (analysisWarnings != null) {
this.analysisWarnings.addUnique(text);
}
}
}
36 changes: 0 additions & 36 deletions ...lugin/src/main/java/org/sonar/plugins/python/warnings/DefaultAnalysisWarningsWrapper.java
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions sonar-python-plugin/src/test/java/org/sonar/plugins/python/PythonPluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import org.sonar.api.utils.Version;
import org.sonar.api.utils.log.LogTester;
import org.sonar.api.utils.log.LoggerLevel;
import org.sonar.plugins.python.warnings.DefaultAnalysisWarningsWrapper;
import org.sonar.plugins.python.warnings.AnalysisWarningsWrapper;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
Expand All @@ -45,8 +45,8 @@ public void testGetExtensions() {
Version v79 = Version.create(7, 9);
SonarRuntime runtime = SonarRuntimeImpl.forSonarQube(v79, SonarQubeSide.SERVER, SonarEdition.DEVELOPER);
assertThat(extensions(runtime)).hasSize(21);
assertThat(extensions(runtime)).contains(DefaultAnalysisWarningsWrapper.class);
assertThat(extensions(SonarRuntimeImpl.forSonarLint(v79))).hasSize(6);
assertThat(extensions(runtime)).contains(AnalysisWarningsWrapper.class);
assertThat(extensions(SonarRuntimeImpl.forSonarLint(v79))).hasSize(7);
}

@Test
Expand Down
20 changes: 10 additions & 10 deletions sonar-python-plugin/src/test/java/org/sonar/plugins/python/PythonSensorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import org.sonar.api.measures.CoreMetrics;
import org.sonar.api.measures.FileLinesContext;
import org.sonar.api.measures.FileLinesContextFactory;
import org.sonar.api.notifications.AnalysisWarnings;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.utils.Version;
import org.sonar.api.utils.log.LogTester;
Expand All @@ -73,6 +72,7 @@
import org.sonar.plugins.python.indexer.PythonIndexer;
import org.sonar.plugins.python.indexer.SonarLintPythonIndexer;
import org.sonar.plugins.python.indexer.TestModuleFileSystem;
import org.sonar.plugins.python.warnings.AnalysisWarningsWrapper;
import org.sonar.python.checks.CheckList;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -131,7 +131,7 @@ public void scanFile(PythonVisitorContext visitorContext) {

private ActiveRules activeRules;

private final AnalysisWarnings analysisWarning = mock(AnalysisWarnings.class);
private final AnalysisWarningsWrapper analysisWarning = mock(AnalysisWarningsWrapper.class);

@org.junit.Rule
public LogTester logTester = new LogTester();
Expand Down Expand Up @@ -319,7 +319,7 @@ public void cross_files_issues_only_one_file_analyzed() {
// "mod.py" created but not added to context
InputFile modFile = createInputFile("mod.py");
PythonIndexer pythonIndexer = pythonIndexer(Arrays.asList(mainFile, modFile));
sensor(CUSTOM_RULES, pythonIndexer, null).execute(context);
sensor(null, pythonIndexer, analysisWarning).execute(context);

@andrea-guarino-sonarsource andrea-guarino-sonarsource Nov 15, 2021

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.

why did you change CUSTOM_RULES -> null ?

@guillaume-dequenne guillaume-dequenne Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wanted to have a unit test with a PythonIndexer and without a PythonCustomRuleRepository to reflect the new constructor.

assertThat(context.allIssues()).hasSize(1);
Issue issue = context.allIssues().iterator().next();
assertThat(issue.primaryLocation().inputComponent()).isEqualTo(mainFile);
Expand All @@ -341,7 +341,7 @@ public void no_indexer_when_project_too_large_sonarlint() {

InputFile mainFile = inputFile("main.py");
PythonIndexer pythonIndexer = pythonIndexer(Collections.singletonList(mainFile));
sensor(CUSTOM_RULES, pythonIndexer, null).execute(context);
sensor(CUSTOM_RULES, pythonIndexer, analysisWarning).execute(context);
assertThat(context.allIssues()).isEmpty();
assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("Project symbol table deactivated due to project size (total number of lines is 4, maximum for indexing is 1)");
assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("Update \"sonar.python.sonarlint.indexing.maxlines\" to set a different limit.");
Expand All @@ -358,7 +358,7 @@ public void loop_in_class_hierarchy() {
InputFile mainFile = inputFile("modA.py");
InputFile modFile = inputFile("modB.py");
PythonIndexer pythonIndexer = pythonIndexer(Arrays.asList(mainFile, modFile));
sensor(null, pythonIndexer, null).execute(context);
sensor(null, pythonIndexer, analysisWarning).execute(context);

assertThat(context.allIssues()).hasSize(1);
}
Expand Down Expand Up @@ -494,7 +494,7 @@ public void cancelled_analysis() {
InputFile inputFile = inputFile(FILE_1);
activeRules = (new ActiveRulesBuilder()).build();
context.setCancelled(true);
sensor(null, null, null).execute(context);
sensor(null, null, analysisWarning).execute(context);
assertThat(context.measure(inputFile.key(), CoreMetrics.NCLOC)).isNull();
assertThat(context.allAnalysisErrors()).isEmpty();
}
Expand Down Expand Up @@ -562,20 +562,20 @@ private PythonSensor sensor() {
return sensor(CUSTOM_RULES, null, analysisWarning);
}

private PythonSensor sensor(@Nullable PythonCustomRuleRepository[] customRuleRepositories, @Nullable PythonIndexer indexer, @Nullable AnalysisWarnings analysisWarnings) {
private PythonSensor sensor(@Nullable PythonCustomRuleRepository[] customRuleRepositories, @Nullable PythonIndexer indexer, AnalysisWarningsWrapper analysisWarnings) {
FileLinesContextFactory fileLinesContextFactory = mock(FileLinesContextFactory.class);
FileLinesContext fileLinesContext = mock(FileLinesContext.class);
when(fileLinesContextFactory.createFor(Mockito.any(InputFile.class))).thenReturn(fileLinesContext);
CheckFactory checkFactory = new CheckFactory(activeRules);
if (indexer == null && customRuleRepositories == null) {
return new PythonSensor(fileLinesContextFactory, checkFactory, mock(NoSonarFilter.class), analysisWarnings);
}
if (indexer != null && customRuleRepositories == null) {
return new PythonSensor(fileLinesContextFactory, checkFactory, mock(NoSonarFilter.class), indexer);
}
if (indexer == null) {
return new PythonSensor(fileLinesContextFactory, checkFactory, mock(NoSonarFilter.class), customRuleRepositories, analysisWarnings);
}
if (customRuleRepositories == null) {
return new PythonSensor(fileLinesContextFactory, checkFactory, mock(NoSonarFilter.class), indexer, analysisWarnings);
}
return new PythonSensor(fileLinesContextFactory, checkFactory, mock(NoSonarFilter.class), customRuleRepositories, indexer, analysisWarnings);
}

Expand Down
2 changes: 1 addition & 1 deletion ...thon-plugin/src/test/java/org/sonar/plugins/python/coverage/PythonCoverageSensorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public void test_unique_report() {
List<Integer> actual = IntStream.range(1, 18).mapToObj(line -> context.lineHits(FILE4_KEY, line)).collect(Collectors.toList());
Integer coverageAtLine6 = actual.get(5);
assertThat(coverageAtLine6).isEqualTo(1);
verify(analysisWarnings, times(1)).addWarning(eq("Property 'sonar.python.coverage.reportPath' has been removed. Please use 'sonar.python.coverage.reportPaths' instead."));
verify(analysisWarnings, times(1)).addUnique(eq("Property 'sonar.python.coverage.reportPath' has been removed. Please use 'sonar.python.coverage.reportPaths' instead."));
}

@Test
Expand Down
6 changes: 3 additions & 3 deletions ...n-plugin/src/test/java/org/sonar/plugins/python/warnings/AnalysisWarningsWrapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ public class AnalysisWarningsWrapperTest {
@Test
public void test() {
AnalysisWarnings analysisWarnings = spy(AnalysisWarnings.class);
DefaultAnalysisWarningsWrapper defaultAnalysisWarningsWrapper = new DefaultAnalysisWarningsWrapper(analysisWarnings);
defaultAnalysisWarningsWrapper.addWarning("abcd");
defaultAnalysisWarningsWrapper.addWarning("def");
AnalysisWarningsWrapper defaultAnalysisWarningsWrapper = new AnalysisWarningsWrapper(analysisWarnings);
defaultAnalysisWarningsWrapper.addUnique("abcd");
defaultAnalysisWarningsWrapper.addUnique("def");
verify(analysisWarnings, times(2)).addUnique(anyString());
}

Expand Down
2 changes: 1 addition & 1 deletion sonar-python-plugin/src/test/java/org/sonar/plugins/python/xunit/PythonXUnitSensorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void shouldLogWarningWhenGivenInvalidTime() {
assertThat(logTester.logs(LoggerLevel.WARN)).contains("Cannot read report 'xunit-reports/invalid-time-xunit-report.xml', " +
"the following exception occurred: java.text.ParseException: Unparseable number: \"brrrr\"");
verify(analysisWarnings, times(1))
.addWarning(eq("An error occurred while trying to import XUnit report(s): 'xunit-reports/invalid-time-xunit-report.xml'"));
.addUnique(eq("An error occurred while trying to import XUnit report(s): 'xunit-reports/invalid-time-xunit-report.xml'"));
}

@Test
Expand Down