Skip to content

Commit c22ec01

Browse files
joke1196ghislainpiot
joke1196
authored and
ghislainpiot
committed
SONARPY-1987: IPynbSensor should not crash when an error occurs during the parsing of notebooks (#1873)
1 parent 0b44f90 commit c22ec01

File tree

7 files changed

+180
-31
lines changed

7 files changed

+180
-31
lines changed

‎sonar-python-plugin/src/main/java/org/sonar/plugins/python/IPynbSensor.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import org.sonar.api.batch.sensor.SensorDescriptor;
3333
import org.sonar.api.issue.NoSonarFilter;
3434
import org.sonar.api.measures.FileLinesContextFactory;
35-
import org.sonar.plugins.python.IpynbNotebookParser.ParseResult;
3635
import org.sonar.plugins.python.api.ProjectPythonVersion;
3736
import org.sonar.plugins.python.api.PythonVersionUtils;
3837
import org.sonar.plugins.python.api.caching.CacheContext;
@@ -50,6 +49,7 @@ public final class IPynbSensor implements Sensor {
5049
private final FileLinesContextFactory fileLinesContextFactory;
5150
private final NoSonarFilter noSonarFilter;
5251
private final PythonIndexer indexer;
52+
private static final String FAIL_FAST_PROPERTY_NAME = "sonar.internal.analysis.failFast";
5353

5454
public IPynbSensor(FileLinesContextFactory fileLinesContextFactory, CheckFactory checkFactory, NoSonarFilter noSonarFilter) {
5555
this(fileLinesContextFactory, checkFactory, noSonarFilter, null);
@@ -86,19 +86,25 @@ public void execute(SensorContext context) {
8686
}
8787

8888
private void processNotebooksFiles(List<PythonInputFile> pythonFiles, SensorContext context) {
89-
pythonFiles = parseNotebooks(pythonFiles);
90-
// Disable caching for IPynb files for now
89+
pythonFiles = parseNotebooks(pythonFiles, context);
90+
// Disable caching for IPynb files for now see: SONARPY-2020
9191
CacheContext cacheContext = CacheContextImpl.dummyCache();
9292
PythonIndexer pythonIndexer = new SonarQubePythonIndexer(pythonFiles, cacheContext, context);
9393
PythonScanner scanner = new PythonScanner(context, checks, fileLinesContextFactory, noSonarFilter, PythonParser.createIPythonParser(), pythonIndexer);
9494
scanner.execute(pythonFiles, context);
9595
}
9696

97-
private static List<PythonInputFile> parseNotebooks(List<PythonInputFile> pythonFiles) {
97+
private static List<PythonInputFile> parseNotebooks(List<PythonInputFile> pythonFiles, SensorContext context) {
9898
List<PythonInputFile> generatedIPythonFiles = new ArrayList<>();
9999
for (PythonInputFile inputFile : pythonFiles) {
100-
ParseResult result = IpynbNotebookParser.parseNotebook(inputFile);
101-
generatedIPythonFiles.add(result.inputFile());
100+
try {
101+
PythonInputFile result = IpynbNotebookParser.parseNotebook(inputFile);
102+
generatedIPythonFiles.add(result);
103+
} catch (Exception e) {
104+
if (context.config().getBoolean(FAIL_FAST_PROPERTY_NAME).orElse(false) && !isErrorOnTestFile(inputFile)) {
105+
throw new IllegalStateException("Exception when parsing " + inputFile, e);
106+
}
107+
}
102108
}
103109
return generatedIPythonFiles;
104110
}
@@ -116,4 +122,8 @@ private static List<PythonInputFile> getInputFiles(SensorContext context) {
116122
it.forEach(f -> list.add(new PythonInputFileImpl(f)));
117123
return Collections.unmodifiableList(list);
118124
}
125+
126+
private static boolean isErrorOnTestFile(PythonInputFile inputFile) {
127+
return inputFile.wrappedFile().type() == InputFile.Type.TEST;
128+
}
119129
}

‎sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonPlugin.java

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ public class PythonPlugin implements Plugin {
5757
private static final String DEPRECATED_PREFIX = "DEPRECATED : Use " + PythonCoverageSensor.REPORT_PATHS_KEY + " instead. ";
5858

5959
public static final String PYTHON_FILE_SUFFIXES_KEY = "sonar.python.file.suffixes";
60-
public static final String IPYNB_FILE_SUFFIXES_KEY = "sonar.ipynb.file.suffixes";
6160

6261
@Override
6362
public void define(Context context) {
@@ -74,7 +73,6 @@ public void define(Context context) {
7473
.defaultValue("py")
7574
.build(),
7675

77-
7876
PropertyDefinition.builder(PYTHON_VERSION_KEY)
7977
.index(11)
8078
.name("Python versions")
@@ -91,19 +89,8 @@ public void define(Context context) {
9189

9290
PythonSensor.class,
9391
PythonRuleRepository.class,
94-
AnalysisWarningsWrapper.class);
92+
AnalysisWarningsWrapper.class,
9593

96-
context.addExtensions(
97-
PropertyDefinition.builder(IPYNB_FILE_SUFFIXES_KEY)
98-
.index(12)
99-
.name("IPython File Suffixes")
100-
.description("List of suffixes of IPython Notebooks files to analyze.")
101-
.multiValues(true)
102-
.category(PYTHON_CATEGORY)
103-
.subCategory(GENERAL)
104-
.onQualifiers(Qualifiers.PROJECT)
105-
.defaultValue("ipynb")
106-
.build(),
10794
IPynb.class,
10895
IPynbProfile.class,
10996
IPynbSensor.class,
@@ -120,7 +107,6 @@ public void define(Context context) {
120107
addRuffExtensions(context);
121108
}
122109

123-
124110
if (sonarRuntime.getProduct() == SonarProduct.SONARLINT) {
125111
SonarLintPluginAPIManager sonarLintPluginAPIManager = new SonarLintPluginAPIManager();
126112
sonarLintPluginAPIManager.addSonarlintPythonIndexer(context, new SonarLintPluginAPIVersion());

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ protected String name() {
151151

152152
@Override
153153
protected void scanFile(PythonInputFile inputFile) throws IOException {
154-
addFile(inputFile);
154+
// Global Symbol Table is deactivated for Notebooks see: SONARPY-2021
155+
if (inputFile.kind() == PythonInputFile.Kind.PYTHON) {
156+
addFile(inputFile);
157+
}
155158
}
156159

157160
@Override

‎sonar-python-plugin/src/test/java/org/sonar/plugins/python/IPynbSensorTest.java

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ private IPynbSensor sensor(PythonIndexer indexer) {
129129
return new IPynbSensor(fileLinesContextFactory, checkFactory, mock(NoSonarFilter.class), indexer);
130130
}
131131

132-
133132
private PythonInputFile inputFile(String name) {
134133
PythonInputFile inputFile = createInputFile(name);
135134
context.fileSystem().add(inputFile.wrappedFile());
@@ -174,11 +173,37 @@ private IPynbSensor notebookSensor() {
174173
}
175174

176175
@Test
177-
void test_notebook_sensor_cannot_read_python_file_is_executed() {
176+
void test_notebook_sensor_error_should_throw_if_fail_fast() {
177+
context.settings().setProperty("sonar.internal.analysis.failFast", true);
178178
inputFile(FILE_1);
179179
activeRules = new ActiveRulesBuilder().build();
180180
IPynbSensor sensor = notebookSensor();
181-
assertThrows("Cannot read file1.ipynb", IllegalStateException.class, () -> sensor.execute(context));
181+
Throwable throwable = assertThrows(IllegalStateException.class, () -> sensor.execute(context));
182+
assertThat(throwable.getClass()).isEqualTo(IllegalStateException.class);
183+
assertThat(throwable.getMessage()).isEqualTo("Exception when parsing file1.ipynb");
184+
}
185+
186+
@Test
187+
void test_notebook_sensor_should_not_throw_on_test_file() {
188+
context.settings().setProperty("sonar.internal.analysis.failFast", true);
189+
PythonInputFile testFile = new PythonInputFileImpl(TestInputFileBuilder.create("moduleKey", FILE_1)
190+
.setModuleBaseDir(baseDir.toPath())
191+
.setCharset(UTF_8)
192+
.setType(InputFile.Type.TEST)
193+
.setLanguage(IPynb.KEY)
194+
.initMetadata(TestUtils.fileContent(new File(baseDir, FILE_1), UTF_8))
195+
.setStatus(InputFile.Status.ADDED)
196+
.build());
197+
context.fileSystem().add(testFile.wrappedFile());
198+
activeRules = new ActiveRulesBuilder().build();
199+
assertDoesNotThrow(() -> notebookSensor().execute(context));
200+
}
201+
202+
@Test
203+
void test_notebook_sensor_cannot_parse_file() {
204+
inputFile(FILE_1);
205+
activeRules = new ActiveRulesBuilder().build();
206+
assertDoesNotThrow(() -> notebookSensor().execute(context));
182207
}
183208

184209
@Test

‎sonar-python-plugin/src/test/java/org/sonar/plugins/python/PythonPluginTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ class PythonPluginTest {
4646
void testGetExtensions() {
4747
Version v79 = Version.create(7, 9);
4848
SonarRuntime runtime = SonarRuntimeImpl.forSonarQube(v79, SonarQubeSide.SERVER, SonarEdition.DEVELOPER);
49-
assertThat(extensions(runtime)).hasSize(33);
49+
assertThat(extensions(runtime)).hasSize(32);
5050
assertThat(extensions(runtime)).contains(AnalysisWarningsWrapper.class);
5151
assertThat(extensions(SonarRuntimeImpl.forSonarLint(v79)))
52-
.hasSize(14)
52+
.hasSize(13)
5353
.contains(SonarLintCache.class);
5454
}
5555

‎sonar-python-plugin/src/test/java/org/sonar/plugins/python/indexer/SonarQubePythonIndexerTest.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ void test_modified_dependency() throws IOException, NoSuchAlgorithmException {
141141
byte[] outdatedEntry = toProtobufModuleDescriptor(Set.of(new VariableDescriptor("outdated", "mod.outdated", null))).toByteArray();
142142
readCache.put(importsMapCacheKey("moduleKey:main.py"), importsAsByteArray(List.of("unknown", "mod", "other")));
143143
readCache.put(importsMapCacheKey("moduleKey:mod.py"), importsAsByteArray(Collections.emptyList()));
144-
readCache.put(projectSymbolTableCacheKey("moduleKey:main.py") , serializedSymbolTable);
144+
readCache.put(projectSymbolTableCacheKey("moduleKey:main.py"), serializedSymbolTable);
145145
readCache.put(projectSymbolTableCacheKey("moduleKey:mod.py"), outdatedEntry);
146146
readCache.put(fileContentHashCacheKey("moduleKey:main.py"), inputFileContentHash(file1.wrappedFile()));
147147
readCache.put(fileContentHashCacheKey("moduleKey:mod.py"), inputFileContentHash(file2.wrappedFile()));
@@ -311,7 +311,6 @@ void test_no_file_modified_invalid_cache_version() throws IOException, NoSuchAlg
311311
.contains("2/2 source files have been analyzed");
312312
}
313313

314-
315314
@Test
316315
void test_no_file_modified_invalid_cache_version_due_to_changed_python_version() throws IOException, NoSuchAlgorithmException {
317316
file1 = createInputFile(baseDir, "main.py", InputFile.Status.SAME, InputFile.Type.MAIN);
@@ -446,8 +445,7 @@ void test_typeshed_modules_not_cached_if_empty() {
446445
void test_regular_scan_when_scan_without_parsing_fails() {
447446
List<PythonInputFile> files = List.of(createInputFile(baseDir, "main.py", InputFile.Status.SAME, InputFile.Type.MAIN));
448447
PythonIndexer.GlobalSymbolsScanner globalSymbolsScanner = spy(
449-
new SonarQubePythonIndexer(files, cacheContext, context).new GlobalSymbolsScanner(context)
450-
);
448+
new SonarQubePythonIndexer(files, cacheContext, context).new GlobalSymbolsScanner(context));
451449
when(globalSymbolsScanner.canBeScannedWithoutParsing(any())).thenReturn(true);
452450
globalSymbolsScanner.execute(files, context);
453451

@@ -518,6 +516,19 @@ void hash_exception_when_trying_to_compare_hash() throws IOException, NoSuchAlgo
518516
assertThat(pythonIndexer.canBePartiallyScannedWithoutParsing(file1)).isFalse();
519517
}
520518

519+
@Test
520+
void test_notebook_should_not_be_in_project_level_symbol_table() {
521+
file1 = createInputFile(baseDir, "notebook.ipynb", InputFile.Status.SAME, InputFile.Type.MAIN);
522+
file2 = createInputFile(baseDir, "mod.py", InputFile.Status.SAME, InputFile.Type.MAIN);
523+
List<PythonInputFile> inputFiles = new ArrayList<>(List.of(file1, file2));
524+
525+
pythonIndexer = new SonarQubePythonIndexer(inputFiles, cacheContext, context);
526+
pythonIndexer.buildOnce(context);
527+
528+
assertThat(pythonIndexer.projectLevelSymbolTable().getSymbolsFromModule("mod")).isNotEmpty();
529+
assertThat(pythonIndexer.projectLevelSymbolTable().getSymbolsFromModule("notebook")).isEmpty();
530+
}
531+
521532
private byte[] importsAsByteArray(List<String> mod) {
522533
return String.join(";", mod).getBytes(StandardCharsets.UTF_8);
523534
}

‎sonar-python-plugin/src/test/resources/org/sonar/plugins/python/indexer/notebook.ipynb

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "code",
5+
"execution_count": 1,
6+
"metadata": {},
7+
"outputs": [
8+
{
9+
"name": "stdout",
10+
"output_type": "stream",
11+
"text": [
12+
"hello world\n"
13+
]
14+
}
15+
],
16+
"source": [
17+
"x = None\n",
18+
"if x is not None:\n",
19+
" print \"not none\"\n",
20+
"\n",
21+
"\n",
22+
"def foo():\n",
23+
" x = 42\n",
24+
" x = 17\n",
25+
" print(x)"
26+
]
27+
},
28+
{
29+
"cell_type": "markdown",
30+
"metadata": {},
31+
"source": [
32+
"# Hello\n",
33+
"This is some markdown"
34+
]
35+
},
36+
{
37+
"cell_type": "markdown",
38+
"metadata": {},
39+
"source": [
40+
"This is another markdown cell"
41+
]
42+
},
43+
{
44+
"cell_type": "code",
45+
"execution_count": null,
46+
"metadata": {},
47+
"outputs": [],
48+
"source": [
49+
"if x is not None:\n",
50+
" print(\"hello\")"
51+
]
52+
},
53+
{
54+
"cell_type": "code",
55+
"execution_count": null,
56+
"metadata": {},
57+
"outputs": [],
58+
"source": [
59+
"x = 42"
60+
]
61+
},
62+
{
63+
"cell_type": "code",
64+
"source": "#Some code\nprint(\"hello world\\n\")",
65+
"execution_count": 0,
66+
"outputs": [],
67+
"metadata": {}
68+
},
69+
{
70+
"cell_type": "code",
71+
"execution_count": null,
72+
"metadata": {},
73+
"outputs": [],
74+
"source": [
75+
"print(\"My\\ntext\")\n", "print(\"Something else\\n\")"
76+
]
77+
},
78+
{
79+
"cell_type": "code",
80+
"execution_count": null,
81+
"metadata": {},
82+
"outputs": [],
83+
"source": "print(\"My\\ntext\")\nprint(\"Something else\\n\")"
84+
},
85+
{
86+
"cell_type": "code",
87+
"execution_count": 1,
88+
"metadata": {},
89+
"outputs": [],
90+
"source": "a = \"A bunch of characters \\n \\t \\f \\r / // \\ \"\nb = None"
91+
}
92+
],
93+
"metadata": {
94+
"kernelspec": {
95+
"display_name": "jupyter-experiment_venv",
96+
"language": "python",
97+
"name": "python3"
98+
},
99+
"language_info": {
100+
"codemirror_mode": {
101+
"name": "ipython",
102+
"version": 3
103+
},
104+
"file_extension": ".py",
105+
"mimetype": "text/x-python",
106+
"name": "python",
107+
"nbconvert_exporter": "python",
108+
"pygments_lexer": "ipython3",
109+
"version": "3.12.2"
110+
}
111+
},
112+
"nbformat": 4,
113+
"nbformat_minor": 2
114+
}

0 commit comments

Comments
 (0)