From 7693903623007ce1299bc655f088433e0c31aa83 Mon Sep 17 00:00:00 2001 From: Arshan Dabirsiaghi Date: Fri, 19 Apr 2024 00:57:47 -0400 Subject: [PATCH] Create shared utility for verifying fix locations (#358) The different codemod test mixins need to start being brought together, and this is a small step towards that. --------- Co-authored-by: Carlos Uscanga --- .../codemods/MavenSecureURLCodemod.java | 10 ++++- .../codemods/MavenSecureURLCodemodTest.java | 5 +++ .../testutils/CodemodTestMixin.java | 18 +------- .../codemodder/testutils/ExpectedFixes.java | 45 +++++++++++++++++++ .../testutils/RawFileCodemodTest.java | 17 ++++++- 5 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 framework/codemodder-testutils/src/main/java/io/codemodder/testutils/ExpectedFixes.java diff --git a/core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java index 1cbd456b2..e0ed30284 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java @@ -106,7 +106,15 @@ private CodemodFileScanningResult processXml(final Path file, final List 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); }) diff --git a/core-codemods/src/test/java/io/codemodder/codemods/MavenSecureURLCodemodTest.java b/core-codemods/src/test/java/io/codemodder/codemods/MavenSecureURLCodemodTest.java index a78c24157..736252cbb 100644 --- a/core-codemods/src/test/java/io/codemodder/codemods/MavenSecureURLCodemodTest.java +++ b/core-codemods/src/test/java/io/codemodder/codemods/MavenSecureURLCodemodTest.java @@ -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 {} diff --git a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java index 831450217..90b5ee2ca 100644 --- a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java +++ b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java @@ -187,22 +187,8 @@ private void verifyCodemod( assertThat(change.getDescription(), is(not(blankOrNullString()))); } - List 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 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()))); diff --git a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/ExpectedFixes.java b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/ExpectedFixes.java new file mode 100644 index 000000000..080f2f7e4 --- /dev/null +++ b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/ExpectedFixes.java @@ -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 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 unfixedFindings = result.getUnfixedFindings(); + for (int expectedFailedFixLine : expectingFailedFixesAtLines) { + assertThat( + unfixedFindings.stream().noneMatch(c -> c.getLine() == expectedFailedFixLine), is(true)); + } + } +} diff --git a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/RawFileCodemodTest.java b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/RawFileCodemodTest.java index f727e5af1..651c894da 100644 --- a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/RawFileCodemodTest.java +++ b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/RawFileCodemodTest.java @@ -41,7 +41,9 @@ private void verifySingleCase( final Metadata metadata, final Path filePathBefore, final Path filePathAfter, - final Map> ruleSarifMap) + final Map> ruleSarifMap, + final int[] expectedFixLines, + final int[] expectingFailedFixesAtLines) throws IOException { String tmpFileName = trimExtension(filePathBefore); @@ -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) { @@ -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()); } } }