From 3ce1c4cb8763cd283dbcb99e2f89c2d993b76a2e Mon Sep 17 00:00:00 2001 From: Javier García Orduña Date: Fri, 20 Dec 2024 17:13:33 +0100 Subject: SONAR-20659 Add logs when importing SARIF report if the referenced files location cannot be resolved Co-authored-by: antoine.vinot --- .../externalissue/sarif/LocationMapper.java | 12 +++++- .../externalissue/sarif/LocationMapperTest.java | 45 ++++++++++++---------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/LocationMapper.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/LocationMapper.java index 97093c2e0a9..8ccb2290881 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/LocationMapper.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/LocationMapper.java @@ -33,10 +33,13 @@ import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.predicates.AbstractFilePredicate; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.issue.NewIssueLocation; +import org.sonar.api.notifications.AnalysisWarnings; import org.sonar.api.scanner.ScannerSide; import org.sonar.sarif.pojo.ArtifactLocation; import org.sonar.sarif.pojo.Location; @@ -46,11 +49,14 @@ import static org.sonar.api.utils.Preconditions.checkArgument; @ScannerSide public class LocationMapper { + private static final Logger LOG = LoggerFactory.getLogger(LocationMapper.class); + private static final int CACHE_SIZE = 500; private static final int CACHE_EXPIRY = 60; private final SensorContext sensorContext; private final RegionMapper regionMapper; + private final AnalysisWarnings analysisWarnings; LoadingCache> inputFileCache = CacheBuilder.newBuilder() .maximumSize(CACHE_SIZE) @@ -58,9 +64,10 @@ public class LocationMapper { .concurrencyLevel(Runtime.getRuntime().availableProcessors()) .build(getCacheLoader()); - LocationMapper(SensorContext sensorContext, RegionMapper regionMapper) { + LocationMapper(SensorContext sensorContext, RegionMapper regionMapper, AnalysisWarnings analysisWarnings) { this.sensorContext = sensorContext; this.regionMapper = regionMapper; + this.analysisWarnings = analysisWarnings; } void fillIssueInProjectLocation(NewIssueLocation newIssueLocation) { @@ -74,6 +81,9 @@ public class LocationMapper { String fileUri = getFileUriOrThrow(location); Optional file = findFile(fileUri); if (file.isEmpty()) { + String unresolvedLocationWarning = String.format("Unable to resolve Issue location from SARIF physical location %s. Falling back to the project location.", fileUri); + analysisWarnings.addUnique(unresolvedLocationWarning); + LOG.warn(unresolvedLocationWarning); return false; } InputFile inputFile = file.get(); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/LocationMapperTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/LocationMapperTest.java index cc8d96e11ad..01a9222699a 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/LocationMapperTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/LocationMapperTest.java @@ -19,14 +19,13 @@ */ package org.sonar.scanner.externalissue.sarif; -import com.tngtech.java.junit.dataprovider.DataProvider; -import com.tngtech.java.junit.dataprovider.DataProviderRunner; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Answers; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -36,6 +35,7 @@ import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.TextRange; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.issue.NewIssueLocation; +import org.sonar.api.notifications.AnalysisWarnings; import org.sonar.api.scanner.fs.InputProject; import org.sonar.sarif.pojo.Location; @@ -48,8 +48,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -@RunWith(DataProviderRunner.class) -public class LocationMapperTest { +class LocationMapperTest { private static final String URI_TEST = "URI_TEST"; private static final String EXPECTED_MESSAGE_URI_MISSING = "The field location.physicalLocation.artifactLocation.uri is not set."; @@ -60,6 +59,9 @@ public class LocationMapperTest { @Mock private RegionMapper regionMapper; + @Mock + private AnalysisWarnings analysisWarnings; + @InjectMocks private LocationMapper locationMapper; @@ -72,8 +74,8 @@ public class LocationMapperTest { @Mock private InputFile inputFile; - @Before - public void setup() { + @BeforeEach + void setup() { MockitoAnnotations.openMocks(this); when(newIssueLocation.on(any())).thenReturn(newIssueLocation); when(newIssueLocation.at(any())).thenReturn(newIssueLocation); @@ -84,7 +86,7 @@ public class LocationMapperTest { } @Test - public void isPredicate_whenDifferentFile_returnsFalse() { + void isPredicate_whenDifferentFile_returnsFalse() { Path path = Paths.get("file"); when(inputFile.path()).thenReturn(Paths.get("file2")); LocationMapper.IsPredicate isPredicate = new LocationMapper.IsPredicate(path); @@ -92,7 +94,7 @@ public class LocationMapperTest { } @Test - public void isPredicate_whenSameFile_returnsTrue() { + void isPredicate_whenSameFile_returnsTrue() { Path path = Paths.get("file"); when(inputFile.path()).thenReturn(path); LocationMapper.IsPredicate isPredicate = new LocationMapper.IsPredicate(path); @@ -100,16 +102,17 @@ public class LocationMapperTest { } @Test - public void fillIssueInFileLocation_whenFileNotFound_returnsFalse() { + void fillIssueInFileLocation_whenFileNotFound_returnsFalseAndAddsWarningMessage() { when(sensorContext.fileSystem().inputFile(any())).thenReturn(null); boolean success = locationMapper.fillIssueInFileLocation(newIssueLocation, location); assertThat(success).isFalse(); + verify(analysisWarnings).addUnique(String.format("Unable to resolve Issue location from SARIF physical location %s. Falling back to the project location.", URI_TEST)); } @Test - public void fillIssueInFileLocation_whenMapRegionReturnsNull_onlyFillsSimpleFields() { + void fillIssueInFileLocation_whenMapRegionReturnsNull_onlyFillsSimpleFields() { when(regionMapper.mapRegion(location.getPhysicalLocation().getRegion(), inputFile)) .thenReturn(Optional.empty()); @@ -121,7 +124,7 @@ public class LocationMapperTest { } @Test - public void fillIssueInFileLocation_whenMapRegionReturnsRegion_callsAt() { + void fillIssueInFileLocation_whenMapRegionReturnsRegion_callsAt() { TextRange textRange = mock(TextRange.class); when(regionMapper.mapRegion(location.getPhysicalLocation().getRegion(), inputFile)) .thenReturn(Optional.of(textRange)); @@ -135,7 +138,7 @@ public class LocationMapperTest { } @Test - public void fillIssueInFileLocation_ifNullUri_throws() { + void fillIssueInFileLocation_ifNullUri_throws() { when(location.getPhysicalLocation().getArtifactLocation().getUri()).thenReturn(null); assertThatIllegalArgumentException() @@ -143,9 +146,9 @@ public class LocationMapperTest { .withMessage(EXPECTED_MESSAGE_URI_MISSING); } - @Test - @DataProvider({"file:///", "file:///path/", "file://host/", "file://host/path/", "file:////server/"}) - public void fillIssueInFileLocation_ifCorrectUriWithFileScheme_returnsTrue(String uriPrefix) { + @ParameterizedTest + @ValueSource(strings = {"file:///", "file:///path/", "file://host/", "file://host/path/", "file:////server/"}) + void fillIssueInFileLocation_ifCorrectUriWithFileScheme_returnsTrue(String uriPrefix) { when(location.getPhysicalLocation().getArtifactLocation().getUri()).thenReturn(uriPrefix + URI_TEST); boolean success = locationMapper.fillIssueInFileLocation(newIssueLocation, location); @@ -156,7 +159,7 @@ public class LocationMapperTest { } @Test - public void fillIssueInFileLocation_ifIncorrectUriWithFileScheme_throws() { + void fillIssueInFileLocation_ifIncorrectUriWithFileScheme_throws() { when(location.getPhysicalLocation().getArtifactLocation().getUri()).thenReturn("file://" + URI_TEST); assertThatThrownBy(() -> locationMapper.fillIssueInFileLocation(newIssueLocation, location)) @@ -164,7 +167,7 @@ public class LocationMapperTest { } @Test - public void fillIssueInFileLocation_ifNullArtifactLocation_throws() { + void fillIssueInFileLocation_ifNullArtifactLocation_throws() { when(location.getPhysicalLocation().getArtifactLocation()).thenReturn(null); assertThatIllegalArgumentException() @@ -173,7 +176,7 @@ public class LocationMapperTest { } @Test - public void fillIssueInFileLocation_ifNullPhysicalLocation_throws() { + void fillIssueInFileLocation_ifNullPhysicalLocation_throws() { when(location.getPhysicalLocation()).thenReturn(null); assertThatIllegalArgumentException() -- cgit v1.2.3