SONARPY-2061: Fix bug when no code is present in the notebook (#1929)

Author: joke1196

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

@@ -24,7 +24,9 @@
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonToken;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -95,23 +97,24 @@ private Optional<NotebookParsingData> parseCells(JsonParser parser) throws IOExc
       if ("cells".equals(fieldName)) {
         // consume array start token
         parser.nextToken();
-        NotebookParsingData data = parseCellArray(parser);
+        Optional<NotebookParsingData> data = parseCellArray(parser);
         parser.close();
-        return Optional.of(data);
+        return data;
       }
     }
     return Optional.empty();
   }
 
-  private NotebookParsingData parseCellArray(JsonParser jParser) throws IOException {
-    NotebookParsingData aggregatedNotebookData = NotebookParsingData.empty();
+  private Optional<NotebookParsingData> parseCellArray(JsonParser jParser) throws IOException {
+    List<NotebookParsingData> cellsData = new ArrayList<>();
 
     while (jParser.nextToken() != JsonToken.END_ARRAY) {
       if (jParser.currentToken() == JsonToken.START_OBJECT) {
-        processCodeCell(aggregatedNotebookData.getAggregatedSourceLine(), jParser).ifPresent(aggregatedNotebookData::combine);
+        processCodeCell(cellsData, jParser);
       }
     }
-    aggregatedNotebookData.removeTrailingExtraLine();
+    Optional<NotebookParsingData> aggregatedNotebookData = cellsData.stream().reduce(NotebookParsingData::combine);
+    aggregatedNotebookData.ifPresent(NotebookParsingData::removeTrailingExtraLine);
     return aggregatedNotebookData;
   }
 
@@ -121,22 +124,34 @@ private static void skipNestedObjects(JsonParser parser) throws IOException {
     }
   }
 
