SLLS-265 fix Jupyter document line indexing

AmadeusITGroup · Sep 23, 2024 · 1419811 · 1419811
1 parent 4de1d2f
commit 1419811
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 6 deletions.
diff --git a/src/main/java/org/sonarsource/sonarlint/ls/AnalysisHelper.java b/src/main/java/org/sonarsource/sonarlint/ls/AnalysisHelper.java
@@ -102,8 +102,12 @@ public void handleIssues(Map<URI, List<RaisedFindingDto>> issuesByFileUri) {
     issuesCache.reportIssues(issuesByFileUri);
     issuesByFileUri.forEach((uri, issues) -> {
       diagnosticPublisher.publishDiagnostics(uri, true);
-      notebookDiagnosticPublisher.cleanupDiagnosticsForCellsWithoutIssues(uri);
-      openNotebooksCache.getFile(uri).ifPresent(notebook -> notebookDiagnosticPublisher.publishNotebookDiagnostics(uri, notebook));
+      openNotebooksCache.getFile(uri).ifPresent(notebook -> {
+        // clean up old diagnostics
+        notebookDiagnosticPublisher.cleanupCellsList(uri);
+        notebookDiagnosticPublisher.cleanupDiagnosticsForCellsWithoutIssues(uri);
+        notebookDiagnosticPublisher.publishNotebookDiagnostics(uri, notebook);
+      });
     });
   }
 

diff --git a/src/main/java/org/sonarsource/sonarlint/ls/notebooks/VersionedOpenNotebook.java b/src/main/java/org/sonarsource/sonarlint/ls/notebooks/VersionedOpenNotebook.java
@@ -69,9 +69,6 @@ private VersionedOpenNotebook(URI uri, int version, List<TextDocumentItem> cells
   }
 
   private void indexCellsByLineNumber() {
-    if (notebookVersion.equals(indexedNotebookVersion)) {
-      return;
-    }
     var lineCount = 1;
     var cellCount = 1;
     for (var cell : orderedCells) {
@@ -117,7 +114,6 @@ public Set<String> getCellUris() {
   }
 
   public Optional<URI> getCellUri(int lineNumber) {
-    indexCellsByLineNumber();
     return Optional.ofNullable(fileLineToCell.get(lineNumber))
       .map(TextDocumentItem::getUri)
       .map(URI::create);

diff --git a/src/test/java/org/sonarsource/sonarlint/ls/notebooks/VersionedOpenNotebookTests.java b/src/test/java/org/sonarsource/sonarlint/ls/notebooks/VersionedOpenNotebookTests.java
@@ -20,8 +20,10 @@
 package org.sonarsource.sonarlint.ls.notebooks;
 
 import java.net.URI;
+import java.time.Instant;
 import java.util.Collections;
 import java.util.List;
+import java.util.UUID;
 import javax.annotation.Nullable;
 import org.eclipse.lsp4j.NotebookCellArrayChange;
 import org.eclipse.lsp4j.NotebookDocumentChangeEvent;
@@ -40,10 +42,14 @@
 import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.IssueFlowDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.QuickFixDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.RaisedFindingDto;
+import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.RaisedIssueDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.TextEditDto;
+import org.sonarsource.sonarlint.core.rpc.protocol.common.CleanCodeAttribute;
 import org.sonarsource.sonarlint.core.rpc.protocol.common.IssueSeverity;
+import org.sonarsource.sonarlint.core.rpc.protocol.common.RuleType;
 import org.sonarsource.sonarlint.core.rpc.protocol.common.TextRangeDto;
 import org.sonarsource.sonarlint.ls.connected.DelegatingFinding;
+import org.sonarsource.sonarlint.ls.connected.DelegatingIssue;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
@@ -110,6 +116,8 @@ void shouldCorrectlyMapFileLineToCellLine() {
     var tmpUri = URI.create("file:///some/notebook.ipynb");
     var underTest = createTestNotebookWithThreeCells(tmpUri);
 
+    manuallyReindexCellLines(underTest);
+
     assertThat(underTest.getCellUri(1)).contains(URI.create(tmpUri + "#cell1"));
     assertThat(underTest.getCellUri(2)).contains(URI.create(tmpUri + "#cell1"));
     assertThat(underTest.getCellUri(3)).contains(URI.create(tmpUri + "#cell1"));
@@ -173,6 +181,8 @@ void shouldConvertQuickFixWithSingleFileEdit() {
     var fakeNotebook = createTestNotebookWithThreeCells(tmpUri);
     var quickFixTextRange = new TextRangeDto(9, 0, 9, 2);
 
+    manuallyReindexCellLines(fakeNotebook);
+
     var textEdit = mock(TextEditDto.class);
     when(textEdit.newText()).thenReturn("");
     when(textEdit.range()).thenReturn(quickFixTextRange);
@@ -211,6 +221,8 @@ void shouldConvertQuickFixWithMultipleFileEdits() {
     var quickFixTextRange1 = new TextRangeDto(9, 0, 9, 2);
     var quickFixTextRange2 = new TextRangeDto(5, 0, 6, 2);
 
+    manuallyReindexCellLines(fakeNotebook);
+
     var textEdit1 = mock(org.sonarsource.sonarlint.core.rpc.protocol.client.issue.TextEditDto.class);
     when(textEdit1.newText()).thenReturn("");
     when(textEdit1.range()).thenReturn(quickFixTextRange1);
@@ -308,6 +320,8 @@ void shouldHandleCellCreation() {
 
     underTest.didChange(2, changeEvent);
 
+    manuallyReindexCellLines(underTest);
+
     assertThat(underTest.getNotebookVersion()).isEqualTo(2);
     assertThat(underTest.getCellUris()).hasSize(4);
     assertThat(underTest.getContent()).isEqualTo("" +
@@ -350,6 +364,8 @@ void shouldHandleCellDeletion() {
 
     underTest.didChange(2, changeEvent);
 
+    manuallyReindexCellLines(underTest);
+
     assertThat(underTest.getNotebookVersion()).isEqualTo(2);
     assertThat(underTest.getCellUris()).hasSize(2);
     assertThat(underTest.getContent()).isEqualTo("" +
@@ -382,6 +398,7 @@ void shouldNotFailWhenRemovingNonexistentCell() {
     changeEvent.setCells(changeEventCells);
 
     underTest.didChange(2, changeEvent);
+    manuallyReindexCellLines(underTest);
 
     assertThat(underTest.getNotebookVersion()).isEqualTo(2);
     assertThat(underTest.getCellUris()).hasSize(3);
@@ -494,4 +511,25 @@ public static VersionedOpenNotebook createTestNotebookWithThreeCells(URI tmpUri)
     return VersionedOpenNotebook.create(tmpUri, 1, List.of(cell1, cell2, cell3), mock(NotebookDiagnosticPublisher.class));
   }
 
+  private void manuallyReindexCellLines(VersionedOpenNotebook underTest) {
+    var fakeFindingDto = new RaisedIssueDto(
+      UUID.randomUUID(),
+      null,
+      "ruleKey",
+      "message",
+      IssueSeverity.BLOCKER,
+      RuleType.BUG,
+      CleanCodeAttribute.TRUSTWORTHY,
+      List.of(),
+      Instant.now(),
+      true,
+      false,
+      new TextRangeDto(1, 0, 1, 0),
+      List.of(),
+      List.of(),
+      null
+    );
+    underTest.toCellIssue(new DelegatingIssue(fakeFindingDto, URI.create("")));
+  }
+
 }