Skip to content

Commit 5732f14

Browse files
joke1196joke1196
authored
SONARPY-2072: Compute correct line location when the cells ends with an empty line (#2180)
1 parent e0dd348 commit 5732f14

5 files changed

Lines changed: 120 additions & 20 deletions

File tree

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ public class IpynbNotebookParser {
3636

3737
private static final Set<String> ACCEPTED_LANGUAGE = Set.of("python", "ipython");
3838

39-
4039
public static Optional<GeneratedIPythonFile> parseNotebook(PythonInputFile inputFile) {
4140
try {
4241
return new IpynbNotebookParser(inputFile).parse();
@@ -187,6 +186,14 @@ private static NotebookParsingData parseSourceArray(int startLine, JsonParser jP
187186
}
188187
if (!lastSourceLine.endsWith("\n")) {
189188
cellData.appendToSource("\n");
189+
} else {
190+
// if the last string of the array ends with a newline character we should add this new line to our representation
191+
var newLineLocation = new IPythonLocation(
192+
tokenLocation.getLineNr(),
193+
tokenLocation.getColumnNr(),
194+
List.of(new EscapeCharPositionInfo(tokenLocation.getColumnNr(), 1)),
195+
false);
196+
cellData.addLineToSource("\n", newLineLocation);
190197
}
191198
// Account for the last cell delimiter
192199
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr());
@@ -209,8 +216,14 @@ private static NotebookParsingData parseSourceMultilineString(int startLine, Jso
209216
previousLen += line.length() + 2;
210217
previousExtraChars += currentCount;
211218
}
219+
220+
if (sourceLine.endsWith("\n")) {
221+
var column = tokenLocation.getColumnNr() + previousExtraChars + previousLen;
222+
cellData.addLineToSource("\n", new IPythonLocation(tokenLocation.getLineNr(), column, List.of(new EscapeCharPositionInfo(column, 1)), true));
223+
previousLen += 2;
224+
}
212225
// Account for the last cell delimiter
213-
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr());
226+
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr() + previousExtraChars + previousLen);
214227
return cellData;
215228
}
216229

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

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,26 +47,26 @@ void testParseNotebook() throws IOException {
4747
assertThat(result.contents()).hasLineCount(27);
4848
assertThat(StringUtils.countMatches(result.contents(), IpynbNotebookParser.SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER))
4949
.isEqualTo(7);
50-
assertThat(result.locationMap()).extracting(map -> map.get(1)).isEqualTo(new IPythonLocation(17, 5));
50+
assertThat(result.locationMap()).extractingByKey(1).isEqualTo(new IPythonLocation(17, 5));
5151
//" print \"not none\"\n"
52-
assertThat(result.locationMap()).extracting(map -> map.get(3)).isEqualTo(new IPythonLocation(19, 5,
52+
assertThat(result.locationMap()).extractingByKey(3).isEqualTo(new IPythonLocation(19, 5,
5353
mapToColumnMappingList(Map.of(10, 1, 19, 1))));
5454
//"source": "#Some code\nprint(\"hello world\\n\")",
55-
assertThat(result.locationMap()).extracting(map -> map.get(16)).isEqualTo(new IPythonLocation(64, 14, List.of(), true));
56-
assertThat(result.locationMap()).extracting(map -> map.get(17)).isEqualTo(new IPythonLocation(64, 26, mapToColumnMappingList(Map.of(6
55+
assertThat(result.locationMap()).extractingByKey(16).isEqualTo(new IPythonLocation(64, 14, List.of(), true));
56+
assertThat(result.locationMap()).extractingByKey(17).isEqualTo(new IPythonLocation(64, 26, mapToColumnMappingList(Map.of(6
5757
, 1, 18, 1, 20, 1)), true));
5858
//"source": "print(\"My\\ntext\")\nprint(\"Something else\\n\")"
59-
assertThat(result.locationMap()).extracting(map -> map.get(22)).isEqualTo(new IPythonLocation(83, 14, mapToColumnMappingList(Map.of(6
59+
assertThat(result.locationMap()).extractingByKey(22).isEqualTo(new IPythonLocation(83, 14, mapToColumnMappingList(Map.of(6
6060
, 1, 9, 1, 15, 1)), true));
61-
assertThat(result.locationMap()).extracting(map -> map.get(23)).isEqualTo(new IPythonLocation(83, 36, mapToColumnMappingList(Map.of(6
61+
assertThat(result.locationMap()).extractingByKey(23).isEqualTo(new IPythonLocation(83, 36, mapToColumnMappingList(Map.of(6
6262
, 1, 21, 1, 23, 1)), true));
6363

6464
//"source": "a = \"A bunch of characters \\n \\f \\r \\ \"\nb = None"
65-
assertThat(result.locationMap()).extracting(map -> map.get(25))
65+
assertThat(result.locationMap()).extractingByKey(25)
6666
.isEqualTo(new IPythonLocation(90, 14, mapToColumnMappingList(Map.of(4, 1, 27, 1, 30, 1, 33, 1, 36, 1, 39, 1)), true));
67-
assertThat(result.locationMap()).extracting(map -> map.get(26)).isEqualTo(new IPythonLocation(90, 62, List.of(), true));
68-
// last line with the cell delimiter which contains the EOF token
69-
assertThat(result.locationMap()).extracting(map -> map.get(27)).isEqualTo(new IPythonLocation(90, 14, List.of()));
67+
assertThat(result.locationMap()).extractingByKey(26).isEqualTo(new IPythonLocation(90, 62, List.of(), true));
68+
// last line with the cell delimiter which contains the EOF token the column of this token should be at the end of the previous line
69+
assertThat(result.locationMap()).extractingByKey(27).isEqualTo(new IPythonLocation(90, 72, List.of(), false));
7070
}
7171

7272
@Test
@@ -108,14 +108,15 @@ void testParseNotebookWithEmptyLines() throws IOException {
108108

109109
var result = resultOptional.get();
110110

111-
assertThat(result.locationMap().keySet()).hasSize(4);
112-
assertThat(result.contents()).hasLineCount(4);
111+
assertThat(result.locationMap().keySet()).hasSize(5);
112+
assertThat(result.contents()).hasLineCount(5);
113113
assertThat(StringUtils.countMatches(result.contents(), IpynbNotebookParser.SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER))
114114
.isEqualTo(1);
115-
assertThat(result.locationMap()).extracting(map -> map.get(3)).isEqualTo(new IPythonLocation(11, 5));
115+
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::line).isEqualTo(11);
116+
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::column).isEqualTo(5);
116117

117118
// last line with the cell delimiter which contains the EOF token
118-
assertThat(result.locationMap()).extracting(map -> map.get(4)).isEqualTo(new IPythonLocation(11, 5));
119+
assertThat(result.locationMap()).extractingByKey(5).isEqualTo(new IPythonLocation(11, 5));
119120
}
120121

121122
@Test
@@ -146,16 +147,55 @@ void testParseNotebookWithNoLanguage() {
146147
}
147148

148149
@Test
149-
void testParseNotebookWithExtraLineEndInArray() throws IOException {
150+
void testDifferentJsonRepresentationOfEmptyLine() throws IOException {
151+
var inputFile = createInputFile(baseDir, "notebook_extra_line.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
152+
var inputFileExtraLineExplicit = createInputFile(baseDir, "notebook_extra_line_explicit.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
153+
var inputFileExtraLineExplicitSingleLine = createInputFile(baseDir, "notebook_extra_line_compressed.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
154+
155+
var notebook = IpynbNotebookParser.parseNotebook(inputFile);
156+
var notebookExtraLineExplicit = IpynbNotebookParser.parseNotebook(inputFileExtraLineExplicit);
157+
var notebookExtraLineExplicitSingleLine = IpynbNotebookParser.parseNotebook(inputFileExtraLineExplicitSingleLine);
158+
159+
assertThat(notebook).isPresent();
160+
assertThat(notebookExtraLineExplicit).isPresent();
161+
assertThat(notebookExtraLineExplicitSingleLine).isPresent();
162+
163+
assertThat(notebook.get().contents()).isEqualTo(notebookExtraLineExplicit.get().contents());
164+
assertThat(notebook.get().contents()).isEqualTo(notebookExtraLineExplicitSingleLine.get().contents());
165+
}
166+
167+
@Test
168+
void testParseNotebookEndingWithEmptyLine() throws IOException {
150169
var inputFile = createInputFile(baseDir, "notebook_extra_line.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
151170

152171
var resultOptional = IpynbNotebookParser.parseNotebook(inputFile);
153172

154173
assertThat(resultOptional).isPresent();
155174

156175
var result = resultOptional.get();
157-
assertThat(result.locationMap()).hasSize(3);
158-
assertThat(result.contents()).hasLineCount(3);
176+
assertThat(result.locationMap()).hasSize(4);
177+
assertThat(result.contents()).hasLineCount(4);
178+
// The empty line
179+
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::line).isEqualTo(19);
180+
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::column).isEqualTo(5);
181+
// The delimiter
182+
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::line).isEqualTo(19);
183+
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::column).isEqualTo(5);
184+
185+
inputFile = createInputFile(baseDir, "notebook_extra_line_compressed.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
186+
resultOptional = IpynbNotebookParser.parseNotebook(inputFile);
187+
188+
assertThat(resultOptional).isPresent();
189+
190+
result = resultOptional.get();
191+
assertThat(result.locationMap()).hasSize(4);
192+
assertThat(result.contents()).hasLineCount(4);
193+
// The empty line
194+
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::line).isEqualTo(1);
195+
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::column).isEqualTo(317);
196+
// The delimiter is added after the empty line
197+
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::line).isEqualTo(1);
198+
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::column).isEqualTo(319);
159199
}
160200

161201
@Test

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
],
1717
"source": [
1818
"#correct += (predicted == y).sum().item()\n",
19-
"#print(f'Epoch {epoch}: {correct / total}')\n"
19+
"print(f'Epoch {epoch}: {correct / total}')\n"
2020
],
2121
"id": "6cbd92ef6319ca9a"
2222
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{ "cells": [{"cell_type": "code","execution_count": 5,"metadata": {},"outputs": [{"name": "stdout","output_type": "stream","text": ["Files already downloaded and verified\n","Files already downloaded and verified\n"]}],"source": "#correct += (predicted == y).sum().item()\nprint(f'Epoch {epoch}: {correct / total}')\n","id": "6cbd92ef6319ca9a" } ], "metadata": { "kernelspec": {"display_name": "Python 3","language": "python","name": "python3" }, "language_info": {"codemirror_mode": {"name": "ipython","version": 2},"file_extension": ".py","mimetype": "text/x-python","name": "python","nbconvert_exporter": "python","pygments_lexer": "ipython2","version": "2.7.6" } }, "nbformat": 4, "nbformat_minor": 5}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "code",
5+
"execution_count": 5,
6+
"metadata": {},
7+
"outputs": [
8+
{
9+
"name": "stdout",
10+
"output_type": "stream",
11+
"text": [
12+
"Files already downloaded and verified\n",
13+
"Files already downloaded and verified\n"
14+
]
15+
}
16+
],
17+
"source": [
18+
"#correct += (predicted == y).sum().item()\n",
19+
"print(f'Epoch {epoch}: {correct / total}')\n",
20+
""
21+
],
22+
"id": "6cbd92ef6319ca9a"
23+
}
24+
],
25+
"metadata": {
26+
"kernelspec": {
27+
"display_name": "Python 3",
28+
"language": "python",
29+
"name": "python3"
30+
},
31+
"language_info": {
32+
"codemirror_mode": {
33+
"name": "ipython",
34+
"version": 2
35+
},
36+
"file_extension": ".py",
37+
"mimetype": "text/x-python",
38+
"name": "python",
39+
"nbconvert_exporter": "python",
40+
"pygments_lexer": "ipython2",
41+
"version": "2.7.6"
42+
}
43+
},
44+
"nbformat": 4,
45+
"nbformat_minor": 5
46+
}

0 commit comments

Comments
 (0)