From 6895a5b7e08fc745f92f19f56723b4e15d91e187 Mon Sep 17 00:00:00 2001 From: Benjamin Campomenosi <109955405+benjamin-campomenosi-sonarsource@users.noreply.github.com> Date: Wed, 7 Dec 2022 14:52:46 +0100 Subject: [PATCH] SONAR-17564 fix Sarif region import --- .../externalissue/sarif/RegionMapper.java | 8 +++- .../externalissue/sarif/RegionMapperTest.java | 46 +++++++++++++++---- 2 files changed, 43 insertions(+), 11 deletions(-) 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 35001e0aabf..6d9a466e77e 100644 --- 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 27a973a15cb..bc8d3e6255f 100644 --- 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) { -- 2.39.5