Skip to content

Commit c8382dc

Browse files
maksim-grebeniuk-sonarsourcesonartech
maksim-grebeniuk-sonarsource
authored and
sonartech
committed
SONARPY-2757 Make PythonHighlighter thread safe (#234)
GitOrigin-RevId: 7fc6f907ab7860cf06ee408f50d0a20f40bcef76
1 parent 4dde9b4 commit c8382dc

3 files changed

Lines changed: 84 additions & 56 deletions

File tree

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

Lines changed: 67 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.sonar.api.batch.sensor.highlighting.TypeOfText;
2626
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2727
import org.sonar.plugins.python.api.PythonVisitorContext;
28+
import org.sonar.plugins.python.api.TokenLocation;
2829
import org.sonar.plugins.python.api.tree.ClassDef;
2930
import org.sonar.plugins.python.api.tree.FileInput;
3031
import org.sonar.plugins.python.api.tree.FunctionDef;
@@ -33,7 +34,6 @@
3334
import org.sonar.plugins.python.api.tree.Tree;
3435
import org.sonar.plugins.python.api.tree.Trivia;
3536
import org.sonar.python.SubscriptionVisitor;
36-
import org.sonar.plugins.python.api.TokenLocation;
3737
import org.sonar.python.api.PythonKeyword;
3838
import org.sonar.python.api.PythonTokenType;
3939

@@ -83,73 +83,93 @@
8383
* Reminder: a docstring is a string literal that occurs as the first statement in a module,
8484
* function, class, or method definition.
8585
*/
86-
public class PythonHighlighter extends PythonSubscriptionCheck {
86+
public class PythonHighlighter {
8787

88-
private NewHighlighting newHighlighting;
89-
private Set<Token> docStringTokens;
88+
private final Object monitor;
9089

91-
public PythonHighlighter(SensorContext context, PythonInputFile inputFile) {
92-
docStringTokens = new HashSet<>();
93-
newHighlighting = context.newHighlighting();
94-
newHighlighting.onFile(inputFile.wrappedFile());
90+
public PythonHighlighter(Object monitor) {
91+
this.monitor = monitor;
9592
}
9693

97-
@Override
98-
public void scanFile(PythonVisitorContext visitorContext) {
99-
SubscriptionVisitor.analyze(Collections.singletonList(this), visitorContext);
94+
public PythonHighlighter() {
95+
this(new Object());
10096
}
10197

102-
@Override
103-
public void initialize(Context context) {
104-
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> checkFirstStatement(((FileInput) ctx.syntaxNode()).docstring()));
105-
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> checkFirstStatement(((FunctionDef) ctx.syntaxNode()).docstring()));
106-
context.registerSyntaxNodeConsumer(Tree.Kind.CLASSDEF, ctx -> checkFirstStatement(((ClassDef) ctx.syntaxNode()).docstring()));
107-
context.registerSyntaxNodeConsumer(Tree.Kind.TOKEN, ctx -> visitToken(((Token) ctx.syntaxNode())));
98+
public void highlight(SensorContext sensorContext, PythonVisitorContext visitorContext, PythonInputFile inputFile) {
99+
var check = new PythonHighlighterSubscriptionCheck(sensorContext, inputFile);
100+
check.scanFile(visitorContext);
101+
save(check.newHighlighting);
108102
}
109103

110-
private void checkFirstStatement(@Nullable StringLiteral docString) {
111-
if (docString == null) {
112-
return;
113-
}
114-
for (Tree stringElement : docString.children()) {
115-
highlight(stringElement.firstToken(), TypeOfText.STRUCTURED_COMMENT);
116-
docStringTokens.add(stringElement.firstToken());
104+
private void save(NewHighlighting newHighlighting) {
105+
synchronized (monitor) {
106+
newHighlighting.save();
117107
}
118108
}
119109

120-
private void visitToken(Token token) {
121-
if (token.type().equals(PythonTokenType.NUMBER)) {
122-
highlight(token, TypeOfText.CONSTANT);
110+
private static class PythonHighlighterSubscriptionCheck extends PythonSubscriptionCheck {
123111

124-
} else if (token.type() instanceof PythonKeyword) {
125-
highlight(token, TypeOfText.KEYWORD);
112+
private final NewHighlighting newHighlighting;
113+
private final Set<Token> docStringTokens;
126114

127-
} else if (token.type().equals(PythonTokenType.STRING) && !docStringTokens.contains(token)) {
128-
highlight(token, TypeOfText.STRING);
115+
public PythonHighlighterSubscriptionCheck(SensorContext context, PythonInputFile inputFile) {
116+
docStringTokens = new HashSet<>();
117+
newHighlighting = context.newHighlighting();
118+
newHighlighting.onFile(inputFile.wrappedFile());
119+
}
129120

130-
} else if (token.type().equals(IDENTIFIER) && isPython3Keyword(token.value())) {
131-
// async and await are keywords starting python 3.5, however, for compatibility with previous versions, we cannot consider them as real keywords
132-
highlight(token, TypeOfText.KEYWORD);
121+
@Override
122+
public void scanFile(PythonVisitorContext visitorContext) {
123+
SubscriptionVisitor.analyze(Collections.singletonList(this), visitorContext);
124+
}
133125

126+
@Override
127+
public void initialize(Context context) {
128+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> checkFirstStatement(((FileInput) ctx.syntaxNode()).docstring()));
129+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> checkFirstStatement(((FunctionDef) ctx.syntaxNode()).docstring()));
130+
context.registerSyntaxNodeConsumer(Tree.Kind.CLASSDEF, ctx -> checkFirstStatement(((ClassDef) ctx.syntaxNode()).docstring()));
131+
context.registerSyntaxNodeConsumer(Tree.Kind.TOKEN, ctx -> visitToken(((Token) ctx.syntaxNode())));
134132
}
135133

136-
for (Trivia trivia : token.trivia()) {
137-
highlight(trivia.token(), TypeOfText.COMMENT);
134+
private void checkFirstStatement(@Nullable StringLiteral docString) {
135+
if (docString == null) {
136+
return;
137+
}
138+
for (Tree stringElement : docString.children()) {
139+
highlight(stringElement.firstToken(), TypeOfText.STRUCTURED_COMMENT);
140+
docStringTokens.add(stringElement.firstToken());
141+
}
138142
}
139-
}
140143

141-
private static boolean isPython3Keyword(String value) {
142-
return "await".equals(value) || "async".equals(value) || "match".equals(value) || "case".equals(value);
143-
}
144+
private void visitToken(Token token) {
145+
if (token.type().equals(PythonTokenType.NUMBER)) {
146+
highlight(token, TypeOfText.CONSTANT);
144147

145-
@Override
146-
public void leaveFile() {
147-
newHighlighting.save();
148-
}
148+
} else if (token.type() instanceof PythonKeyword) {
149+
highlight(token, TypeOfText.KEYWORD);
149150

150-
private void highlight(Token token, TypeOfText typeOfText) {
151-
TokenLocation tokenLocation = new TokenLocation(token);
152-
newHighlighting.highlight(tokenLocation.startLine(), tokenLocation.startLineOffset(), tokenLocation.endLine(), tokenLocation.endLineOffset(), typeOfText);
151+
} else if (token.type().equals(PythonTokenType.STRING) && !docStringTokens.contains(token)) {
152+
highlight(token, TypeOfText.STRING);
153+
154+
} else if (token.type().equals(IDENTIFIER) && isPython3Keyword(token.value())) {
155+
// async and await are keywords starting python 3.5, however, for compatibility with previous versions, we cannot consider them as real keywords
156+
highlight(token, TypeOfText.KEYWORD);
157+
158+
}
159+
160+
for (Trivia trivia : token.trivia()) {
161+
highlight(trivia.token(), TypeOfText.COMMENT);
162+
}
163+
}
164+
165+
private static boolean isPython3Keyword(String value) {
166+
return "await".equals(value) || "async".equals(value) || "match".equals(value) || "case".equals(value);
167+
}
168+
169+
private void highlight(Token token, TypeOfText typeOfText) {
170+
TokenLocation tokenLocation = new TokenLocation(token);
171+
newHighlighting.highlight(tokenLocation.startLine(), tokenLocation.startLineOffset(), tokenLocation.endLine(), tokenLocation.endLineOffset(), typeOfText);
172+
}
153173
}
154174

155175
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ public class PythonScanner extends Scanner {
9292
private final PythonFileConsumer architectureCallback;
9393
private final Map<String, Object> repositoryLocks;
9494
private final NewSymbolsCollector newSymbolsCollector;
95+
private final PythonHighlighter pythonHighlighter;
9596

9697
public PythonScanner(
9798
SensorContext context, PythonChecks checks, FileLinesContextFactory fileLinesContextFactory, NoSonarFilter noSonarFilter,
@@ -110,6 +111,7 @@ public PythonScanner(
110111
this.foundDatabricks = new AtomicBoolean(false);
111112
this.repositoryLocks = new ConcurrentHashMap<>();
112113
this.newSymbolsCollector = new NewSymbolsCollector(this);
114+
this.pythonHighlighter = new PythonHighlighter(this);
113115
}
114116

115117
@Override
@@ -156,8 +158,8 @@ protected void scanFile(PythonInputFile inputFile) throws IOException {
156158
saveIssues(inputFile, visitorContext.getIssues());
157159

158160
if (visitorContext.rootTree() != null && !isInSonarLint(context)) {
159-
new PythonHighlighter(context, inputFile).scanFile(visitorContext);
160161
newSymbolsCollector.collect(context.newSymbolTable().onFile(inputFile.wrappedFile()), visitorContext.rootTree());
162+
pythonHighlighter.highlight(context, visitorContext, inputFile);
161163
}
162164

163165
searchForDataBricks(visitorContext);

‎python-commons/src/test/java/org/sonar/plugins/python/PythonHighlighterTest.java‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.sonar.api.batch.sensor.internal.SensorContextTester;
3030
import org.sonar.python.IPythonLocation;
3131
import org.sonar.python.TestPythonVisitorRunner;
32+
import org.sonar.python.caching.CacheContextImpl;
33+
import org.sonar.python.semantic.ProjectLevelSymbolTable;
3234

3335
import static org.assertj.core.api.Assertions.assertThat;
3436
import static org.sonar.plugins.python.NotebookTestUtils.mapToColumnMappingList;
@@ -67,8 +69,9 @@ void scanFile() {
6769
context.fileSystem().add(notebookInputFile);
6870
context.fileSystem().add(notebookInputFileSingleLine);
6971

70-
PythonHighlighter pythonHighlighter = new PythonHighlighter(context, new PythonInputFileImpl(inputFile));
71-
TestPythonVisitorRunner.scanFile(file, pythonHighlighter);
72+
var visitorContext = TestPythonVisitorRunner.createContext(file);
73+
var pythonHighlighter = new PythonHighlighter();
74+
pythonHighlighter.highlight(context, visitorContext, new PythonInputFileImpl(inputFile));
7275
}
7376

7477
@Test
@@ -225,9 +228,10 @@ def foo():
225228
5, new IPythonLocation(13, 5),
226229
6, new IPythonLocation(14, 5, mapToColumnMappingList(Map.of(4, 1))),
227230
7, new IPythonLocation(14, 5)); //EOF Token
228-
PythonHighlighter pythonHighlighter = new PythonHighlighter(context, new GeneratedIPythonFile(notebookInputFile, pythonContent,
229-
locations));
230-
TestPythonVisitorRunner.scanNotebookFile(notebookFile, locations, pythonContent, pythonHighlighter);
231+
232+
var visitorContext = TestPythonVisitorRunner.createNotebookContext(notebookFile, locations, pythonContent, "", ProjectLevelSymbolTable.empty(), CacheContextImpl.dummyCache());
233+
var pythonHighlighter = new PythonHighlighter();
234+
pythonHighlighter.highlight(context, visitorContext, new GeneratedIPythonFile(notebookInputFile, pythonContent, locations));
231235
// def
232236
checkOnRange(9, 5, 3, notebookFile, TypeOfText.KEYWORD);
233237
// pass
@@ -252,9 +256,11 @@ void highlightingNotebooksSingleLine() {
252256
2, new IPythonLocation(1, 108, List.of(), true),
253257
3, new IPythonLocation(1, 121, List.of(), true),
254258
4, new IPythonLocation(1, 93, List.of(), true)); //EOF Token
255-
PythonHighlighter pythonHighlighter = new PythonHighlighter(context, new GeneratedIPythonFile(notebookInputFileSingleLine,
256-
pythonContent, locations));
257-
TestPythonVisitorRunner.scanNotebookFile(notebookFileSingleLine, locations, pythonContent, pythonHighlighter);
259+
260+
var visitorContext = TestPythonVisitorRunner.createNotebookContext(notebookFileSingleLine, locations, pythonContent, "", ProjectLevelSymbolTable.empty(), CacheContextImpl.dummyCache());
261+
var pythonHighlighter = new PythonHighlighter();
262+
pythonHighlighter.highlight(context, visitorContext, new GeneratedIPythonFile(notebookInputFileSingleLine, pythonContent, locations));
263+
258264
// def
259265
checkOnRange(1, 93, 3, notebookFileSingleLine, TypeOfText.KEYWORD);
260266
// pass

0 commit comments

Comments
 (0)