Skip to content

Commit 7937219

Browse files
maksim-grebeniuk-sonarsourcesonartech
authored andcommitted
SONARPY-2872 Introduce IssuesRepository responsible to save issues in a thread-safe way (#236)
GitOrigin-RevId: f8dc45fe7cdbb830ab2462c635f6dd3113a35b85
1 parent b6c4d38 commit 7937219

2 files changed

Lines changed: 170 additions & 119 deletions

File tree

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.plugins.python;
18+
19+
import java.io.File;
20+
import java.util.ArrayDeque;
21+
import java.util.List;
22+
import java.util.Optional;
23+
import javax.annotation.CheckForNull;
24+
import org.slf4j.Logger;
25+
import org.slf4j.LoggerFactory;
26+
import org.sonar.api.batch.fs.InputFile;
27+
import org.sonar.api.batch.fs.TextRange;
28+
import org.sonar.api.batch.sensor.SensorContext;
29+
import org.sonar.api.batch.sensor.issue.NewIssue;
30+
import org.sonar.api.batch.sensor.issue.NewIssueLocation;
31+
import org.sonar.api.rule.RuleKey;
32+
import org.sonar.plugins.python.api.IssueLocation;
33+
import org.sonar.plugins.python.api.PythonCheck;
34+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
35+
import org.sonar.plugins.python.api.quickfix.PythonTextEdit;
36+
import org.sonar.plugins.python.indexer.PythonIndexer;
37+
38+
public class IssuesRepository {
39+
private static final Logger LOG = LoggerFactory.getLogger(IssuesRepository.class);
40+
private final SensorContext context;
41+
private final PythonChecks checks;
42+
private final PythonIndexer indexer;
43+
private final boolean isInSonarLint;
44+
private final Object monitor;
45+
46+
public IssuesRepository(SensorContext context, PythonChecks checks, PythonIndexer indexer, boolean isInSonarLint, Object monitor) {
47+
this.context = context;
48+
this.checks = checks;
49+
this.indexer = indexer;
50+
this.isInSonarLint = isInSonarLint;
51+
this.monitor = monitor;
52+
}
53+
54+
public void save(PythonInputFile inputFile, List<PythonCheck.PreciseIssue> issues) {
55+
issues.forEach(preciseIssue -> save(inputFile, preciseIssue));
56+
}
57+
58+
private void save(PythonInputFile inputFile, PythonCheck.PreciseIssue preciseIssue) {
59+
var ruleKey = checks.ruleKey(preciseIssue.check());
60+
var newIssue = context
61+
.newIssue()
62+
.forRule(ruleKey);
63+
64+
var cost = preciseIssue.cost();
65+
if (cost != null) {
66+
newIssue.gap(cost.doubleValue());
67+
}
68+
69+
var primaryLocation = newLocation(inputFile, newIssue, preciseIssue.primaryLocation());
70+
newIssue.at(primaryLocation);
71+
72+
var secondaryLocationsFlow = new ArrayDeque<NewIssueLocation>();
73+
for (var secondaryLocation : preciseIssue.secondaryLocations()) {
74+
String fileId = secondaryLocation.fileId();
75+
if (fileId != null) {
76+
InputFile issueLocationFile = component(fileId, context);
77+
if (issueLocationFile != null) {
78+
secondaryLocationsFlow.addFirst(newLocation(new PythonInputFileImpl(issueLocationFile), newIssue, secondaryLocation));
79+
}
80+
} else {
81+
newIssue.addLocation(newLocation(inputFile, newIssue, secondaryLocation));
82+
}
83+
}
84+
85+
// secondary locations on multiple files are only supported using flows
86+
if (!secondaryLocationsFlow.isEmpty()) {
87+
secondaryLocationsFlow.addFirst(primaryLocation);
88+
newIssue.addFlow(secondaryLocationsFlow);
89+
}
90+
91+
handleQuickFixes(inputFile.wrappedFile(), ruleKey, newIssue, preciseIssue);
92+
93+
save(newIssue);
94+
}
95+
96+
private void save(NewIssue newIssue) {
97+
synchronized (monitor) {
98+
newIssue.save();
99+
}
100+
}
101+
102+
@CheckForNull
103+
private InputFile component(String fileId, SensorContext sensorContext) {
104+
var predicate = sensorContext.fileSystem().predicates().is(new File(fileId));
105+
var inputFile = Optional.ofNullable(sensorContext.fileSystem().inputFile(predicate))
106+
.orElseGet(() -> indexer.getFileWithId(fileId));
107+
if (inputFile == null) {
108+
LOG.debug("Failed to find InputFile for {}", fileId);
109+
}
110+
return inputFile;
111+
}
112+
113+
private static NewIssueLocation newLocation(PythonInputFile inputFile, NewIssue issue, IssueLocation location) {
114+
var newLocation = issue.newLocation().on(inputFile.wrappedFile());
115+
116+
if (location.startLine() != IssueLocation.UNDEFINED_LINE) {
117+
TextRange range;
118+
if (location.startLineOffset() == IssueLocation.UNDEFINED_OFFSET) {
119+
range = inputFile.wrappedFile().selectLine(location.startLine());
120+
} else {
121+
range = inputFile.wrappedFile().newRange(location.startLine(), location.startLineOffset(), location.endLine(),
122+
location.endLineOffset());
123+
}
124+
newLocation.at(range);
125+
}
126+
127+
var message = location.message();
128+
if (message != null) {
129+
newLocation.message(message);
130+
}
131+
return newLocation;
132+
}
133+
134+
private void handleQuickFixes(InputFile inputFile, RuleKey ruleKey, NewIssue newIssue, PythonCheck.PreciseIssue preciseIssue) {
135+
if (isInSonarLint) {
136+
var quickFixes = preciseIssue.quickFixes();
137+
addQuickFixes(inputFile, ruleKey, quickFixes, newIssue);
138+
}
139+
}
140+
141+
private static void addQuickFixes(InputFile inputFile, RuleKey ruleKey, Iterable<PythonQuickFix> quickFixes, NewIssue sonarLintIssue) {
142+
try {
143+
for (var quickFix : quickFixes) {
144+
var newQuickFix = sonarLintIssue.newQuickFix()
145+
.message(quickFix.getDescription());
146+
147+
var edit = newQuickFix.newInputFileEdit().on(inputFile);
148+
149+
quickFix.getTextEdits().stream()
150+
.map(pythonTextEdit -> edit.newTextEdit().at(rangeFromTextSpan(inputFile, pythonTextEdit))
151+
.withNewText(pythonTextEdit.replacementText()))
152+
.forEach(edit::addTextEdit);
153+
newQuickFix.addInputFileEdit(edit);
154+
sonarLintIssue.addQuickFix(newQuickFix);
155+
}
156+
// TODO : is this try/catch still necessary ?
157+
} catch (RuntimeException e) {
158+
// We still want to report the issue if we did not manage to create a quick fix.
159+
LOG.warn("Could not report quick fixes for rule: {}. {}: {}", ruleKey, e.getClass().getName(), e.getMessage());
160+
}
161+
}
162+
163+
private static TextRange rangeFromTextSpan(InputFile file, PythonTextEdit pythonTextEdit) {
164+
return file.newRange(pythonTextEdit.startLine(), pythonTextEdit.startLineOffset(), pythonTextEdit.endLine(),
165+
pythonTextEdit.endLineOffset());
166+
}
167+
}

python-commons/src/main/java/org/sonar/plugins/python/PythonScanner.java

Lines changed: 3 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,12 @@
2020
import com.sonar.sslr.api.RecognitionException;
2121
import java.io.File;
2222
import java.io.IOException;
23-
import java.util.ArrayDeque;
2423
import java.util.ArrayList;
2524
import java.util.Collection;
2625
import java.util.Collections;
27-
import java.util.Deque;
2826
import java.util.HashSet;
2927
import java.util.List;
3028
import java.util.Map;
31-
import java.util.Optional;
3229
import java.util.Set;
3330
import java.util.concurrent.ConcurrentHashMap;
3431
import java.util.concurrent.atomic.AtomicBoolean;
@@ -40,26 +37,18 @@
4037
import org.slf4j.LoggerFactory;
4138
import org.sonar.api.SonarProduct;
4239
import org.sonar.api.batch.fs.InputFile;
43-
import org.sonar.api.batch.fs.TextRange;
4440
import org.sonar.api.batch.sensor.SensorContext;
45-
import org.sonar.api.batch.sensor.issue.NewIssue;
46-
import org.sonar.api.batch.sensor.issue.NewIssueLocation;
4741
import org.sonar.api.issue.NoSonarFilter;
4842
import org.sonar.api.measures.CoreMetrics;
4943
import org.sonar.api.measures.FileLinesContext;
5044
import org.sonar.api.measures.FileLinesContextFactory;
5145
import org.sonar.api.measures.Metric;
52-
import org.sonar.api.rule.RuleKey;
53-
import org.sonar.plugins.python.api.IssueLocation;
5446
import org.sonar.plugins.python.api.PythonCheck;
55-
import org.sonar.plugins.python.api.PythonCheck.PreciseIssue;
5647
import org.sonar.plugins.python.api.PythonFile;
5748
import org.sonar.plugins.python.api.PythonFileConsumer;
5849
import org.sonar.plugins.python.api.PythonInputFileContext;
5950
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
6051
import org.sonar.plugins.python.api.PythonVisitorContext;
61-
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
62-
import org.sonar.plugins.python.api.quickfix.PythonTextEdit;
6352
import org.sonar.plugins.python.api.tree.FileInput;
6453
import org.sonar.plugins.python.cpd.PythonCpdAnalyzer;
6554
import org.sonar.plugins.python.indexer.PythonIndexer;
@@ -91,6 +80,7 @@ public class PythonScanner extends Scanner {
9180
private final Map<String, Object> repositoryLocks;
9281
private final NewSymbolsCollector newSymbolsCollector;
9382
private final PythonHighlighter pythonHighlighter;
83+
private final IssuesRepository issuesRepository;
9484

9585
public PythonScanner(
9686
SensorContext context, PythonChecks checks, FileLinesContextFactory fileLinesContextFactory, NoSonarFilter noSonarFilter,
@@ -110,6 +100,7 @@ public PythonScanner(
110100
this.repositoryLocks = new ConcurrentHashMap<>();
111101
this.newSymbolsCollector = new NewSymbolsCollector(this);
112102
this.pythonHighlighter = new PythonHighlighter(this);
103+
this.issuesRepository = new IssuesRepository(context, checks, indexer, isInSonarLint(context), this);
113104
}
114105

115106
@Override
@@ -135,7 +126,7 @@ protected void scanFile(PythonInputFile inputFile) throws IOException {
135126
architectureCallback.scanFile(visitorContext);
136127
}
137128

138-
saveIssues(inputFile, visitorContext.getIssues());
129+
issuesRepository.save(inputFile, visitorContext.getIssues());
139130

140131
if (visitorContext.rootTree() != null && !isInSonarLint(context)) {
141132
newSymbolsCollector.collect(context.newSymbolTable().onFile(inputFile.wrappedFile()), visitorContext.rootTree());
@@ -341,79 +332,6 @@ protected void reportStatistics(int numSkippedFiles, int numTotalFiles) {
341332
numSkippedFiles, numTotalFiles);
342333
}
343334

344-
private synchronized void saveIssues(PythonInputFile inputFile, List<PreciseIssue> issues) {
345-
for (PreciseIssue preciseIssue : issues) {
346-
RuleKey ruleKey = checks.ruleKey(preciseIssue.check());
347-
NewIssue newIssue = context
348-
.newIssue()
349-
.forRule(ruleKey);
350-
351-
Integer cost = preciseIssue.cost();
352-
if (cost != null) {
353-
newIssue.gap(cost.doubleValue());
354-
}
355-
356-
NewIssueLocation primaryLocation = newLocation(inputFile, newIssue, preciseIssue.primaryLocation());
357-
newIssue.at(primaryLocation);
358-
359-
Deque<NewIssueLocation> secondaryLocationsFlow = new ArrayDeque<>();
360-
361-
for (IssueLocation secondaryLocation : preciseIssue.secondaryLocations()) {
362-
String fileId = secondaryLocation.fileId();
363-
if (fileId != null) {
364-
InputFile issueLocationFile = component(fileId, context);
365-
if (issueLocationFile != null) {
366-
secondaryLocationsFlow.addFirst(newLocation(new PythonInputFileImpl(issueLocationFile), newIssue, secondaryLocation));
367-
}
368-
} else {
369-
newIssue.addLocation(newLocation(inputFile, newIssue, secondaryLocation));
370-
}
371-
}
372-
373-
// secondary locations on multiple files are only supported using flows
374-
if (!secondaryLocationsFlow.isEmpty()) {
375-
secondaryLocationsFlow.addFirst(primaryLocation);
376-
newIssue.addFlow(secondaryLocationsFlow);
377-
}
378-
379-
handleQuickFixes(inputFile.wrappedFile(), ruleKey, newIssue, preciseIssue);
380-
381-
newIssue.save();
382-
}
383-
}
384-
385-
@CheckForNull
386-
private InputFile component(String fileId, SensorContext sensorContext) {
387-
var predicate = sensorContext.fileSystem().predicates().is(new File(fileId));
388-
InputFile inputFile = Optional.ofNullable(sensorContext.fileSystem().inputFile(predicate))
389-
.orElseGet(() -> indexer.getFileWithId(fileId));
390-
if (inputFile == null) {
391-
LOG.debug("Failed to find InputFile for {}", fileId);
392-
}
393-
return inputFile;
394-
}
395-
396-
private static NewIssueLocation newLocation(PythonInputFile inputFile, NewIssue issue, IssueLocation location) {
397-
NewIssueLocation newLocation = issue.newLocation()
398-
.on(inputFile.wrappedFile());
399-
if (location.startLine() != IssueLocation.UNDEFINED_LINE) {
400-
TextRange range;
401-
if (location.startLineOffset() == IssueLocation.UNDEFINED_OFFSET) {
402-
range = inputFile.wrappedFile().selectLine(location.startLine());
403-
} else {
404-
range = inputFile.wrappedFile().newRange(location.startLine(), location.startLineOffset(), location.endLine(),
405-
location.endLineOffset());
406-
}
407-
newLocation.at(range);
408-
}
409-
410-
String message = location.message();
411-
if (message != null) {
412-
newLocation.message(message);
413-
}
414-
return newLocation;
415-
}
416-
417335
private synchronized void saveMeasures(PythonInputFile inputFile, PythonVisitorContext visitorContext) {
418336
FileMetrics fileMetrics = new FileMetrics(visitorContext, isNotebook(inputFile));
419337
FileLinesVisitor fileLinesVisitor = fileMetrics.fileLinesVisitor();
@@ -467,40 +385,6 @@ private void saveMetricOnFile(PythonInputFile inputFile, Metric<Integer> metric,
467385
.save();
468386
}
469387

470-
private void handleQuickFixes(InputFile inputFile, RuleKey ruleKey, NewIssue newIssue, PreciseIssue preciseIssue) {
471-
if (isInSonarLint(context)) {
472-
List<PythonQuickFix> quickFixes = preciseIssue.quickFixes();
473-
addQuickFixes(inputFile, ruleKey, quickFixes, newIssue);
474-
}
475-
}
476-
477-
private static void addQuickFixes(InputFile inputFile, RuleKey ruleKey, Iterable<PythonQuickFix> quickFixes, NewIssue sonarLintIssue) {
478-
try {
479-
for (PythonQuickFix quickFix : quickFixes) {
480-
var newQuickFix = sonarLintIssue.newQuickFix()
481-
.message(quickFix.getDescription());
482-
483-
var edit = newQuickFix.newInputFileEdit().on(inputFile);
484-
485-
quickFix.getTextEdits().stream()
486-
.map(pythonTextEdit -> edit.newTextEdit().at(rangeFromTextSpan(inputFile, pythonTextEdit))
487-
.withNewText(pythonTextEdit.replacementText()))
488-
.forEach(edit::addTextEdit);
489-
newQuickFix.addInputFileEdit(edit);
490-
sonarLintIssue.addQuickFix(newQuickFix);
491-
}
492-
// TODO : is this try/catch still necessary ?
493-
} catch (RuntimeException e) {
494-
// We still want to report the issue if we did not manage to create a quick fix.
495-
LOG.warn(String.format("Could not report quick fixes for rule: %s. %s: %s", ruleKey, e.getClass().getName(), e.getMessage()));
496-
}
497-
}
498-
499-
private static TextRange rangeFromTextSpan(InputFile file, PythonTextEdit pythonTextEdit) {
500-
return file.newRange(pythonTextEdit.startLine(), pythonTextEdit.startLineOffset(), pythonTextEdit.endLine(),
501-
pythonTextEdit.endLineOffset());
502-
}
503-
504388
public int getRecognitionErrorCount() {
505389
return recognitionErrorCount.get();
506390
}

0 commit comments

Comments
 (0)