Skip to content

Commit 4af3650

Browse files
ghislainpiotsonartech
ghislainpiot
authored and
sonartech
committed
SONARPY-2824 Un-parallelize foreign rule repositories (#229)
GitOrigin-RevId: 1867a1405e04f3eac7555d5a38f2580299910703
1 parent c5a5c82 commit 4af3650

3 files changed

Lines changed: 134 additions & 42 deletions

File tree

‎python-commons/src/main/java/org/sonar/plugins/python/PythonChecks.java‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import java.util.Collection;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.Set;
2223
import java.util.concurrent.ConcurrentHashMap;
24+
import java.util.stream.Collectors;
2325
import java.util.stream.Stream;
2426
import javax.annotation.Nullable;
2527
import org.sonar.api.batch.rule.CheckFactory;
@@ -30,13 +32,17 @@
3032
import org.sonar.plugins.python.api.internal.EndOfAnalysis;
3133

3234
public class PythonChecks {
35+
private static final Set<String> SONAR_PYTHON_REPOSITORIES = Set.of("python", "pythonenterprise", "ipython", "ipythonenterprise");
36+
3337
private final CheckFactory checkFactory;
34-
private final Map<String, RepositoryChecksInfo> repositoriesChecks;
38+
private final Map<String, RepositoryChecksInfo> sonarPythonRepositoriesChecks;
39+
private final Map<String, RepositoryChecksInfo> noSonarPythonRepositoriesChecks;
3540
private final Map<Class<? extends PythonCheck>, RuleKey> ruleKeys;
3641

3742
PythonChecks(CheckFactory checkFactory) {
3843
this.checkFactory = checkFactory;
39-
this.repositoriesChecks = new ConcurrentHashMap<>();
44+
this.sonarPythonRepositoriesChecks = new ConcurrentHashMap<>();
45+
this.noSonarPythonRepositoriesChecks = new ConcurrentHashMap<>();
4046
this.ruleKeys = new ConcurrentHashMap<>();
4147
}
4248

@@ -48,7 +54,11 @@ public PythonChecks addChecks(String repositoryKey, Iterable<Class<?>> checkClas
4854
var ruleKey = checks.ruleKey(check);
4955
ruleKeys.put(checkClass, ruleKey);
5056
});
51-
repositoriesChecks.put(repositoryChecksInfo.repositoryKey, repositoryChecksInfo);
57+
if (SONAR_PYTHON_REPOSITORIES.contains(repositoryKey)) {
58+
sonarPythonRepositoriesChecks.put(repositoryChecksInfo.repositoryKey, repositoryChecksInfo);
59+
} else {
60+
noSonarPythonRepositoriesChecks.put(repositoryChecksInfo.repositoryKey, repositoryChecksInfo);
61+
}
5262
return this;
5363
}
5464

@@ -59,21 +69,37 @@ public PythonChecks addCustomChecks(@Nullable PythonCustomRuleRepository[] custo
5969
return this;
6070
}
6171

