Skip to content

Commit

Permalink
SONARPY-2072: Compute correct line location when the cells ends with …
Browse files Browse the repository at this point in the history 
…an empty line
  • Loading branch information
@joke1196
joke1196 committed Nov 26, 2024
1 parent a7ce426 commit 8fc1e1c
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 20 deletions.
17 changes: 15 additions & 2 deletions sonar-python-plugin/src/main/java/org/sonar/plugins/python/IpynbNotebookParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public class IpynbNotebookParser {

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


public static Optional<GeneratedIPythonFile> parseNotebook(PythonInputFile inputFile) {
try {
return new IpynbNotebookParser(inputFile).parse();
Expand Down Expand Up @@ -190,6 +189,14 @@ private static NotebookParsingData parseSourceArray(int startLine, JsonParser jP
}
if (!lastSourceLine.endsWith("\n")) {
cellData.appendToSource("\n");
} else {
// if the last string of the array ends with a newline character we should add this new line to our representation
var newLineLocation = new IPythonLocation(
tokenLocation.getLineNr(),
tokenLocation.getColumnNr(),
List.of(new EscapeCharPositionInfo(tokenLocation.getColumnNr(), 1)),
false);
cellData.addLineToSource("\n", newLineLocation);
}
// Account for the last cell delimiter
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr());
Expand All @@ -212,8 +219,14 @@ private static NotebookParsingData parseSourceMultilineString(int startLine, Jso
previousLen += line.length() + 2;
previousExtraChars += currentCount;
}

if (sourceLine.endsWith("\n")) {
var column = tokenLocation.getColumnNr() + previousExtraChars + previousLen;
cellData.addLineToSource("\n", new IPythonLocation(tokenLocation.getLineNr(), column, List.of(new EscapeCharPositionInfo(column, 1)), true));
previousLen += 2;
}
// Account for the last cell delimiter
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr());
cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr() + previousExtraChars + previousLen);
return cellData;
}