-  private Optional<NotebookParsingData> processCodeCell(int startLine, JsonParser jParser) throws IOException {
+  private static boolean processCodeCellType(JsonParser jParser) throws IOException {
+    if (JsonToken.FIELD_NAME.equals(jParser.currentToken()) && "cell_type".equals(jParser.currentName())) {
+      jParser.nextToken();
+      if ("code".equals(jParser.getValueAsString())) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private void processCodeCell(List<NotebookParsingData> accumulator, JsonParser jParser) throws IOException {
     boolean isCodeCell = false;
     Optional<NotebookParsingData> notebookData = Optional.empty();
     while (jParser.nextToken() != JsonToken.END_OBJECT) {
 
       skipNestedObjects(jParser);
 
-      if (JsonToken.FIELD_NAME.equals(jParser.currentToken()) && "cell_type".equals(jParser.currentName())) {
-        jParser.nextToken();
-        String cellType = jParser.getValueAsString();
-        if ("code".equals(cellType)) {
-          isCodeCell = true;
-        }
+      if (!isCodeCell) {
+        isCodeCell = processCodeCellType(jParser);
       }
+
       if (JsonToken.FIELD_NAME.equals(jParser.currentToken()) && "source".equals(jParser.currentName())) {
         jParser.nextToken();
+
+        int startLine = 0;
+        if (!accumulator.isEmpty()) {
+          startLine = accumulator.get(accumulator.size() - 1).getAggregatedSourceLine();
+        }
         switch (jParser.currentToken()) {
           case START_ARRAY:
             notebookData = Optional.of(parseSourceArray(startLine, jParser));
@@ -149,14 +164,14 @@ private Optional<NotebookParsingData> processCodeCell(int startLine, JsonParser
         }
       }
     }
+
     if (isCodeCell && notebookData.isPresent()) {
-      lastPythonLine = notebookData.get().getAggregatedSourceLine();
-      return notebookData;
+      var data = notebookData.get();
+      lastPythonLine = data.getAggregatedSourceLine();
+      accumulator.add(data);
     }
-    return Optional.empty();
   }
 
-
   private static NotebookParsingData parseSourceArray(int startLine, JsonParser jParser) throws IOException {
     NotebookParsingData cellData = NotebookParsingData.fromLine(startLine);
     JsonLocation tokenLocation = jParser.currentTokenLocation();

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

@@ -59,10 +59,11 @@ public Integer getAggregatedSourceLine() {
     return aggregatedSourceLine;
   }
 
-  public void combine(NotebookParsingData other) {
+  public NotebookParsingData combine(NotebookParsingData other) {
     aggregatedSource.append(other.aggregatedSource);
     aggregatedSourceLine = other.aggregatedSourceLine;
     locationMap.putAll(other.locationMap);
+    return this;
   }
 
   public void appendToSource(String str) {

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

@@ -44,38 +44,8 @@ void testParseNotebook() throws IOException {
 
     var result = resultOptional.get();
 
-    var pythonContent = """
-      x = None
-      if x is not None:
-          print \"not none\"
-      
-      
-      def foo():
-          x = 42
-          x = 17
-          print(x)
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
-      if x is not None:
-          print(\"hello\")
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
-      x = 42
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
-      #Some code
-      print(\"hello world\\n\") 
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
-      print(\"My\\ntext\") 
-      print(\"Something else\\n\")
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER 
-      print(\"My\\ntext\") 
-      print(\"Something else\\n\")
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
-      a = \"A bunch of characters \\n \\f \\r \\ 	 \"
-      b = None
-      #SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER""";
-
     assertThat(result.locationMap().keySet()).hasSize(27);
     assertThat(result.contents()).hasLineCount(27);
-    assertThat(result.contents()).isEqualTo(pythonContent);
     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, Map.of(-1, 0)));
@@ -134,18 +104,6 @@ void testParseMojoNotebook() {
     assertThat(resultOptional).isEmpty();
   }
 
-  @Test
-  void testParseNotebookWithSourceBeforeCellType() throws IOException {
-    var inputFile = createInputFile(baseDir, "notebook_source_before_cell_type.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
-
-    var resultOptional = IpynbNotebookParser.parseNotebook(inputFile);
-
-    assertThat(resultOptional).isPresent();
-    var result = resultOptional.get();
-    assertThat(result.locationMap()).hasSize(2);
-    assertThat(result.contents()).isEqualTo("x = None\n#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER");
-  }
-
   @Test
   void testParseNotebookWithNoLanguage() {
     var inputFile = createInputFile(baseDir, "notebook_no_language.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
@@ -182,4 +140,17 @@ void testParseNotebookSingleLine() throws IOException {
     // position of variable t
     assertThat(result.locationMap().get(4).column()).isEqualTo(451);
   }
+
+  @Test
+  void testParseNotebook1() throws IOException {
+    var inputFile = createInputFile(baseDir, "notebook_no_code.ipynb", InputFile.Status.CHANGED, InputFile.Type.MAIN);
+
+    var resultOptional = IpynbNotebookParser.parseNotebook(inputFile);
+
+    assertThat(resultOptional).isPresent();
+
+    var result = resultOptional.get();
+    assertThat(result.locationMap()).isEmpty();
+    assertThat(result.contents()).isEmpty();
+  }
 }

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

@@ -84,6 +84,11 @@ void testRemoveTrailingExtraLineDoesNothing() {
     assertThat(data).extracting(d -> d.getAggregatedSource().toString()).isEqualTo("First line");
     data.removeTrailingExtraLine();
     assertThat(data).extracting(d -> d.getAggregatedSource().toString()).isEqualTo("First line");
+
+    var emptyLines = new NotebookParsingData(new StringBuilder(), new LinkedHashMap<>(), 5);
+    assertThat(emptyLines).extracting(d -> d.getAggregatedSource().toString()).isEqualTo("");
+    emptyLines.removeTrailingExtraLine();
+    assertThat(emptyLines).extracting(d -> d.getAggregatedSource().toString()).isEqualTo("");
   }
 
   @Test

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

@@ -0,0 +1,18 @@
+{
+ "cells": [
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "## Markdown"
+   ]
+  }
+ ],
+ "metadata": {
+  "language_info": {
+   "name": "python"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 2
+}