diff --git a/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java b/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java index 6e00deb07..89bb47ff1 100644 --- a/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java +++ b/python-commons/src/test/java/org/sonar/plugins/python/IpynbNotebookParserScannerTest.java @@ -92,4 +92,39 @@ void trailing_whitespace_compressed() throws IOException { assertThat(issues.get(2).primaryLocation().endLineOffset()).isEqualTo(249); // Should be 255 } + @Test + void multiline_string_in_source_array() throws IOException { + var inputFile = createInputFile(baseDir, "notebook_multiline_string_in_array.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); + var result = IpynbNotebookParser.parseNotebook(inputFile).get(); + + // Should not throw IllegalStateException("No IPythonLocation found for line ...") + var fileInput = TestPythonVisitorRunner.parseNotebookFile(result.locationMap(), result.contents()); + + var statements = fileInput.statements().statements(); + // Every statement lives on the same raw ipynb line, since the whole array element is one JSON string + // spanning a single physical line; columns must still strictly increase to reflect their real position. + assertThat(statements) + .hasSize(8) + .allSatisfy(stmt -> assertThat(stmt.firstToken().line()).isEqualTo(9)) + .extracting(stmt -> stmt.firstToken().column()).isSorted(); + } + + @Test + void multiline_array_element_without_trailing_newline_continues_next_element() throws IOException { + // "source": ["x = 1\ny", " = 2"]: the first element's last split line ("y") has no trailing newline of + // its own, so the second element (" = 2") continues it on the same physical line instead of starting + // a new one. This used to add one locationMap entry too many, misaligning every line after it. + var inputFile = createInputFile(baseDir, "notebook_multiline_string_in_array_no_trailing_newline.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); + var result = IpynbNotebookParser.parseNotebook(inputFile).get(); + + var fileInput = TestPythonVisitorRunner.parseNotebookFile(result.locationMap(), result.contents()); + + var statements = fileInput.statements().statements(); + assertThat(statements).hasSize(2); + assertThat(statements.get(0).firstToken().line()).isEqualTo(9); + assertThat(statements.get(1).firstToken().line()).isEqualTo(9); + // "y = 2" starts further along the raw ipynb line than "x = 1" + assertThat(statements.get(1).firstToken().column()).isGreaterThan(statements.get(0).firstToken().column()); + } + } diff --git a/python-commons/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array.ipynb b/python-commons/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array.ipynb new file mode 100644 index 000000000..8c14af694 --- /dev/null +++ b/python-commons/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array.ipynb @@ -0,0 +1,22 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 0, + "metadata": {}, + "outputs": [], + "source": [ + "print(\"Test 1\")\ntestA = \"1\"\nprint(\"Test 2\")\ntestB = \"2\"\nprint(\"Test 3\")\ntestC = \"3\"\n\n\ntestAll = testA + testB + testC\n\nprint(testAll)" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "name": "test", + "notebookId": 1 + }, + "nbformat": 4, + "nbformat_minor": 0 +} diff --git a/python-commons/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array_no_trailing_newline.ipynb b/python-commons/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array_no_trailing_newline.ipynb new file mode 100644 index 000000000..a27406576 --- /dev/null +++ b/python-commons/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array_no_trailing_newline.ipynb @@ -0,0 +1,23 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 0, + "metadata": {}, + "outputs": [], + "source": [ + "x = 1\ny", + " = 2" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "name": "test", + "notebookId": 1 + }, + "nbformat": 4, + "nbformat_minor": 0 +} diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/IpynbNotebookParser.java b/python-frontend/src/main/java/org/sonar/plugins/python/IpynbNotebookParser.java index 533ccdc4e..e75f06104 100644 --- a/python-frontend/src/main/java/org/sonar/plugins/python/IpynbNotebookParser.java +++ b/python-frontend/src/main/java/org/sonar/plugins/python/IpynbNotebookParser.java @@ -240,57 +240,148 @@ private static NotebookParsingData parseSourceArray(int startLine, JsonParser jP JsonLocation tokenLocation = jParser.currentTokenLocation(); // In case of an empty cell, we don't add an extra line var lastSourceLine = "\n"; + var lastOffset = LineSplitOffset.NONE; + // Whether the array element about to be processed starts on a fresh physical line. An element only + // does if the previous one ended with a newline; otherwise it is glued onto the previous element's + // still-open line and must not get its own locationMap entry (there is only room for one per line). + var startsNewLine = true; while (jParser.nextToken() != JsonToken.END_ARRAY) { String sourceLine = jParser.getValueAsString(); var newTokenLocation = jParser.currentTokenLocation(); - var countEscapedChar = countEscapeCharacters(sourceLine); - cellData.addLineToSource(sourceLine, newTokenLocation.getLineNr(), newTokenLocation.getColumnNr(), countEscapedChar, isCompressed); + lastOffset = addSourceArrayElement(cellData, sourceLine, newTokenLocation, isCompressed, startsNewLine); + startsNewLine = sourceLine.endsWith("\n"); lastSourceLine = sourceLine; tokenLocation = newTokenLocation; } + // Column right after the last array element's content, accounting for any lines split out of a + // multiline string contained in that element. + int lastColumn = tokenLocation.getColumnNr() + lastOffset.length() + lastOffset.extraChars(); 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)), + lastColumn, + List.of(new EscapeCharPositionInfo(lastColumn, 1)), false); cellData.addLineToSource("\n", newLineLocation); } // 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(), lastColumn); return cellData; } + /** + * Adds a single "source" array element to the cell data. Most array elements represent exactly one + * Python source line, but the JSON schema also allows an element to itself contain embedded newlines + * (a multiline string used as one array item), in which case it is split the same way a top-level + * multiline string "source" value is. Returns the raw-content offset consumed within the element's + * JSON token, relative to its token location, so the caller can correctly position whatever follows it. + * + *

If the previous element did not end with a newline, this element continues that still-open + * physical line rather than starting a new one: its first (or only) segment is appended as plain text, + * with no locationMap entry of its own, since a generated line can only be anchored to a single + * original position. + */ + private static LineSplitOffset addSourceArrayElement(NotebookParsingData cellData, String sourceLine, JsonLocation tokenLocation, boolean isCompressed, + boolean startsNewLine) { + List lines = sourceLine.lines().toList(); + if (lines.size() <= 1) { + if (!startsNewLine) { + cellData.appendToSource(sourceLine); + return LineSplitOffset.NONE; + } + var countEscapedChar = countEscapeCharacters(sourceLine); + cellData.addLineToSource(sourceLine, tokenLocation.getLineNr(), tokenLocation.getColumnNr(), countEscapedChar, isCompressed); + return LineSplitOffset.NONE; + } + // The element packs multiple Python lines into a single array entry: each embedded line needs its + // own location entry, the same way parseSourceMultilineString handles a plain-string "source" - + // except its first line, which only gets one if it genuinely starts a new physical line. + var offset = addSourceLinesToCellData(cellData, lines, tokenLocation, true, startsNewLine); + if (sourceLine.endsWith("\n")) { + cellData.appendToSource("\n"); + offset = offset.plusNewline(); + } + return offset; + } + private static NotebookParsingData parseSourceMultilineString(int startLine, JsonParser jParser) throws IOException { NotebookParsingData cellData = NotebookParsingData.fromLine(startLine); String sourceLine = jParser.getValueAsString(); JsonLocation tokenLocation = jParser.currentTokenLocation(); - var previousLen = 0; - var previousExtraChars = 0; - for (String line : sourceLine.lines().toList()) { - var countEscapedChar = countEscapeCharacters(line); - var currentCount = countEscapedChar.stream().mapToInt(EscapeCharPositionInfo::numberOfExtraChars).sum(); - cellData.addLineToSource(line, new IPythonLocation(tokenLocation.getLineNr(), - tokenLocation.getColumnNr() + previousLen + previousExtraChars, countEscapedChar, true)); - cellData.appendToSource("\n"); - previousLen += line.length() + 2; - previousExtraChars += currentCount; - } + var offset = addSourceLinesToCellData(cellData, sourceLine.lines().toList(), tokenLocation, true, true); + // The last split line is always followed by a newline: either the cell delimiter or the next cell's content. + cellData.appendToSource("\n"); + offset = offset.plusNewline(); if (sourceLine.endsWith("\n")) { - var column = tokenLocation.getColumnNr() + previousExtraChars + previousLen; + var column = tokenLocation.getColumnNr() + offset.length() + offset.extraChars(); cellData.addLineToSource("\n", new IPythonLocation(tokenLocation.getLineNr(), column, List.of(new EscapeCharPositionInfo(column, 1)), true)); - previousLen += 2; + offset = offset.plusNewline(); } // Account for the last cell delimiter - cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr() + previousExtraChars + previousLen); + cellData.addDelimiterToSource(SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER + "\n", tokenLocation.getLineNr(), tokenLocation.getColumnNr() + offset.length() + offset.extraChars()); return cellData; } + /** + * Splits a JSON string value's content into individual Python source lines and adds each to the cell + * data, computing the column of every line within the original JSON token. Every line except the last + * is followed by an explicit newline in the aggregated source, since it was followed by an embedded + * newline in the JSON value; the caller decides how to terminate the last one. + * + *

When {@code firstLineStartsNewLine} is false, the first line continues a still-open physical line + * from whatever was appended just before it, so it is added as plain text with no locationMap entry of + * its own (a generated line can only be anchored to a single original position). + */ + private static LineSplitOffset addSourceLinesToCellData(NotebookParsingData cellData, List lines, JsonLocation tokenLocation, boolean isCompressed, + boolean firstLineStartsNewLine) { + var offset = LineSplitOffset.NONE; + for (int i = 0; i < lines.size(); i++) { + String line = lines.get(i); + var countEscapedChar = countEscapeCharacters(line); + var currentExtraChars = countEscapedChar.stream().mapToInt(EscapeCharPositionInfo::numberOfExtraChars).sum(); + if (i > 0 || firstLineStartsNewLine) { + cellData.addLineToSource(line, new IPythonLocation(tokenLocation.getLineNr(), + tokenLocation.getColumnNr() + offset.length() + offset.extraChars(), countEscapedChar, isCompressed)); + } else { + cellData.appendToSource(line); + } + boolean hasMoreLines = i < lines.size() - 1; + if (hasMoreLines) { + cellData.appendToSource("\n"); + offset = offset.plusLine(line.length(), currentExtraChars); + } else { + offset = offset.plusLastLine(line.length(), currentExtraChars); + } + } + return offset; + } + + /** + * Tracks how many raw JSON characters (and how many of those are "extra" escape characters) have + * been consumed from a JSON string token while splitting it into individual source lines. + */ + private record LineSplitOffset(int length, int extraChars) { + static final LineSplitOffset NONE = new LineSplitOffset(0, 0); + + LineSplitOffset plusLine(int lineLength, int lineExtraChars) { + // +2 accounts for the JSON-escaped "\n" (backslash + n) separating this line from the next one. + return new LineSplitOffset(length + lineLength + 2, extraChars + lineExtraChars); + } + + LineSplitOffset plusLastLine(int lineLength, int lineExtraChars) { + return new LineSplitOffset(length + lineLength, extraChars + lineExtraChars); + } + + LineSplitOffset plusNewline() { + return new LineSplitOffset(length + 2, extraChars); + } + } + private static List countEscapeCharacters(String sourceLine) { List escapeCharPositionInfoList = new LinkedList<>(); var arr = sourceLine.toCharArray(); diff --git a/python-frontend/src/test/java/org/sonar/plugins/python/IpynbNotebookParserTest.java b/python-frontend/src/test/java/org/sonar/plugins/python/IpynbNotebookParserTest.java index 01934ccbb..beed8cf18 100644 --- a/python-frontend/src/test/java/org/sonar/plugins/python/IpynbNotebookParserTest.java +++ b/python-frontend/src/test/java/org/sonar/plugins/python/IpynbNotebookParserTest.java @@ -260,6 +260,57 @@ void testParseNotebookSingleLine() throws IOException { assertThat(result.locationMap()).containsEntry(8, new IPythonLocation(1, 636, mapToColumnMappingList(Map.of(1, 1, 2, 1, 0, 1)), true)); } + @Test + void testParseNotebookWithMultilineArrayElementWithoutTrailingNewline() throws IOException { + // "source": ["x = 1\ny", " = 2"] - the first element has no trailing newline, so its last split + // line ("y") is continued, not terminated, by the next array element (" = 2"), giving 2 python lines + // ("x = 1" and "y = 2") rather than 3. Before the fix, each array element always got its own + // locationMap entry regardless of whether the previous one ended with a newline, producing one more + // entry than there are physical lines and misaligning every location after it. + var inputFile = createInputFile(baseDir, "notebook_multiline_string_in_array_no_trailing_newline.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); + + var resultOptional = IpynbNotebookParser.parseNotebook(inputFile); + + assertThat(resultOptional).isPresent(); + + var result = resultOptional.get(); + assertThat(result.contents()).hasLineCount(3); + assertThat(result.locationMap().keySet()).hasSize(3); + //"x = 1" + assertThat(result.locationMap()).extractingByKey(1).isEqualTo(new IPythonLocation(9, 9, List.of(), true)); + //"y = 2" ("y" starts the line, " = 2" from the next array element continues it) + assertThat(result.locationMap()).extractingByKey(2).isEqualTo(new IPythonLocation(9, 16, List.of(), true)); + // the cell delimiter + assertThat(result.locationMap()).extractingByKey(3).isEqualTo(new IPythonLocation(10, 9, List.of(), false)); + } + + @Test + void testParseNotebookWithMultilineStringInSourceArray() throws IOException { + var inputFile = createInputFile(baseDir, "notebook_multiline_string_in_array.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); + + var resultOptional = IpynbNotebookParser.parseNotebook(inputFile); + + assertThat(resultOptional).isPresent(); + + var result = resultOptional.get(); + // 11 python lines packed into the single "source" array element, plus the cell delimiter. + // Before the fix, only 2 entries were produced (the whole array element counted as a single line), + // which made TokenEnricher fail with "No IPythonLocation found for line 3". + assertThat(result.contents()).hasLineCount(12); + assertThat(result.locationMap().keySet()).hasSize(12); + //"print(\"Test 1\")" + assertThat(result.locationMap()).extractingByKey(1).isEqualTo(new IPythonLocation(9, 9, mapToColumnMappingList(Map.of(6, 1, 13, 1)), true)); + //"testA = \"1\"" + assertThat(result.locationMap()).extractingByKey(2).isEqualTo(new IPythonLocation(9, 28, mapToColumnMappingList(Map.of(8, 1, 10, 1)), true)); + // two consecutive empty lines + assertThat(result.locationMap()).extractingByKey(7).isEqualTo(new IPythonLocation(9, 111, List.of(), true)); + assertThat(result.locationMap()).extractingByKey(8).isEqualTo(new IPythonLocation(9, 113, List.of(), true)); + //"print(testAll)" (last content line, no trailing newline in the JSON value) + assertThat(result.locationMap()).extractingByKey(11).isEqualTo(new IPythonLocation(9, 150, List.of(), true)); + // the cell delimiter + assertThat(result.locationMap()).extractingByKey(12).isEqualTo(new IPythonLocation(9, 164, List.of(), false)); + } + @Test void testParseNotebook1() throws IOException { var inputFile = createInputFile(baseDir, "notebook_no_code.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN); diff --git a/python-frontend/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array.ipynb b/python-frontend/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array.ipynb new file mode 100644 index 000000000..8c14af694 --- /dev/null +++ b/python-frontend/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array.ipynb @@ -0,0 +1,22 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 0, + "metadata": {}, + "outputs": [], + "source": [ + "print(\"Test 1\")\ntestA = \"1\"\nprint(\"Test 2\")\ntestB = \"2\"\nprint(\"Test 3\")\ntestC = \"3\"\n\n\ntestAll = testA + testB + testC\n\nprint(testAll)" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "name": "test", + "notebookId": 1 + }, + "nbformat": 4, + "nbformat_minor": 0 +} diff --git a/python-frontend/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array_no_trailing_newline.ipynb b/python-frontend/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array_no_trailing_newline.ipynb new file mode 100644 index 000000000..a27406576 --- /dev/null +++ b/python-frontend/src/test/resources/org/sonar/plugins/python/notebook_multiline_string_in_array_no_trailing_newline.ipynb @@ -0,0 +1,23 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 0, + "metadata": {}, + "outputs": [], + "source": [ + "x = 1\ny", + " = 2" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "name": "test", + "notebookId": 1 + }, + "nbformat": 4, + "nbformat_minor": 0 +}