Skip to content

Commit

Permalink
Create shared utility for verifying fix locations (#358)
Browse files Browse the repository at this point in the history
The different codemod test mixins need to start being brought together,
and this is a small step towards that.

---------

Co-authored-by: Carlos Uscanga <[email protected]>
  • Loading branch information
nahsra and carlosu7 authored Apr 19, 2024
1 parent 189133b commit 7693903
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,15 @@ private CodemodFileScanningResult processXml(final Path file, final List<Result>
if (matchingResult.isPresent()) {
String id =
SarifFindingKeyUtil.buildFindingId(matchingResult.get(), file, line);
return CodemodChange.from(line, new FixedFinding(id, detectorRule()));
Integer sarifLine =
matchingResult
.get()
.getLocations()
.get(0)
.getPhysicalLocation()
.getRegion()
.getStartLine();
return CodemodChange.from(sarifLine, new FixedFinding(id, detectorRule()));
}
return CodemodChange.from(line);
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
import io.codemodder.testutils.Metadata;
import io.codemodder.testutils.RawFileCodemodTest;

/**
* This test needs to be fixed once this bug is addressed:
* https://github.com/pixee/codemodder-java/issues/359
*/
@Metadata(
codemodType = MavenSecureURLCodemod.class,
testResourceDir = "maven-non-https-url",
// expectingFixesAtLines = {22, 26, 30},
dependencies = {})
final class MavenSecureURLCodemodTest implements RawFileCodemodTest {}
Original file line number Diff line number Diff line change
Expand Up @@ -187,22 +187,8 @@ private void verifyCodemod(
assertThat(change.getDescription(), is(not(blankOrNullString())));
}

List<CodeTFChange> changes =
changeset.stream().map(CodeTFChangesetEntry::getChanges).flatMap(List::stream).toList();

if (codemod.getChanger() instanceof FixOnlyCodeChanger) {
assertThat(changes.stream().anyMatch(c -> !c.getFixedFindings().isEmpty()), is(true));
}

for (int expectedFixLine : expectedFixLines) {
assertThat(changes.stream().anyMatch(c -> c.getLineNumber() == expectedFixLine), is(true));
}

List<UnfixedFinding> unfixedFindings = result.getUnfixedFindings();
for (int expectedFailedFixLine : expectingFailedFixesAtLines) {
assertThat(
unfixedFindings.stream().noneMatch(c -> c.getLine() == expectedFailedFixLine), is(true));
}
ExpectedFixes.verifyExpectedFixes(
result, codemod.getChanger(), expectedFixLines, expectingFailedFixesAtLines);

// make sure that some of the basics are being reported
assertThat(result.getSummary(), is(not(blankOrNullString())));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.codemodder.testutils;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

import io.codemodder.CodeChanger;
import io.codemodder.FixOnlyCodeChanger;
import io.codemodder.codetf.CodeTFChange;
import io.codemodder.codetf.CodeTFChangesetEntry;
import io.codemodder.codetf.CodeTFResult;
import io.codemodder.codetf.UnfixedFinding;
import java.util.List;

/** Utilities for verifying expected fixes. */
final class ExpectedFixes {

private ExpectedFixes() {}

/** Verify the expected fix metadata. */
static void verifyExpectedFixes(
final CodeTFResult result,
final CodeChanger changer,
final int[] expectedFixLines,
final int[] expectingFailedFixesAtLines) {
List<CodeTFChange> changes =
result.getChangeset().stream()
.map(CodeTFChangesetEntry::getChanges)
.flatMap(List::stream)
.toList();

if (changer instanceof FixOnlyCodeChanger) {
assertThat(changes.stream().anyMatch(c -> !c.getFixedFindings().isEmpty()), is(true));
}

for (int expectedFixLine : expectedFixLines) {
assertThat(changes.stream().anyMatch(c -> c.getLineNumber() == expectedFixLine), is(true));
}

List<UnfixedFinding> unfixedFindings = result.getUnfixedFindings();
for (int expectedFailedFixLine : expectingFailedFixesAtLines) {
assertThat(
unfixedFindings.stream().noneMatch(c -> c.getLine() == expectedFailedFixLine), is(true));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ private void verifySingleCase(
final Metadata metadata,
final Path filePathBefore,
final Path filePathAfter,
final Map<String, List<RuleSarif>> ruleSarifMap)
final Map<String, List<RuleSarif>> ruleSarifMap,
final int[] expectedFixLines,
final int[] expectingFailedFixesAtLines)
throws IOException {

String tmpFileName = trimExtension(filePathBefore);
Expand Down Expand Up @@ -101,6 +103,9 @@ private void verifySingleCase(
assertThat(modifiedFile, equalTo(Files.readString(filePathAfter)));
}
Files.deleteIfExists(tmpFilePath);

ExpectedFixes.verifyExpectedFixes(
result, pair.getChanger(), expectedFixLines, expectingFailedFixesAtLines);
}

private static String trimExtension(final Path path) {
Expand Down Expand Up @@ -145,7 +150,15 @@ private void verifyCodemod(
for (var beforeFile : allBeforeFiles) {
final var afterFile = afterFilesMap.get(trimExtension(beforeFile));
// run the codemod
verifySingleCase(codemod, tmpDir, metadata, beforeFile, afterFile, map);
verifySingleCase(
codemod,
tmpDir,
metadata,
beforeFile,
afterFile,
map,
metadata.expectingFixesAtLines(),
metadata.expectingFailedFixesAtLines());
}
}
}

0 comments on commit 7693903

Please sign in to comment.