Analysis should not fail when highlighting has invalid text range (#4604

)

sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/AnalysisProcessor.java

@@ -191,10 +191,16 @@ private void saveIssues(List<Issue> issues) {
   private void saveHighlights(BridgeServer.Highlight[] highlights) {
     NewHighlighting highlighting = context.newHighlighting().onFile(file);
     for (BridgeServer.Highlight highlight : highlights) {
-      highlighting.highlight(
-        highlight.location.toTextRange(file),
-        TypeOfText.valueOf(highlight.textType)
-      );
+      try {
+        highlighting.highlight(
+          highlight.location.toTextRange(file),
+          TypeOfText.valueOf(highlight.textType)
+        );
+      } catch (IllegalArgumentException e) {
+        LOG.warn("Failed to save highlight in {} at {}", file.uri(), highlight.location);
+        LOG.warn("Exception cause", e);
+        // continue processing other highlights
+      }
     }
     highlighting.save();
   }
@@ -203,19 +209,30 @@ private void saveHighlightedSymbols(BridgeServer.HighlightedSymbol[] highlighted
     NewSymbolTable symbolTable = context.newSymbolTable().onFile(file);
     for (BridgeServer.HighlightedSymbol highlightedSymbol : highlightedSymbols) {
       BridgeServer.Location declaration = highlightedSymbol.declaration;
-      NewSymbol newSymbol = symbolTable.newSymbol(
-        declaration.startLine,
-        declaration.startCol,
-        declaration.endLine,
-        declaration.endCol
-      );
+      NewSymbol newSymbol;
+      try {
+        newSymbol =
+          symbolTable.newSymbol(
+            declaration.startLine,
+            declaration.startCol,
+            declaration.endLine,
+            declaration.endCol
+          );
+      } catch (IllegalArgumentException e) {
+        LOG.warn("Failed to create symbol declaration in {} at {}", file.uri(), declaration);
+        continue;
+      }
       for (BridgeServer.Location reference : highlightedSymbol.references) {
-        newSymbol.newReference(
-          reference.startLine,
-          reference.startCol,
-          reference.endLine,
-          reference.endCol
-        );
+        try {
+          newSymbol.newReference(
+            reference.startLine,
+            reference.startCol,
+            reference.endLine,
+            reference.endCol
+          );
+        } catch (IllegalArgumentException e) {
+          LOG.warn("Failed to create symbol reference in {} at {}", file.uri(), reference);
+        }
       }
     }
     symbolTable.save();

sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java

@@ -195,6 +195,15 @@ class Location {
     int endLine;
     int endCol;
 
+    public Location() {}
+
+    Location(int startLine, int startCol, int endLine, int endCol) {
+      this.startLine = startLine;
+      this.startCol = startCol;
+      this.endLine = endLine;
+      this.endCol = endCol;
+    }
+
     public int getStartLine() {
       return startLine;
     }
@@ -230,6 +239,11 @@ public void setEndCol(int endCol) {
     TextRange toTextRange(InputFile inputFile) {
       return inputFile.newRange(this.startLine, this.startCol, this.endLine, this.endCol);
     }
+
+    @Override
+    public String toString() {
+      return String.format("%d:%d-%d:%d", startLine, startCol, endLine, endCol);
+    }
   }
 
   class Metrics {

sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/AnalysisProcessorTest.java

@@ -0,0 +1,70 @@
+package org.sonar.plugins.javascript.bridge;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.nio.file.Path;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
+import org.sonar.api.batch.sensor.internal.SensorContextTester;
+import org.sonar.api.issue.NoSonarFilter;
+import org.sonar.api.measures.FileLinesContext;
+import org.sonar.api.measures.FileLinesContextFactory;
+import org.sonar.api.testfixtures.log.LogTesterJUnit5;
+
+class AnalysisProcessorTest {
+
+  @TempDir
+  Path baseDir;
+
+  @org.junit.jupiter.api.extension.RegisterExtension
+  LogTesterJUnit5 logTester = new LogTesterJUnit5();
+
+  @Test
+  void should_not_fail_when_invalid_range() {
+    var fileLinesContextFactory = mock(FileLinesContextFactory.class);
+    when(fileLinesContextFactory.createFor(any())).thenReturn(mock(FileLinesContext.class));
+    var processor = new AnalysisProcessor(mock(NoSonarFilter.class), fileLinesContextFactory);
+    var context = SensorContextTester.create(baseDir);
+    var file = TestInputFileBuilder
+      .create("moduleKey", "file.js")
+      .setContents("var x  = 1;")
+      .build();
+    var response = new BridgeServer.AnalysisResponse();
+    var highlight = new BridgeServer.Highlight();
+    highlight.location = new BridgeServer.Location(1, 2, 1, 1); // invalid range startCol > endCol
+    response.highlights = new BridgeServer.Highlight[] { highlight };
+    processor.processResponse(context, mock(JsTsChecks.class), file, response);
+    assertThat(logTester.logs())
+      .contains("Failed to save highlight in " + file.uri() + " at 1:2-1:1");
+  }
+
+  @Test
+  void should_not_fail_when_invalid_symbol() {
+    var fileLinesContextFactory = mock(FileLinesContextFactory.class);
+    when(fileLinesContextFactory.createFor(any())).thenReturn(mock(FileLinesContext.class));
+    var processor = new AnalysisProcessor(mock(NoSonarFilter.class), fileLinesContextFactory);
+    var context = SensorContextTester.create(baseDir);
+    var file = TestInputFileBuilder
+      .create("moduleKey", "file.js")
+      .setContents("var x  = 1;")
+      .build();
+    var response = new BridgeServer.AnalysisResponse();
+    var symbol = new BridgeServer.HighlightedSymbol();
+    symbol.declaration = new BridgeServer.Location(1, 2, 1, 1); // invalid range startCol > endCol
+    response.highlightedSymbols = new BridgeServer.HighlightedSymbol[] { symbol };
+    processor.processResponse(context, mock(JsTsChecks.class), file, response);
+    assertThat(logTester.logs())
+      .contains("Failed to create symbol declaration in " + file.uri() + " at 1:2-1:1");
+
+    context = SensorContextTester.create(baseDir);
+    symbol.declaration = new BridgeServer.Location(1, 1, 1, 2);
+    symbol.references = new BridgeServer.Location[] { new BridgeServer.Location(2, 2, 2, 1) };
+    processor.processResponse(context, mock(JsTsChecks.class), file, response);
+    assertThat(logTester.logs())
+      .contains("Failed to create symbol reference in " + file.uri() + " at 2:2-2:1");
+  }
+}