62-
public synchronized List<PythonCheck> all() {
63-
return repositoriesChecks.values().stream()
72+
public synchronized List<PythonCheck> sonarPythonChecks() {
73+
return sonarPythonRepositoriesChecks.values().stream()
6474
.map(this::createChecks)
6575
.map(Checks::all)
6676
.flatMap(Collection::stream)
6777
.toList();
6878
}
6979

70-
public List<EndOfAnalysis> endOfAnalyses() {
71-
return all().stream()
80+
public synchronized Map<String, List<PythonCheck>> noSonarPythonChecks() {
81+
return noSonarPythonRepositoriesChecks.values()
82+
.stream()
83+
.collect(Collectors.toMap(
84+
RepositoryChecksInfo::repositoryKey,
85+
repositoryChecksInfo -> createChecks(repositoryChecksInfo).all().stream().toList()));
86+
}
87+
88+
public List<EndOfAnalysis> sonarPythonEndOfAnalyses() {
89+
return sonarPythonChecks().stream()
7290
.filter(EndOfAnalysis.class::isInstance)
7391
.map(EndOfAnalysis.class::cast)
7492
.toList();
7593
}
7694

95+
public Map<String, List<EndOfAnalysis>> noSonarPythonEndOfAnalyses() {
96+
return noSonarPythonChecks().entrySet().stream()
97+
.collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().stream()
98+
.filter(EndOfAnalysis.class::isInstance)
99+
.map(EndOfAnalysis.class::cast)
100+
.toList()));
101+
}
102+
77103
@Nullable
78104
public RuleKey ruleKey(PythonCheck check) {
79105
return ruleKeys.getOrDefault(check.getClass(), null);

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

Lines changed: 101 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.io.IOException;
2323
import java.util.ArrayDeque;
2424
import java.util.ArrayList;
25+
import java.util.Collection;
2526
import java.util.Collections;
2627
import java.util.Deque;
2728
import java.util.HashSet;
@@ -77,6 +78,7 @@ public class PythonScanner extends Scanner {
7778
private static final Logger LOG = LoggerFactory.getLogger(PythonScanner.class);
7879
private static final Pattern DATABRICKS_MAGIC_COMMAND_PATTERN = Pattern.compile("^\\h*#\\h*(MAGIC|COMMAND).*");
7980
public static final String THREADS_PROPERTY_NAME = "sonar.python.analysis.threads";
81+
private static final String ARCHITECTURE_CALLBACK_LOCK_KEY = "architectureCallbackLock";
8082

8183
private final Supplier<PythonParser> parserSupplier;
8284
private final PythonChecks checks;
@@ -88,6 +90,7 @@ public class PythonScanner extends Scanner {
8890
private final AtomicInteger recognitionErrorCount;
8991
private final AtomicBoolean foundDatabricks;
9092
private final PythonFileConsumer architectureCallback;
93+
private final Map<String, Object> repositoryLocks;
9194

9295
public PythonScanner(
9396
SensorContext context, PythonChecks checks, FileLinesContextFactory fileLinesContextFactory, NoSonarFilter noSonarFilter,
@@ -104,6 +107,7 @@ public PythonScanner(
104107
this.checksExecutedWithoutParsingByFiles = new ConcurrentHashMap<>();
105108
this.recognitionErrorCount = new AtomicInteger(0);
106109
this.foundDatabricks = new AtomicBoolean(false);
110+
this.repositoryLocks = new ConcurrentHashMap<>();
107111
}
108112

109113
@Override
@@ -137,8 +141,29 @@ protected Stream<PythonInputFile> getFilesStream(List<PythonInputFile> files) {
137141
@Override
138142
protected void scanFile(PythonInputFile inputFile) throws IOException {
139143
var pythonFile = SonarQubePythonFile.create(inputFile);
140-
PythonVisitorContext visitorContext;
141144
InputFile.Type fileType = inputFile.wrappedFile().type();
145+
PythonVisitorContext visitorContext = createVisitorContext(inputFile, pythonFile, fileType);
146+
147+
executeChecks(visitorContext, checks.sonarPythonChecks(), fileType, inputFile);
148+
executeOtherChecks(inputFile, visitorContext, fileType);
149+
150+
synchronized (repositoryLocks.computeIfAbsent(ARCHITECTURE_CALLBACK_LOCK_KEY, k -> new Object())) {
151+
architectureCallback.scanFile(visitorContext);
152+
}
153+
154+
saveIssues(inputFile, visitorContext.getIssues());
155+
156+
if (visitorContext.rootTree() != null && !isInSonarLint(context)) {
157+
new SymbolVisitor(context.newSymbolTable().onFile(inputFile.wrappedFile())).visitFileInput(visitorContext.rootTree());
158+
new PythonHighlighter(context, inputFile).scanFile(visitorContext);
159+
}
160+
161+
searchForDataBricks(visitorContext);
162+
}
163+
164+
165+
private PythonVisitorContext createVisitorContext(PythonInputFile inputFile, PythonFile pythonFile, InputFile.Type fileType) throws IOException {
166+
PythonVisitorContext visitorContext;
142167
try {
143168
AstNode astNode = parserSupplier.get().parse(inputFile.contents());
144169
PythonTreeMaker treeMaker = getTreeMaker(inputFile);
@@ -169,28 +194,33 @@ protected void scanFile(PythonInputFile inputFile) throws IOException {
169194
.message(newMessage)
170195
.save();
171196
}
172-
List<PythonSubscriptionCheck> checksBasedOnTree = new ArrayList<>();
173-
for (PythonCheck check : checks.all()) {
197+
return visitorContext;
198+
}
199+
200+
private void executeChecks(PythonVisitorContext visitorContext, Collection<PythonCheck> checks, InputFile.Type fileType, PythonInputFile inputFile) {
201+
Collection<PythonSubscriptionCheck> subscriptionChecks = new ArrayList<>();
202+
for (PythonCheck check : checks) {
174203
if (isCheckNotApplicable(check, fileType)
175-
|| checksExecutedWithoutParsingByFiles.getOrDefault(inputFile, Collections.emptySet()).contains(check.getClass())) {
204+
|| checksExecutedWithoutParsingByFiles.getOrDefault(inputFile, Collections.emptySet()).contains(check.getClass())) {
176205
continue;
177206
}
178207
if (check instanceof PythonSubscriptionCheck pythonSubscriptionCheck) {
179-
checksBasedOnTree.add(pythonSubscriptionCheck);
208+
subscriptionChecks.add(pythonSubscriptionCheck);
180209
} else {
181210
check.scanFile(visitorContext);
182211
}
183212
}
184-
SubscriptionVisitor.analyze(checksBasedOnTree, visitorContext);
185-
architectureCallback.scanFile(visitorContext);
186-
saveIssues(inputFile, visitorContext.getIssues());
187-
188-
if (visitorContext.rootTree() != null && !isInSonarLint(context)) {
189-
new SymbolVisitor(context.newSymbolTable().onFile(inputFile.wrappedFile())).visitFileInput(visitorContext.rootTree());
190-
new PythonHighlighter(context, inputFile).scanFile(visitorContext);
191-
}
213+
SubscriptionVisitor.analyze(subscriptionChecks, visitorContext);
214+
}
192215

193-
searchForDataBricks(visitorContext);
216+
private void executeOtherChecks(PythonInputFile inputFile, PythonVisitorContext visitorContext, InputFile.Type fileType) {
217+
checks.noSonarPythonChecks().forEach((repositoryKey, repositoryChecks) -> {
218+
var lock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new Object());
219+
synchronized (lock) {
220+
executeChecks(visitorContext, repositoryChecks, fileType, inputFile);
221+
}
222+
}
223+
);
194224
}
195225

196226
private void searchForDataBricks(PythonVisitorContext visitorContext) {
@@ -223,25 +253,21 @@ public boolean scanFileWithoutParsing(PythonInputFile inputFile) {
223253
context.runtime().getProduct(),
224254
indexer.projectLevelSymbolTable()
225255
);
226-
for (PythonCheck check : checks.all()) {
227-
if (isCheckNotApplicable(check, fileType)) {
228-
continue;
229-
}
230-
if (checkRequiresParsingOfImpactedFile(inputFile, check)) {
231-
// For regular Python checks, only directly modified files need to be analyzed
232-
// For DBD and Security, transitively impacted files must be re-analyzed.
233-
result = false;
234-
continue;
235-
}
236256

237-
if (check.scanWithoutParsing(inputFileContext)) {
238-
var executedChecks = checksExecutedWithoutParsingByFiles.getOrDefault(inputFile, new HashSet<>());
239-
executedChecks.add(check.getClass());
240-
checksExecutedWithoutParsingByFiles.putIfAbsent(inputFile, executedChecks);
241-
} else {
242-
result = false;
257+
result = scanFileWithoutParsingSonarPython(inputFile, fileType, inputFileContext, result);
258+
259+
var otherChecks = checks.noSonarPythonChecks();
260+
for (var entry : otherChecks.entrySet()) {
261+
var repositoryKey = entry.getKey();
262+
var repositoryChecks = entry.getValue();
263+
var lock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new Object());
264+
synchronized (lock) {
265+
for (var check : repositoryChecks) {
266+
result = scanFileWithoutParsingNotSonarPython(inputFile, check, fileType, result, inputFileContext);
267+
}
243268
}
244269
}
270+
245271
result &= architectureCallback.scanWithoutParsing(inputFileContext);
246272
if (!result) {
247273
// If scan without parsing is not successful, measures will be pushed during regular scan.
@@ -251,14 +277,55 @@ public boolean scanFileWithoutParsing(PythonInputFile inputFile) {
251277
return restoreAndPushMeasuresIfApplicable(inputFile);
252278
}
253279

254-
private boolean checkRequiresParsingOfImpactedFile(PythonInputFile inputFile, PythonCheck check) {
255-
return !indexer.canBeFullyScannedWithoutParsing(inputFile) && !check.getClass().getPackageName().startsWith("org.sonar.python.checks");
280+
private boolean scanFileWithoutParsingNotSonarPython(PythonInputFile inputFile, PythonCheck check, InputFile.Type fileType, boolean result,
281+
PythonInputFileContext inputFileContext) {
282+
if (isCheckNotApplicable(check, fileType)) {
283+
return result;
284+
}
285+
if (!indexer.canBeFullyScannedWithoutParsing(inputFile)) {
286+
result = false;
287+
return result;
288+
}
289+
if (check.scanWithoutParsing(inputFileContext)) {
290+
var executedChecks = checksExecutedWithoutParsingByFiles.getOrDefault(inputFile, new HashSet<>());
291+
executedChecks.add(check.getClass());
292+
checksExecutedWithoutParsingByFiles.putIfAbsent(inputFile, executedChecks);
293+
} else {
294+
result = false;
295+
}
296+
return result;
297+
}
298+
299+
private boolean scanFileWithoutParsingSonarPython(PythonInputFile inputFile, InputFile.Type fileType, PythonInputFileContext inputFileContext, boolean result) {
300+
var ourChecks = checks.sonarPythonChecks();
301+
for (var check : ourChecks) {
302+
if (isCheckNotApplicable(check, fileType)) {
303+
continue;
304+
}
305+
if (check.scanWithoutParsing(inputFileContext)) {
306+
var executedChecks = checksExecutedWithoutParsingByFiles.getOrDefault(inputFile, new HashSet<>());
307+
executedChecks.add(check.getClass());
308+
checksExecutedWithoutParsingByFiles.putIfAbsent(inputFile, executedChecks);
309+
} else {
310+
result = false;
311+
}
312+
}
313+
return result;
256314
}
257315

316+
258317
@Override
259318
public void endOfAnalysis() {
260319
indexer.postAnalysis(context);
261-
checks.endOfAnalyses().forEach(c -> c.endOfAnalysis(indexer.cacheContext()));
320+
checks.sonarPythonEndOfAnalyses().forEach(c -> c.endOfAnalysis(indexer.cacheContext()));
321+
checks.noSonarPythonEndOfAnalyses().forEach(
322+
(repositoryKey, endOfAnalyses) -> {
323+
var lock = repositoryLocks.computeIfAbsent(repositoryKey, k -> new Object());
324+
synchronized (lock) {
325+
endOfAnalyses.forEach(c -> c.endOfAnalysis(indexer.cacheContext()));
326+
}
327+
}
328+
);
262329
}
263330

264331
boolean isCheckNotApplicable(PythonCheck pythonCheck, InputFile.Type fileType) {
@@ -401,7 +468,7 @@ static boolean isNotebook(PythonInputFile inputFile) {
401468
return inputFile.kind() == PythonInputFile.Kind.IPYTHON;
402469
}
403470

404-
private boolean restoreAndPushMeasuresIfApplicable(PythonInputFile inputFile) {
471+
private synchronized boolean restoreAndPushMeasuresIfApplicable(PythonInputFile inputFile) {
405472
if (inputFile.wrappedFile().type() == InputFile.Type.TEST) {
406473
return true;
407474
}

‎python-commons/src/main/java/org/sonar/plugins/python/indexer/PythonIndexer.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.sonar.plugins.python.api.tree.FileInput;
4343
import org.sonar.python.parser.PythonParser;
4444
import org.sonar.python.semantic.ProjectLevelSymbolTable;
45-
import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
4645
import org.sonar.python.tree.PythonTreeMaker;
4746

4847
import static org.sonar.python.semantic.SymbolUtils.pythonPackageName;

0 commit comments

Comments
 (0)