Skip to content

Commit 89ec6c0

Browse files
committed
SONARPY-2067 First line of the first cell of notebooks should end at the correct location (#1920)
1 parent 269d714 commit 89ec6c0

File tree

4 files changed

+24
-11
lines changed

4 files changed

+24
-11
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ private static NotebookParsingData parseSourceMultilineString(int startLine, Jso
198198
JsonLocation tokenLocation = jParser.currentTokenLocation();
199199
var previousLen = 0;
200200
var previousExtraChars = 0;
201+
var isFirstLine = true;
201202

202203
for (String line : sourceLine.lines().toList()) {
203204
var countEscapedChar = countEscapeCharacters(line, previousLen + previousExtraChars + tokenLocation.getColumnNr());
@@ -207,6 +208,10 @@ private static NotebookParsingData parseSourceMultilineString(int startLine, Jso
207208
cellData.appendToSource("\n");
208209
previousLen = previousLen + line.length() + 2;
209210
previousExtraChars = previousExtraChars + currentCount;
211+
if (isFirstLine) {
212+
isFirstLine = false;
213+
previousLen += 1;
214+
}
210215
}
211216
// Account for the last cell delimiter
212217
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr());
@@ -218,7 +223,7 @@ private static Map<Integer, Integer> countEscapeCharacters(String sourceLine, in
218223
int count = 0;
219224
var numberOfExtraChars = 0;
220225
var arr = sourceLine.toCharArray();
221-
for (int i = 1; i < sourceLine.length(); ++i) {
226+
for (int i = 0; i < sourceLine.length(); ++i) {
222227
char c = arr[i];
223228
switch (c) {
224229
case '"', '\\':

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,14 @@ void testParseNotebook() throws IOException {
5353
assertThat(result.locationMap()).extracting(map -> map.get(3)).isEqualTo(new IPythonLocation(19, 5, Map.of(10, 16, 19, 26, -1, 2)));
5454
//"source": "#Some code\nprint(\"hello world\\n\")",
5555
assertThat(result.locationMap()).extracting(map -> map.get(16)).isEqualTo(new IPythonLocation(64, 14, Map.of(-1, 0)));
56-
assertThat(result.locationMap()).extracting(map -> map.get(17)).isEqualTo(new IPythonLocation(64, 26, Map.of(6, 33, 18, 46, 20, 49, -1, 3)));
56+
assertThat(result.locationMap()).extracting(map -> map.get(17)).isEqualTo(new IPythonLocation(64, 27, Map.of(6, 34, 18, 47, 20, 50, -1, 3)));
5757
//"source": "print(\"My\\ntext\")\nprint(\"Something else\\n\")"
5858
assertThat(result.locationMap()).extracting(map -> map.get(22)).isEqualTo(new IPythonLocation(83, 14, Map.of(6, 21, 9, 25, 15, 32, -1, 3)));
59-
assertThat(result.locationMap()).extracting(map -> map.get(23)).isEqualTo(new IPythonLocation(83, 36, Map.of(6, 43, 21, 59, 23, 62, -1, 3)));
59+
assertThat(result.locationMap()).extracting(map -> map.get(23)).isEqualTo(new IPythonLocation(83, 37, Map.of(6, 44, 21, 60, 23, 63, -1, 3)));
6060

61-
//"source": "a = \"A bunch of characters \\n \\f \\r \\ \t \"\nb = None"
61+
//"source": "a = \"A bunch of characters \\n \\f \\r \\ \"\nb = None"
6262
assertThat(result.locationMap()).extracting(map -> map.get(25))
63-
.isEqualTo(new IPythonLocation(90, 14, Map.of(4, 19, 27, 43, 30, 47, 33, 51, 36, 55, 40, 61, -1, 6)));
63+
.isEqualTo(new IPythonLocation(90, 14, Map.of(4,19, 27, 43, 30, 47, 33, 51, 36, 55, 39, 59, -1, 6)));
6464
assertThat(result.locationMap()).extracting(map -> map.get(26)).isEqualTo(new IPythonLocation(90, 63, Map.of(-1, 0)));
6565
// last line with the cell delimiter which contains the EOF token
6666
assertThat(result.locationMap()).extracting(map -> map.get(27)).isEqualTo(new IPythonLocation(90, 14, Map.of(-1, 0)));
@@ -82,7 +82,7 @@ void testParseNotebookWithEmptyLines() throws IOException {
8282
.isEqualTo(1);
8383
assertThat(result.locationMap()).extracting(map -> map.get(3)).isEqualTo(new IPythonLocation(11, 5, Map.of(-1, 0)));
8484

85-
// last line with the cell delimiter which contains the EOF token
85+
// last line with the cell delimiter which contains the EOF token
8686
assertThat(result.locationMap()).extracting(map -> map.get(4)).isEqualTo(new IPythonLocation(11, 5, Map.of(-1, 0)));
8787
}
8888

@@ -135,10 +135,18 @@ void testParseNotebookSingleLine() throws IOException {
135135
assertThat(resultOptional).isPresent();
136136

137137
var result = resultOptional.get();
138-
assertThat(result.locationMap()).hasSize(5);
139-
assertThat(result.contents()).hasLineCount(5);
138+
assertThat(result.locationMap()).hasSize(9);
139+
assertThat(result.contents()).hasLineCount(9);
140140
// position of variable t
141-
assertThat(result.locationMap().get(4).column()).isEqualTo(451);
141+
assertThat(result.locationMap().get(4).column()).isEqualTo(452);
142+
143+
// First and second line
144+
assertThat(result.locationMap()).containsEntry(1, new IPythonLocation(1, 382, Map.of(-1, 0)));
145+
assertThat(result.locationMap()).containsEntry(2, new IPythonLocation(1, 429, Map.of(-1, 0)));
146+
147+
assertThat(result.locationMap()).containsEntry(6, new IPythonLocation(1, 559, Map.of(-1, 3, 0, 560, 1, 562, 2, 564)));
148+
assertThat(result.locationMap()).containsEntry(7, new IPythonLocation(1, 610, Map.of(-1, 0)));
149+
assertThat(result.locationMap()).containsEntry(8, new IPythonLocation(1, 637, Map.of(-1, 3, 1, 640, 2, 642, 0, 638)));
142150
}
143151

144152
@Test

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
"execution_count": 1,
8888
"metadata": {},
8989
"outputs": [],
90-
"source": "a = \"A bunch of characters \\n \\f \\r \\ \t \"\nb = None"
90+
"source": "a = \"A bunch of characters \\n \\f \\r \\ \"\nb = None"
9191
}
9292
],
9393
"metadata": {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
{"cells":[{"cell_type":"code","execution_count":null,"metadata":{"execution":{"iopub.execute_input":"2022-04-18T13:57:08.798932Z","iopub.status.busy":"2022-04-18T13:57:08.798479Z","iopub.status.idle":"2022-04-18T13:57:10.082993Z","shell.execute_reply":"2022-04-18T13:57:10.081937Z","shell.execute_reply.started":"2022-04-18T13:57:08.798842Z"},"trusted":true},"outputs":[],"source":"# Code you have previously used to load data\nimport pandas as pd\n\nt = '../input/home-data-for-ml-course/train.csv'"}],"metadata":{"kernelspec":{"display_name":"Python 3","language":"python","name":"python3"},"language_info":{"codemirror_mode":{"name":"ipython","version":3},"file_extension":".py","mimetype":"text/x-python","name":"python","nbconvert_exporter":"python","pygments_lexer":"ipython3","version":"3.6.4"}},"nbformat":4,"nbformat_minor":4}
1+
{"cells":[{"cell_type":"code","execution_count":null,"metadata":{"execution":{"iopub.execute_input":"2022-04-18T13:57:08.798932Z","iopub.status.busy":"2022-04-18T13:57:08.798479Z","iopub.status.idle":"2022-04-18T13:57:10.082993Z","shell.execute_reply":"2022-04-18T13:57:10.081937Z","shell.execute_reply.started":"2022-04-18T13:57:08.798842Z"},"trusted":true},"outputs":[],"source":"# Code you have previously used to load data\nimport pandas as pd\n\nt = '../input/home-data-for-ml-course/train.csv'"},{"cell_type":"code","metadata":{},"outputs":[],"source":"\"\"\" A docstring that is going to be multiline\nHere is the second line !\n\"\"\""}],"metadata":{"kernelspec":{"display_name":"Python 3","language":"python","name":"python3"},"language_info":{"codemirror_mode":{"name":"ipython","version":3},"file_extension":".py","mimetype":"text/x-python","name":"python","nbconvert_exporter":"python","pygments_lexer":"ipython3","version":"3.6.4"}},"nbformat":4,"nbformat_minor":4}
22

0 commit comments

Comments
 (0)