Skip to content

Commit

Permalink
[QD-7821] Ensure that newly generated rule lists are mutable
Browse files Browse the repository at this point in the history
While `Arrays.asList()` creates an `ArrayList`, this is NOT a
`java.util.ArrayList` - and is in fact immutable.
Update the tests to catch this
  • Loading branch information
jckoenen committed Dec 5, 2023
1 parent 900577b commit cc5d317
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,11 @@ void addTo(Run run) {
getOrCreate(e::getRules, e::setRules, ArrayList::new)
.add(descriptor);
} else {
// Collections.singletonList() is immutable
//noinspection ArraysAsListWithZeroOrOneArgument
getOrCreate(tool::getExtensions, tool::setExtensions, HashSet::new)
// we don't want to copy all rules from the source AND not override them
.add(location.shallowCopy()
.withIsComprehensive(false)
.withRules(Arrays.asList(descriptor))
.withRules(new ArrayList<>(Collections.singletonList(descriptor)))
);
}

Expand Down
23 changes: 14 additions & 9 deletions sarif/src/test/java/com/jetbrains/qodana/sarif/BaselineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.stream.Collectors;

import static com.jetbrains.qodana.sarif.baseline.BaselineCalculation.Options.DEFAULT;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;

public class BaselineTest {
Expand Down Expand Up @@ -177,7 +178,7 @@ public void testAbsentResultWithChangedIdAndSameVersion() throws IOException {
SarifReport report = readReport("src/test/resources/testData/AbsentBaselineTest/report.json");
SarifReport baseline = readReport("src/test/resources/testData/AbsentBaselineTest/baseline.json");

doTest(report, baseline, 17, 1, 1, new BaselineCalculation.Options(true));
doTest(report, baseline, 1, 17, 18, new BaselineCalculation.Options(true));

Set<String> knownDescriptorIds = RuleUtil.allRules(report)
.map(ReportingDescriptor::getId)
Expand All @@ -198,7 +199,7 @@ public void testAbsentResultWithChangedIdAndOldVersion() throws IOException {
SarifReport report = readReport("src/test/resources/testData/AbsentBaselineTest/report.json");
SarifReport baseline = readReport("src/test/resources/testData/AbsentBaselineTest/baseline_old.json");

doTest(report, baseline, 17, 1, 1, new BaselineCalculation.Options(true));
doTest(report, baseline, 0, 18, 19, new BaselineCalculation.Options(true));

Set<String> knownDescriptorIds = RuleUtil.allRules(report)
.map(ReportingDescriptor::getId)
Expand All @@ -214,7 +215,6 @@ public void testAbsentResultWithChangedIdAndOldVersion() throws IOException {
assertEquals(new ArrayList<String>(), withoutDescriptor);
}


private void doTest(SarifReport report,
SarifReport baseline,
int expectedUnchanged,
Expand All @@ -223,9 +223,12 @@ private void doTest(SarifReport report,
BaselineCalculation.Options options
) {
BaselineCalculation calculation = BaselineCalculation.compare(report, baseline, options);
assertEquals(expectedUnchanged, calculation.getUnchangedResults(), "Unchanged:");
assertEquals(expectedAbsent, calculation.getAbsentResults(), "Absent:");
assertEquals(expectedNew, calculation.getNewResults(), "New:");
assertAll(
() -> assertEquals(expectedUnchanged, calculation.getUnchangedResults(), "Unchanged:"),
() -> assertEquals(expectedAbsent, calculation.getAbsentResults(), "Absent:"),
() -> assertEquals(expectedNew, calculation.getNewResults(), "New:")
);

List<Result> results = report.getRuns().get(0).getResults();

if (!options.isFillBaselineState()) {
Expand All @@ -242,9 +245,11 @@ private void doTest(SarifReport report,
List<Result> resultsUnchanged = grouped.get(Result.BaselineState.UNCHANGED);
List<Result> resultsAbsent = grouped.get(Result.BaselineState.ABSENT);
List<Result> resultsNew = grouped.get(Result.BaselineState.NEW);
assertEquals(expectedUnchanged, resultsUnchanged == null ? 0 : resultsUnchanged.size(), "Unchanged:");
assertEquals(expectedAbsent, resultsAbsent == null ? 0 : resultsAbsent.size(), "Absent:");
assertEquals(expectedNew, resultsNew == null ? 0 : resultsNew.size(), "New:");
assertAll(
() -> assertEquals(expectedUnchanged, resultsUnchanged == null ? 0 : resultsUnchanged.size(), "Unchanged:"),
() -> assertEquals(expectedAbsent, resultsAbsent == null ? 0 : resultsAbsent.size(), "Absent:"),
() -> assertEquals(expectedNew, resultsNew == null ? 0 : resultsNew.size(), "New:")
);
}

private void doTest(SarifReport report,
Expand Down
Git LFS file not shown
Git LFS file not shown

0 comments on commit cc5d317

Please sign in to comment.