SONAR-17564 fix Sarif region import

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/RegionMapper.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/RegionMapper.java
index 35001e0aabff548e0dd71777c969d98d556f1f2e..6d9a466e77e3ff31490b041e0cc15d35a9042174 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/RegionMapper.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/RegionMapper.java
@@ -37,12 +37,16 @@ public class RegionMapper {
     int startLine = Objects.requireNonNull(region.getStartLine(), "No start line defined for the region.");
     Integer endLine = region.getEndLine();
     if (endLine != null) {
-      int startColumn = Optional.ofNullable(region.getStartColumn()).orElse(1);
-      int endColumn = Optional.ofNullable(region.getEndColumn())
+      int startColumn = Optional.ofNullable(region.getStartColumn()).map(RegionMapper::adjustSarifColumnIndexToSqIndex).orElse(0);
+      int endColumn = Optional.ofNullable(region.getEndColumn()).map(RegionMapper::adjustSarifColumnIndexToSqIndex)
         .orElseGet(() -> file.selectLine(endLine).end().lineOffset());
       return Optional.of(file.newRange(startLine, startColumn, endLine, endColumn));
     } else {
       return Optional.of(file.selectLine(startLine));
     }
   }
+
+  private static int adjustSarifColumnIndexToSqIndex(int index) {
+    return index - 1;
+  }
 }

diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/RegionMapperTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/RegionMapperTest.java
index 27a973a15cb790d97d734436a825eac82e24bb7e..bc8d3e6255f0eba86868b57a2f53d15b6e2f6636 100644 (file)
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/RegionMapperTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/RegionMapperTest.java
@@ -19,15 +19,19 @@
  */
 package org.sonar.scanner.externalissue.sarif;
 
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.nio.file.Paths;
 import java.util.Optional;
 import java.util.stream.IntStream;
 import javax.annotation.Nullable;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.MockitoAnnotations;
 import org.sonar.api.batch.fs.TextRange;
 import org.sonar.api.batch.fs.internal.DefaultIndexedFile;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
@@ -35,12 +39,12 @@ import org.sonar.api.batch.fs.internal.Metadata;
 import org.sonar.core.sarif.Region;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.assertj.core.api.Assertions.assertThatNullPointerException;
+import static org.assertj.core.api.Fail.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-@RunWith(MockitoJUnitRunner.class)
+@RunWith(DataProviderRunner.class)
 public class RegionMapperTest {
   private static final int LINE_END_OFFSET = 10;
   private static final DefaultInputFile INPUT_FILE = new DefaultInputFile(new DefaultIndexedFile("ABCDE", Paths.get("module"), "relative/path", null),
@@ -61,6 +65,11 @@ public class RegionMapperTest {
   @InjectMocks
   private RegionMapper regionMapper;
 
+  @Before
+  public void setup() {
+    MockitoAnnotations.openMocks(this);
+  }
+
   @Test
   public void mapRegion_whenNullRegion_returnsEmpty() {
     assertThat(regionMapper.mapRegion(null, INPUT_FILE)).isEmpty();
@@ -75,6 +84,25 @@ public class RegionMapperTest {
       .withMessage("No start line defined for the region.");
   }
 
+  @Test
+  @UseDataProvider("index")
+  public void mapRegion_whenColumnsDefined_convert1BasedIndexTo0BasedIndex(int startColumn, int endColumn, int startOffsetExpected, int endOffsetExpected) {
+    Region fullRegion = mockRegion(startColumn, endColumn, 3, 4);
+
+    TextRange textRange = regionMapper.mapRegion(fullRegion, INPUT_FILE).orElseGet(() -> fail("No TextRange"));
+
+    assertThat(textRange.start().lineOffset()).isEqualTo(startOffsetExpected);
+    assertThat(textRange.end().lineOffset()).isEqualTo(endOffsetExpected);
+  }
+
+  @DataProvider
+  public static Object[][] index() {
+    return new Object[][]{
+      {1,3, 0,2},
+      {5,8, 4,7}
+    };
+  }
+
   @Test
   public void mapRegion_whenAllCoordinatesDefined() {
     Region fullRegion = mockRegion(1, 2, 3, 4);
@@ -84,9 +112,9 @@ public class RegionMapperTest {
     assertThat(optTextRange).isPresent();
     TextRange textRange = optTextRange.get();
     assertThat(textRange.start().line()).isEqualTo(fullRegion.getStartLine());
-    assertThat(textRange.start().lineOffset()).isEqualTo(fullRegion.getStartColumn());
+    assertThat(textRange.start().lineOffset()).isZero();
     assertThat(textRange.end().line()).isEqualTo(fullRegion.getEndLine());
-    assertThat(textRange.end().lineOffset()).isEqualTo(fullRegion.getEndColumn());
+    assertThat(textRange.end().lineOffset()).isEqualTo(1);
   }
 
   @Test
@@ -98,7 +126,7 @@ public class RegionMapperTest {
     assertThat(optTextRange).isPresent();
     TextRange textRange = optTextRange.get();
     assertThat(textRange.start().line()).isEqualTo(fullRegion.getStartLine());
-    assertThat(textRange.start().lineOffset()).isEqualTo(1);
+    assertThat(textRange.start().lineOffset()).isZero();
     assertThat(textRange.end().line()).isEqualTo(fullRegion.getEndLine());
     assertThat(textRange.end().lineOffset()).isEqualTo(LINE_END_OFFSET);
   }
@@ -112,7 +140,7 @@ public class RegionMapperTest {
     assertThat(optTextRange).isPresent();
     TextRange textRange = optTextRange.get();
     assertThat(textRange.start().line()).isEqualTo(fullRegion.getStartLine());
-    assertThat(textRange.start().lineOffset()).isEqualTo(fullRegion.getStartColumn());
+    assertThat(textRange.start().lineOffset()).isEqualTo(7);
     assertThat(textRange.end().line()).isEqualTo(fullRegion.getEndLine());
     assertThat(textRange.end().lineOffset()).isEqualTo(LINE_END_OFFSET);
   }
@@ -126,9 +154,9 @@ public class RegionMapperTest {
     assertThat(optTextRange).isPresent();
     TextRange textRange = optTextRange.get();
     assertThat(textRange.start().line()).isEqualTo(fullRegion.getStartLine());
-    assertThat(textRange.start().lineOffset()).isEqualTo(1);
+    assertThat(textRange.start().lineOffset()).isZero();
     assertThat(textRange.end().line()).isEqualTo(fullRegion.getEndLine());
-    assertThat(textRange.end().lineOffset()).isEqualTo(fullRegion.getEndLine());
+    assertThat(textRange.end().lineOffset()).isEqualTo(7);
   }
 
   private static Region mockRegion(@Nullable Integer startColumn, @Nullable Integer endColumn, @Nullable Integer startLine, @Nullable Integer endLine) {