Expand Down
74 changes: 57 additions & 17 deletions sonar-python-plugin/src/test/java/org/sonar/plugins/python/IpynbNotebookParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,26 @@ void testParseNotebook() throws IOException {
assertThat(result.contents()).hasLineCount(27);
assertThat(StringUtils.countMatches(result.contents(), IpynbNotebookParser.SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER))
.isEqualTo(7);
assertThat(result.locationMap()).extracting(map -> map.get(1)).isEqualTo(new IPythonLocation(17, 5));
assertThat(result.locationMap()).extractingByKey(1).isEqualTo(new IPythonLocation(17, 5));
//" print \"not none\"\n"
assertThat(result.locationMap()).extracting(map -> map.get(3)).isEqualTo(new IPythonLocation(19, 5,
assertThat(result.locationMap()).extractingByKey(3).isEqualTo(new IPythonLocation(19, 5,
mapToColumnMappingList(Map.of(10, 1, 19, 1))));
//"source": "#Some code\nprint(\"hello world\\n\")",
assertThat(result.locationMap()).extracting(map -> map.get(16)).isEqualTo(new IPythonLocation(64, 14, List.of(), true));
assertThat(result.locationMap()).extracting(map -> map.get(17)).isEqualTo(new IPythonLocation(64, 26, mapToColumnMappingList(Map.of(6
assertThat(result.locationMap()).extractingByKey(16).isEqualTo(new IPythonLocation(64, 14, List.of(), true));
assertThat(result.locationMap()).extractingByKey(17).isEqualTo(new IPythonLocation(64, 26, mapToColumnMappingList(Map.of(6
, 1, 18, 1, 20, 1)), true));
//"source": "print(\"My\\ntext\")\nprint(\"Something else\\n\")"
assertThat(result.locationMap()).extracting(map -> map.get(22)).isEqualTo(new IPythonLocation(83, 14, mapToColumnMappingList(Map.of(6
assertThat(result.locationMap()).extractingByKey(22).isEqualTo(new IPythonLocation(83, 14, mapToColumnMappingList(Map.of(6
, 1, 9, 1, 15, 1)), true));
assertThat(result.locationMap()).extracting(map -> map.get(23)).isEqualTo(new IPythonLocation(83, 36, mapToColumnMappingList(Map.of(6
assertThat(result.locationMap()).extractingByKey(23).isEqualTo(new IPythonLocation(83, 36, mapToColumnMappingList(Map.of(6
, 1, 21, 1, 23, 1)), true));

//"source": "a = \"A bunch of characters \\n \\f \\r \\ \"\nb = None"
assertThat(result.locationMap()).extracting(map -> map.get(25))
assertThat(result.locationMap()).extractingByKey(25)
.isEqualTo(new IPythonLocation(90, 14, mapToColumnMappingList(Map.of(4, 1, 27, 1, 30, 1, 33, 1, 36, 1, 39, 1)), true));
assertThat(result.locationMap()).extracting(map -> map.get(26)).isEqualTo(new IPythonLocation(90, 62, List.of(), true));
// last line with the cell delimiter which contains the EOF token
assertThat(result.locationMap()).extracting(map -> map.get(27)).isEqualTo(new IPythonLocation(90, 14, List.of()));
assertThat(result.locationMap()).extractingByKey(26).isEqualTo(new IPythonLocation(90, 62, List.of(), true));
// 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
assertThat(result.locationMap()).extractingByKey(27).isEqualTo(new IPythonLocation(90, 72, List.of(), false));
}

@Test
Expand Down Expand Up @@ -111,14 +111,15 @@ void testParseNotebookWithEmptyLines() throws IOException {

var result = resultOptional.get();

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

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

@Test
Expand Down Expand Up @@ -149,16 +150,55 @@ void testParseNotebookWithNoLanguage() {
}

@Test
void testParseNotebookWithExtraLineEndInArray() throws IOException {
void testDifferentJsonRepresentationOfEmptyLine() throws IOException {
var inputFile = createInputFile(baseDir, "notebook_extra_line.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
var inputFileExtraLineExplicit = createInputFile(baseDir, "notebook_extra_line_explicit.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
var inputFileExtraLineExplicitSingleLine = createInputFile(baseDir, "notebook_extra_line_compressed.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);

var notebook = IpynbNotebookParser.parseNotebook(inputFile);
var notebookExtraLineExplicit = IpynbNotebookParser.parseNotebook(inputFileExtraLineExplicit);
var notebookExtraLineExplicitSingleLine = IpynbNotebookParser.parseNotebook(inputFileExtraLineExplicitSingleLine);

assertThat(notebook).isPresent();
assertThat(notebookExtraLineExplicit).isPresent();
assertThat(notebookExtraLineExplicitSingleLine).isPresent();

assertThat(notebook.get().contents()).isEqualTo(notebookExtraLineExplicit.get().contents());
assertThat(notebook.get().contents()).isEqualTo(notebookExtraLineExplicitSingleLine.get().contents());
}

@Test
void testParseNotebookEndingWithEmptyLine() throws IOException {
var inputFile = createInputFile(baseDir, "notebook_extra_line.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);

var resultOptional = IpynbNotebookParser.parseNotebook(inputFile);

assertThat(resultOptional).isPresent();

var result = resultOptional.get();
assertThat(result.locationMap()).hasSize(3);
assertThat(result.contents()).hasLineCount(3);
assertThat(result.locationMap()).hasSize(4);
assertThat(result.contents()).hasLineCount(4);
// The empty line
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::line).isEqualTo(19);
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::column).isEqualTo(5);
// The delimiter
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::line).isEqualTo(19);
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::column).isEqualTo(5);

inputFile = createInputFile(baseDir, "notebook_extra_line_compressed.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
resultOptional = IpynbNotebookParser.parseNotebook(inputFile);

assertThat(resultOptional).isPresent();

result = resultOptional.get();
assertThat(result.locationMap()).hasSize(4);
assertThat(result.contents()).hasLineCount(4);
// The empty line
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::line).isEqualTo(1);
assertThat(result.locationMap()).extractingByKey(3).extracting(IPythonLocation::column).isEqualTo(317);
// The delimiter is added after the empty line
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::line).isEqualTo(1);
assertThat(result.locationMap()).extractingByKey(4).extracting(IPythonLocation::column).isEqualTo(319);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion sonar-python-plugin/src/test/resources/org/sonar/plugins/python/notebook_extra_line.ipynb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
],
"source": [
"#correct += (predicted == y).sum().item()\n",
"#print(f'Epoch {epoch}: {correct / total}')\n"
"print(f'Epoch {epoch}: {correct / total}')\n"
],
"id": "6cbd92ef6319ca9a"
}
Expand Down
1 change: 1 addition & 0 deletions ...n-plugin/src/test/resources/org/sonar/plugins/python/notebook_extra_line_compressed.ipynb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +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}
46 changes: 46 additions & 0 deletions ...hon-plugin/src/test/resources/org/sonar/plugins/python/notebook_extra_line_explicit.ipynb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"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()\n",
"print(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
}

0 comments on commit 8fc1e1c

Please sign in to comment.