Skip to content

Commit

Permalink
Do not use for baseline comparing a logical location if a result prov…
Browse files Browse the repository at this point in the history
…ides a physical location
  • Loading branch information
avafanasiev committed Jul 15, 2024
1 parent 97a63ba commit 505d3e5
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ private boolean equalsLocation(Location location, Location oLocation) {
if (location == null || oLocation == null) return location == oLocation;

if (!equalsPhysicalLocation(location.getPhysicalLocation(), oLocation.getPhysicalLocation())) return false;
if (location.getPhysicalLocation() != null) return true;
Set<LogicalLocation> locations = location.getLogicalLocations();
Set<LogicalLocation> oLocations = oLocation.getLogicalLocations();
if (locations == null || oLocations == null) return locations == oLocations;
Expand Down Expand Up @@ -104,11 +105,15 @@ public int hashCode() {

if (result.getLocations() == null) return hash;
for (Location location : result.getLocations()) {
hash = ((hash * 31) + hashPhysicalLocation(location.getPhysicalLocation()));
Set<LogicalLocation> logicalLocations = location.getLogicalLocations();
if (logicalLocations != null) {
for (LogicalLocation logicalLocation : logicalLocations) {
hash = ((hash * 31) + hashLogicalLocation(logicalLocation));
PhysicalLocation physicalLocation = location.getPhysicalLocation();
if (physicalLocation != null) {
hash = ((hash * 31) + hashPhysicalLocation(physicalLocation));
} else {
Set<LogicalLocation> logicalLocations = location.getLogicalLocations();
if (logicalLocations != null) {
for (LogicalLocation logicalLocation : logicalLocations) {
hash = ((hash * 31) + hashLogicalLocation(logicalLocation));
}
}
}
}
Expand Down
98 changes: 83 additions & 15 deletions sarif/src/test/java/com/jetbrains/qodana/sarif/BaselineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.jetbrains.qodana.sarif.baseline.BaselineCalculation;
import com.jetbrains.qodana.sarif.model.*;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import java.io.IOException;
Expand All @@ -12,15 +11,32 @@
import java.util.stream.Collectors;

import static com.jetbrains.qodana.sarif.baseline.BaselineCalculation.Options.DEFAULT;
import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;

@SuppressWarnings({"OptionalGetWithoutIsPresent", "SameParameterValue"})
public class BaselineTest {

private static final String QODANA_REPORT_JSON = "src/test/resources/testData/readWriteTest/qodanaReport.json";
private static final String QODANA_REPORT_JSON_2 = "src/test/resources/testData/readWriteTest/qodanaReport2.json";

private static final BaselineCalculation.Options INCLUDE_ABSENT = new BaselineCalculation.Options(true);

private static int problemsCount(SarifReport report) {
return report.getRuns().stream().mapToInt(it -> it.getResults().size()).sum();
}

private static SarifReport readReport() throws IOException {
return readReport(QODANA_REPORT_JSON);
}

private static SarifReport readReport(String path) throws IOException {
Path reportPath = Paths.get(path);
return SarifUtil.readReport(reportPath);
}

@Test
public void testSameReport() throws IOException {
SarifReport report = readReport();
Expand Down Expand Up @@ -165,7 +181,6 @@ public void testIncrementalBaselineOneAbsentOneNew() throws IOException {
assertEquals(Result.BaselineState.UNCHANGED, result4.getBaselineState());
}


@Test
public void testDifferentToolName() throws IOException {
SarifReport report = readReport();
Expand Down Expand Up @@ -217,6 +232,72 @@ public void testAbsentResultWithChangedIdAndOldVersion() throws IOException {
assertEquals(new ArrayList<String>(), withoutDescriptor);
}

@Test
public void testDoNotUseLogicalLocationsIfProblemHasPhysical() {
SarifReport report = newReport();
SarifReport baseline = newReport();

Set<LogicalLocation> logicalLocations =
report.getRuns().get(0).getResults().get(0).getLocations().get(0).getLogicalLocations();
logicalLocations.stream().findFirst().get().withName("new name");

doTest(report, baseline, 1, 0, 0, INCLUDE_ABSENT);
}

@Test
public void testComparePhysicalEvenIfOneReportHasNotValue() {
SarifReport report = newReport();
SarifReport baseline = newReport();
report.getRuns().get(0).getResults().get(0).getLocations().get(0).withPhysicalLocation(null);

doTest(report, baseline, 0, 1, 1, INCLUDE_ABSENT);
}

@Test
public void testProblemHasNotPhysicalLocations() {
SarifReport report = newReport();
SarifReport baseline = newReport();
report.getRuns().get(0).getResults().get(0).getLocations().get(0).withPhysicalLocation(null);
baseline.getRuns().get(0).getResults().get(0).getLocations().get(0).withPhysicalLocation(null);

doTest(report, baseline, 1, 0, 0, INCLUDE_ABSENT);
}

@Test
public void testDoUseLogicalLocationsIfProblemHasNotPhysical() {
SarifReport report = newReport();
SarifReport baseline = newReport();
report.getRuns().get(0).getResults().get(0).getLocations().get(0).withPhysicalLocation(null);
baseline.getRuns().get(0).getResults().get(0).getLocations().get(0).withPhysicalLocation(null);

Set<LogicalLocation> logicalLocations =
report.getRuns().get(0).getResults().get(0).getLocations().get(0).getLogicalLocations();
logicalLocations.stream().findFirst().get().withName("new name");
doTest(report, baseline, 0, 1, 1, INCLUDE_ABSENT);
}

private SarifReport newReport() {
return new SarifReport()
.withRuns(
singletonList(new Run()
.withResults(singletonList(newResult()))
)
);
}

private Result newResult() {
return new Result(new Message().withText("message"))
.withLocations(singletonList(
new Location()
.withPhysicalLocation(new PhysicalLocation()
.withArtifactLocation(new ArtifactLocation().withUri("path/file.txt"))
)
.withLogicalLocations(
singleton(new LogicalLocation().withName("name").withKind("module"))
)
));
}

private void doTest(SarifReport report,
SarifReport baseline,
int expectedUnchanged,
Expand Down Expand Up @@ -262,17 +343,4 @@ private void doTest(SarifReport report,
) {
doTest(report, baseline, expectedUnchanged, expectedAbsent, expectedNew, DEFAULT);
}

private static int problemsCount(SarifReport report) {
return report.getRuns().stream().mapToInt(it -> it.getResults().size()).sum();
}

private static SarifReport readReport() throws IOException {
return readReport(QODANA_REPORT_JSON);
}

private static SarifReport readReport(String path) throws IOException {
Path reportPath = Paths.get(path);
return SarifUtil.readReport(reportPath);
}
}

0 comments on commit 505d3e5

Please sign in to comment.